FIX: Use random file name for temporary uploads (#14250)

FIX: Use random file name for temporary uploads (#14250)

Other locale characters in file names (e.g. é, ä) as well as special characters can cause issues on S3, notably the S3 copy object operation does not support these special characters. Instead of storing the original file name in the key, which is unnecessary, we now generate a random file name with the original extension for the temporary file and use that for all external upload stub operations.

diff --git a/lib/file_store/base_store.rb b/lib/file_store/base_store.rb
index 865929e..788e6cd 100644
--- a/lib/file_store/base_store.rb
+++ b/lib/file_store/base_store.rb
@@ -42,12 +42,16 @@ module FileStore
     end
 
     def temporary_upload_path(file_name, folder_prefix: "")
+      # We don't want to use the original file name as it can contain special
+      # characters, which can interfere with external providers operations and
+      # introduce other unexpected behaviour.
+      file_name_random = "#{SecureRandom.hex}#{File.extname(file_name)}"
       File.join(
         TEMPORARY_UPLOAD_PREFIX,
         folder_prefix,
         upload_path,
         SecureRandom.hex,
-        file_name
+        file_name_random
       )
     end
 
diff --git a/spec/multisite/s3_store_spec.rb b/spec/multisite/s3_store_spec.rb
index e86535f..17eb47f 100644
--- a/spec/multisite/s3_store_spec.rb
+++ b/spec/multisite/s3_store_spec.rb
@@ -313,7 +313,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
         url = store.signed_url_for_temporary_upload("test.png")
         key = store.path_from_url(url)
         expect(url).to match(/Amz-Expires/)
-        expect(key).to match(/temp\/uploads\/default\/test_[0-9]\/[a-zA-z0-9]{0,32}\/test.png/)
+        expect(key).to match(/temp\/uploads\/default\/test_[0-9]\/[a-zA-z0-9]{0,32}\/[a-zA-z0-9]{0,32}.png/)
       end
 
       it "presigned url contans the metadata when provided" do
@@ -329,7 +329,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
         url = store.signed_url_for_temporary_upload("test.png")
         key = store.path_from_url(url)
         expect(url).to match(/Amz-Expires/)
-        expect(key).to match(/temp\/site\/uploads\/default\/test_[0-9]\/[a-zA-z0-9]{0,32}\/test.png/)
+        expect(key).to match(/temp\/site\/uploads\/default\/test_[0-9]\/[a-zA-z0-9]{0,32}\/[a-zA-z0-9]{0,32}.png/)
       end
     end
 
@@ -341,7 +341,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
           url = store.signed_url_for_temporary_upload("test.png")
           key = store.path_from_url(url)
           expect(url).to match(/Amz-Expires/)
-          expect(key).to match(/temp\/standard99\/uploads\/second\/test_[0-9]\/[a-zA-z0-9]{0,32}\/test.png/)
+          expect(key).to match(/temp\/standard99\/uploads\/second\/test_[0-9]\/[a-zA-z0-9]{0,32}\/[a-zA-z0-9]{0,32}.png/)
         end
       end
     end

GitHub sha: dd4b8c2afaa9ee8e6a176894cd427c6c3cf66315

This commit appears in #14250 which was approved by lis2. It was merged by martin.