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.
This commit is contained in:
Martin Brennan 2021-09-06 10:21:20 +10:00 committed by GitHub
parent f859fd6bde
commit dd4b8c2afa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 8 additions and 4 deletions

View File

@ -42,12 +42,16 @@ module FileStore
end end
def temporary_upload_path(file_name, folder_prefix: "") 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( File.join(
TEMPORARY_UPLOAD_PREFIX, TEMPORARY_UPLOAD_PREFIX,
folder_prefix, folder_prefix,
upload_path, upload_path,
SecureRandom.hex, SecureRandom.hex,
file_name file_name_random
) )
end end

View File

@ -313,7 +313,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
url = store.signed_url_for_temporary_upload("test.png") url = store.signed_url_for_temporary_upload("test.png")
key = store.path_from_url(url) key = store.path_from_url(url)
expect(url).to match(/Amz-Expires/) 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 end
it "presigned url contans the metadata when provided" do 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") url = store.signed_url_for_temporary_upload("test.png")
key = store.path_from_url(url) key = store.path_from_url(url)
expect(url).to match(/Amz-Expires/) 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
end end
@ -341,7 +341,7 @@ RSpec.describe 'Multisite s3 uploads', type: :multisite do
url = store.signed_url_for_temporary_upload("test.png") url = store.signed_url_for_temporary_upload("test.png")
key = store.path_from_url(url) key = store.path_from_url(url)
expect(url).to match(/Amz-Expires/) 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 end
end end