Merge branch 'bvl-security-9-2-28917-contain-uploads-in-system-dir' into 'security-9-2'

(security-9-2) Upload files into `public/upload/system` instead of `public/upload`

See merge request !2104

Conflicts:
  app/validators/dynamic_path_validator.rb

Fixed conflicts based on 3c7c859c359bf5d3955dd300d6861ff33af21ca7

2 files changed, 23 insertions, 4 deletions

diff --git a/app/uploaders/file_uploader.rb b/app/uploaders/file_uploader.rb
index 7e94218c23d..652277e3b78 100644
--- a/app/uploaders/file_uploader.rb
+++ b/app/uploaders/file_uploader.rb
@@ -13,6 +13,13 @@ class FileUploader < GitlabUploader
     )
   end
 
+  # Not using `GitlabUploader.base_dir` because all project namespaces are in
+  # the `public/uploads` dir.
+  #
+  def self.base_dir
+    root_dir
+  end
+
   # Returns the part of `store_dir` that can change based on the model's current
   # path
   #
diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index e0a6c9b4067..449850bf0d5 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -3,16 +3,28 @@ class GitlabUploader < CarrierWave::Uploader::Base
     File.join(CarrierWave.root, upload_record.path)
   end
 
-  def self.base_dir
+  def self.root_dir
     'uploads'
   end
 
-  delegate :base_dir, to: :class
+  # When object storage is used, keep the `root_dir` as `base_dir`.
+  # The files aren't really in folders there, they just have a name.
+  # The files that contain user input in their name, also contain a hash, so
+  # the names are still unique
+  #
+  # This method is overridden in the `FileUploader`
+  def self.base_dir
+    return root_dir unless file_storage?
+
+    File.join(root_dir, 'system')
+  end
 
-  def file_storage?
-    self.class.storage == CarrierWave::Storage::File
+  def self.file_storage?
+    self.storage == CarrierWave::Storage::File
   end
 
+  delegate :base_dir, :file_storage?, to: :class
+
   # Reduce disk IO
   def move_to_cache
     true