Add latest changes from gitlab-org/gitlab@master

4 files changed, 43 insertions, 18 deletions

diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb
index dfa80eb4a64..5fe06b9c5e6 100644
--- a/lib/gitlab/diff/position.rb
+++ b/lib/gitlab/diff/position.rb
@@ -118,8 +118,14 @@ module Gitlab
             path: file_path
           }
 
+          # Takes action when creating diff notes (multiple calls are
+          # submitted to this method).
           Gitlab::SafeRequestStore.fetch(key) { find_diff_file(repository) }
         end
+
+        # We need to unfold diff lines according to the position in order
+        # to correctly calculate the line code and trace position changes.
+        @diff_file&.tap { |file| file.unfold_diff_lines(self) }
       end
 
       def diff_options
@@ -152,13 +158,7 @@ module Gitlab
         return unless diff_refs.complete?
         return unless comparison = diff_refs.compare_in(repository.project)
 
-        file = comparison.diffs(diff_options).diff_files.first
-
-        # We need to unfold diff lines according to the position in order
-        # to correctly calculate the line code and trace position changes.
-        file&.unfold_diff_lines(self)
-
-        file
+        comparison.diffs(diff_options).diff_files.first
       end
 
       def get_formatter_class(type)
diff --git a/lib/gitlab/file_markdown_link_builder.rb b/lib/gitlab/file_markdown_link_builder.rb
index 180140e7da2..e9e5172e6f8 100644
--- a/lib/gitlab/file_markdown_link_builder.rb
+++ b/lib/gitlab/file_markdown_link_builder.rb
@@ -10,7 +10,7 @@ module Gitlab
       return unless name = markdown_name
 
       markdown = "[#{name.gsub(']', '\\]')}](#{secure_url})"
-      markdown = "!#{markdown}" if image_or_video? || dangerous?
+      markdown = "!#{markdown}" if image_or_video? || dangerous_image_or_video?
       markdown
     end
 
diff --git a/lib/gitlab/file_type_detection.rb b/lib/gitlab/file_type_detection.rb
index 25ee07cf940..c2b9dfa562d 100644
--- a/lib/gitlab/file_type_detection.rb
+++ b/lib/gitlab/file_type_detection.rb
@@ -1,34 +1,59 @@
 # frozen_string_literal: true
 
-# File helpers methods.
-# It needs the method filename to be defined.
+# The method `filename` must be defined in classes that use this module.
+#
+# This module is intended to be used as a helper and not a security gate
+# to validate that a file is safe, as it identifies files only by the
+# file extension and not its actual contents.
+#
+# An example useage of this module is in `FileMarkdownLinkBuilder` that
+# renders markdown depending on a file name.
+#
+# We use Workhorse to detect the real extension when we serve files with
+# the `SendsBlob` helper methods, and ask Workhorse to set the content
+# type when it serves the file:
+# https://gitlab.com/gitlab-org/gitlab-ce/blob/33e5955/app/helpers/workhorse_helper.rb#L48.
+#
+# Because Workhorse has access to the content when it is downloaded, if
+# the type/extension doesn't match the real type, we adjust the
+# `Content-Type` and `Content-Disposition` to the one we get from the detection.
 module Gitlab
   module FileTypeDetection
-    IMAGE_EXT = %w[png jpg jpeg gif bmp tiff ico].freeze
+    SAFE_IMAGE_EXT = %w[png jpg jpeg gif bmp tiff ico].freeze
     # We recommend using the .mp4 format over .mov. Videos in .mov format can
     # still be used but you really need to make sure they are served with the
     # proper MIME type video/mp4 and not video/quicktime or your videos won't play
     # on IE >= 9.
     # http://archive.sublimevideo.info/20150912/docs.sublimevideo.net/troubleshooting.html
-    VIDEO_EXT = %w[mp4 m4v mov webm ogv].freeze
+    SAFE_VIDEO_EXT = %w[mp4 m4v mov webm ogv].freeze
+
     # These extension types can contain dangerous code and should only be embedded inline with
     # proper filtering. They should always be tagged as "Content-Disposition: attachment", not "inline".
-    DANGEROUS_EXT = %w[svg].freeze
+    DANGEROUS_IMAGE_EXT = %w[svg].freeze
+    DANGEROUS_VIDEO_EXT = [].freeze # None, yet
 
     def image?
-      extension_match?(IMAGE_EXT)
+      extension_match?(SAFE_IMAGE_EXT)
     end
 
     def video?
-      extension_match?(VIDEO_EXT)
+      extension_match?(SAFE_VIDEO_EXT)
     end
 
     def image_or_video?
       image? || video?
     end
 
-    def dangerous?
-      extension_match?(DANGEROUS_EXT)
+    def dangerous_image?
+      extension_match?(DANGEROUS_IMAGE_EXT)
+    end
+
+    def dangerous_video?
+      extension_match?(DANGEROUS_VIDEO_EXT)
+    end
+
+    def dangerous_image_or_video?
+      dangerous_image? || dangerous_video?
     end
 
     private
diff --git a/lib/gitlab/usage_data.rb b/lib/gitlab/usage_data.rb
index ed2693aaedf..c5303dad558 100644
--- a/lib/gitlab/usage_data.rb
+++ b/lib/gitlab/usage_data.rb
@@ -17,7 +17,6 @@ module Gitlab
           .merge(features_usage_data)
           .merge(components_usage_data)
           .merge(cycle_analytics_usage_data)
-          .merge(usage_counters)
       end
 
       def to_json(force_refresh: false)
@@ -99,6 +98,7 @@ module Gitlab
             web_hooks: count(WebHook)
           }.merge(services_usage)
             .merge(approximate_counts)
+            .merge(usage_counters)
         }.tap do |data|
           data[:counts][:user_preferences] = user_preferences_usage
         end