diff --git a/lib/s3_helper.rb b/lib/s3_helper.rb index 712929af6f6..e82e1ac1406 100644 --- a/lib/s3_helper.rb +++ b/lib/s3_helper.rb @@ -264,7 +264,12 @@ class S3Helper end def get_path_for_s3_upload(path) - path = File.join(@s3_bucket_folder_path, path) if @s3_bucket_folder_path && path !~ /^#{@s3_bucket_folder_path}\// + if @s3_bucket_folder_path && + !path.starts_with?(@s3_bucket_folder_path) && + !path.starts_with?(File.join(FileStore::BaseStore::TEMPORARY_UPLOAD_PREFIX, @s3_bucket_folder_path)) + return File.join(@s3_bucket_folder_path, path) + end + path end diff --git a/spec/components/file_store/s3_store_spec.rb b/spec/components/file_store/s3_store_spec.rb index 695ee24ba08..cad9c05df41 100644 --- a/spec/components/file_store/s3_store_spec.rb +++ b/spec/components/file_store/s3_store_spec.rb @@ -420,6 +420,18 @@ describe FileStore::S3Store do expect(store.signed_url_for_path("special/optimized/file.png")).not_to eq(upload.url) end + + it "does not prefix the s3_bucket_folder_path onto temporary upload prefixed keys" do + SiteSetting.s3_upload_bucket = "s3-upload-bucket/folder_path" + uri = URI.parse(store.signed_url_for_path("#{FileStore::BaseStore::TEMPORARY_UPLOAD_PREFIX}folder_path/uploads/default/blah/def.xyz")) + expect(uri.path).to eq( + "/#{FileStore::BaseStore::TEMPORARY_UPLOAD_PREFIX}folder_path/uploads/default/blah/def.xyz" + ) + uri = URI.parse(store.signed_url_for_path("uploads/default/blah/def.xyz")) + expect(uri.path).to eq( + "/folder_path/uploads/default/blah/def.xyz" + ) + end end def expect_copy_from(s3_object, source) diff --git a/spec/components/s3_helper_spec.rb b/spec/components/s3_helper_spec.rb index 9d6473885b1..f74c737bfa2 100644 --- a/spec/components/s3_helper_spec.rb +++ b/spec/components/s3_helper_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true -require "s3_helper" require "rails_helper" +require "s3_helper" describe "S3Helper" do let(:client) { Aws::S3::Client.new(stub_responses: true) } @@ -78,11 +78,21 @@ describe "S3Helper" do end it "should prefix bucket folder path only if not exists" do - s3_helper = S3Helper.new('bucket/folder_path', "", client: client) + s3_helper = S3Helper.new("bucket/folder_path", "", client: client) object1 = s3_helper.object("original/1X/def.xyz") object2 = s3_helper.object("folder_path/original/1X/def.xyz") expect(object1.key).to eq(object2.key) end + + it "should not prefix the bucket folder path if the key begins with the temporary upload prefix" do + s3_helper = S3Helper.new("bucket/folder_path", "", client: client) + + object1 = s3_helper.object("original/1X/def.xyz") + object2 = s3_helper.object("#{FileStore::BaseStore::TEMPORARY_UPLOAD_PREFIX}folder_path/uploads/default/blah/def.xyz") + + expect(object1.key).to eq("folder_path/original/1X/def.xyz") + expect(object2.key).to eq("#{FileStore::BaseStore::TEMPORARY_UPLOAD_PREFIX}folder_path/uploads/default/blah/def.xyz") + end end