Leave upload Content-Type unchaged

There are two important changes in this commit.

1. We only run the content type check when the content_type_whitelist
method returns truthy.
2. We don't alter the uploaded file content type as this causes problems
on Google Cloud Storage.

app/uploaders/content_type_whitelist.rb

@@ -26,14 +26,14 @@ module ContentTypeWhitelist
     # Here we override and extend CarrierWave's method that does not parse the
     # magic headers.
     def check_content_type_whitelist!(new_file)
-      new_file.content_type = mime_magic_content_type(new_file.path)
+      if content_type_whitelist
+        content_type = mime_magic_content_type(new_file.path)
 
-      if content_type_whitelist && !whitelisted_content_type?(new_file.content_type)
-        message = I18n.translate(:"errors.messages.content_type_whitelist_error", allowed_types: Array(content_type_whitelist).join(", "))
-        raise CarrierWave::IntegrityError, message
+        unless whitelisted_content_type?(content_type)
+          message = I18n.translate(:"errors.messages.content_type_whitelist_error", allowed_types: Array(content_type_whitelist).join(", "))
+          raise CarrierWave::IntegrityError, message
+        end
       end
-
-      super(new_file)
     end
 
     def whitelisted_content_type?(content_type)

changelogs/unreleased/212264-gcs-job-log-is-stored-with-content-type-invalid-invalid.yml

+---
+title: Leave upload Content-Type unchaged
+merge_request: 27864
+author:
+type: fixed

spec/support/shared_examples/uploaders/upload_type_shared_examples.rb

@@ -26,3 +26,15 @@ shared_examples 'accepted carrierwave upload' do
     expect { uploader.cache!(fixture_file) }.to change { uploader.file }.from(nil).to(kind_of(CarrierWave::SanitizedFile))
   end
 end
+
+# @param path [String] the path to file to upload. E.g. File.join('spec', 'fixtures', 'sanitized.svg')
+# @param uploader [CarrierWave::Uploader::Base] uploader to handle the upload.
+# @param content_type [String] the upload file content type after cache
+shared_examples 'upload with content type' do |content_type|
+  let(:fixture_file) { fixture_file_upload(path, content_type) }
+
+  it 'will not change upload file content type' do
+    uploader.cache!(fixture_file)
+    expect(uploader.file.content_type).to eq(content_type)
+  end
+end

spec/uploaders/content_type_whitelist_spec.rb

@@ -18,6 +18,7 @@ describe ContentTypeWhitelist do
     let(:path) { File.join('spec', 'fixtures', 'rails_sample.jpg') }
 
     it_behaves_like 'accepted carrierwave upload'
+    it_behaves_like 'upload with content type', 'image/jpeg'
   end
 
   context 'upload non-whitelisted file content type' do

spec/uploaders/job_artifact_uploader_spec.rb

@@ -97,5 +97,12 @@ describe JobArtifactUploader do
 
     it_behaves_like "migrates", to_store: described_class::Store::REMOTE
     it_behaves_like "migrates", from_store: described_class::Store::REMOTE, to_store: described_class::Store::LOCAL
+
+    # CI job artifacts usually are shown as text/plain, but they contain
+    # escape characters so MIME detectors usually fail to determine what
+    # the Content-Type is.
+    it 'does not set Content-Type' do
+      expect(uploader.file.content_type).to be_blank
+    end
   end
 end