From 6a31219ae8350b04bb5b332ae239e0138360d832 Mon Sep 17 00:00:00 2001 From: Bartosz Rak Date: Wed, 8 Apr 2026 18:55:31 +0200 Subject: [PATCH 1/9] feat: migrate S3 storage to aws-sdk v2 and bump to 2.7.0.3 [no-ado] - Replace AWS::S3.new with Aws::S3::Resource.new - Replace s3_interface.buckets[name] with s3_interface.bucket(name) - Replace s3_bucket.objects[key] with s3_bucket.object(key) - Replace s3_object.write(file, opts) with s3_object.put(body: file, opts) - Replace s3_object.read with s3_object.get.body.read - Replace s3_interface.buckets.create with s3_interface.create_bucket - Replace expiring_url to use Aws::S3::Presigner#presigned_url - Replace all AWS::Errors/AWS::S3::Errors rescues with Aws:: equivalents - Convert ACL symbols (:public_read) to hyphenated strings ("public-read") - Add translate_s3_options to map v1 keys (s3_force_path_style, use_ssl) to their v2 equivalents or drop them appropriately - Remove dead establish_connection! and use_secure_protocol? methods - Build endpoint from URI scheme instead of :use_ssl option - Require :region (falls back to AWS_REGION env or us-east-1) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/paperclip/storage/s3.rb | 81 +++++++++++++++++++++++-------------- lib/paperclip/version.rb | 2 +- 2 files changed, 52 insertions(+), 31 deletions(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index aec2c9b10..fec2538a6 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -3,8 +3,8 @@ module Storage # Amazon's S3 file hosting service is a scalable, easy place to store files for # distribution. You can find out more about it at http://aws.amazon.com/s3 # - # To use Paperclip with S3, include the +aws-sdk+ gem in your Gemfile: - # gem 'aws-sdk' + # To use Paperclip with S3, include the +aws-sdk+ gem (v2+) in your Gemfile: + # gem 'aws-sdk', '~> 2.0' # There are a few S3-specific options for has_attached_file: # * +s3_credentials+: Takes a path, a File, or a Hash. The path (or File) must point # to a YAML file containing the +access_key_id+ and +secret_access_key+ that Amazon @@ -88,7 +88,7 @@ def self.extended base rescue LoadError => e e.message << " (You may need to install the aws-sdk gem)" raise e - end unless defined?(AWS::Core) + end unless defined?(Aws::S3) base.instance_eval do @s3_options = @options[:s3_options] || {} @@ -140,8 +140,14 @@ def self.extended base end def expiring_url(time = 3600, style_name = default_style) - if path - s3_object(style_name).url_for(:read, :expires => time, :secure => use_secure_protocol?(style_name)).to_s + if path(style_name) + presigner = Aws::S3::Presigner.new(:client => s3_interface.client) + presigner.presigned_url( + :get_object, + :bucket => bucket_name, + :key => path(style_name).sub(%r{^/}, ''), + :expires_in => time + ) end end @@ -167,10 +173,18 @@ def bucket_name def s3_interface @s3_interface ||= begin - config = { :s3_endpoint => s3_host_name } + config = {} + config[:region] = s3_credentials[:region] || ENV['AWS_REGION'] || 'us-east-1' + + # Use_ssl is implied by the endpoint URI scheme. Build a custom endpoint + # only when not using the default AWS S3 host (e.g. MinIO in development). + host = s3_host_name + unless host == 's3.amazonaws.com' + use_ssl = !@s3_options.key?(:use_ssl) || @s3_options[:use_ssl] + config[:endpoint] = "#{use_ssl ? 'https' : 'http'}://#{host}" + end if using_http_proxy? - proxy_opts = { :host => http_proxy_host } proxy_opts[:port] = http_proxy_port if http_proxy_port if http_proxy_user @@ -185,16 +199,18 @@ def s3_interface config[opt] = s3_credentials[opt] if s3_credentials[opt] end - AWS::S3.new(config.merge(@s3_options)) + config.merge!(translate_s3_options(@s3_options)) + + Aws::S3::Resource.new(config) end end def s3_bucket - @s3_bucket ||= s3_interface.buckets[bucket_name] + @s3_bucket ||= s3_interface.bucket(bucket_name) end def s3_object style_name = default_style - s3_bucket.objects[path(style_name).sub(%r{^/},'')] + s3_bucket.object(path(style_name).sub(%r{^/},'')) end def using_http_proxy? @@ -238,9 +254,7 @@ def exists?(style = default_style) else false end - rescue AWS::Errors::Base => e - false - end + rescue Aws::Errors::ServiceError def s3_permissions(style = default_style) s3_permissions = @s3_permissions[style] || @s3_permissions[:default] @@ -268,13 +282,13 @@ def to_file style = default_style basename = File.basename(filename, extname) file = Tempfile.new([basename, extname]) file.binmode - file.write(s3_object(style).read) + file.write(s3_object(style).get.body.read) file.rewind return file end def create_bucket - s3_interface.buckets.create(bucket_name) + s3_interface.create_bucket(:bucket => bucket_name) end def flush_writes #:nodoc: @@ -283,7 +297,11 @@ def flush_writes #:nodoc: log("saving #{path(style)}") acl = @s3_permissions[style] || @s3_permissions[:default] acl = acl.call(self, style) if acl.respond_to?(:call) + # SDK v2 expects ACL as a hyphenated string (e.g. "public-read"), + # whereas v1 used underscore symbols (e.g. :public_read). + acl = acl.to_s.tr('_', '-') write_options = { + :body => file, :content_type => file.content_type.to_s.strip, :acl => acl } @@ -292,8 +310,8 @@ def flush_writes #:nodoc: write_options[:server_side_encryption] = @s3_server_side_encryption end write_options.merge!(@s3_headers) - s3_object(style).write(file, write_options) - rescue AWS::S3::Errors::NoSuchBucket => e + s3_object(style).put(write_options) + rescue Aws::S3::Errors::NoSuchBucket create_bucket retry end @@ -308,8 +326,8 @@ def flush_deletes #:nodoc: @queued_for_delete.each do |path| begin log("deleting #{path}") - s3_bucket.objects[path.sub(%r{^/},'')].delete - rescue AWS::Errors::Base => e + s3_bucket.object(path.sub(%r{^/},'')).delete + rescue Aws::Errors::ServiceError # Ignore this. end end @@ -330,18 +348,21 @@ def find_credentials creds end private :find_credentials - def establish_connection! - @connection ||= AWS::S3::Base.establish_connection!( @s3_options.merge( - :access_key_id => s3_credentials[:access_key_id], - :secret_access_key => s3_credentials[:secret_access_key] - )) - end - private :establish_connection! - - def use_secure_protocol?(style_name) - s3_protocol(style_name) == "https" + # Translate legacy aws-sdk v1 s3_options keys to their v2 equivalents. + # Unknown keys are passed through so callers don't lose custom options. + def translate_s3_options(opts) + return opts if opts.empty? + translated = opts.dup + # v1: :s3_force_path_style => v2: :force_path_style + if translated.key?(:s3_force_path_style) + translated[:force_path_style] = translated.delete(:s3_force_path_style) + end + # v1: :use_ssl is handled via the endpoint URI scheme; drop it here + # so it is not passed as an unknown option to Aws::S3::Resource.new. + translated.delete(:use_ssl) + translated end - private :use_secure_protocol? + private :translate_s3_options end end end diff --git a/lib/paperclip/version.rb b/lib/paperclip/version.rb index b2fd25740..26d58fb23 100644 --- a/lib/paperclip/version.rb +++ b/lib/paperclip/version.rb @@ -1,3 +1,3 @@ module Paperclip - VERSION = "2.7.0.2" unless defined? Paperclip::VERSION + VERSION = "2.7.0.3" unless defined? Paperclip::VERSION end From f358232acd93cdcf3adc4713508847aa5ee79cd3 Mon Sep 17 00:00:00 2001 From: Bartosz Rak Date: Thu, 9 Apr 2026 12:40:31 +0200 Subject: [PATCH 2/9] fix: pin aws-sdk development dependency to v2 [no-ado] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- paperclip.gemspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/paperclip.gemspec b/paperclip.gemspec index 990f291e8..36e6d6eb6 100644 --- a/paperclip.gemspec +++ b/paperclip.gemspec @@ -28,7 +28,7 @@ Gem::Specification.new do |s| s.add_development_dependency('shoulda') s.add_development_dependency('appraisal', '~> 0.4.0') s.add_development_dependency('mocha') - s.add_development_dependency('aws-sdk') + s.add_development_dependency('aws-sdk', '~> 2.0') s.add_development_dependency('sqlite3', '~> 1.3.4') s.add_development_dependency('cucumber', '~> 1.1.0') s.add_development_dependency('aruba') From acbef0fd2e13d83e4c4eda0a80ba86ace99ffd7a Mon Sep 17 00:00:00 2001 From: Bartosz Rak Date: Thu, 9 Apr 2026 13:24:10 +0200 Subject: [PATCH 3/9] fix: address PR review comments for aws-sdk v2 migration [no-ado] - Fix exists? method: add missing rescue body (false) and closing end - Update expiring_url tests to mock Aws::S3::Presigner instead of url_for - Update flush_writes tests to assert put() with :body and string ACLs - Migrate test setup from AWS v1 (require 'aws', AWS.config) to v2 - Update remaining v1 class references in tests (bucket creation, delete, exists) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/paperclip/storage/s3.rb | 2 + test/storage/s3_test.rb | 85 +++++++++++++++++++++++++------------ 2 files changed, 59 insertions(+), 28 deletions(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index fec2538a6..0d58bc300 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -255,6 +255,8 @@ def exists?(style = default_style) false end rescue Aws::Errors::ServiceError + false + end def s3_permissions(style = default_style) s3_permissions = @s3_permissions[style] || @s3_permissions[:default] diff --git a/test/storage/s3_test.rb b/test/storage/s3_test.rb index 625fb709e..d617265c2 100644 --- a/test/storage/s3_test.rb +++ b/test/storage/s3_test.rb @@ -1,5 +1,5 @@ require './test/helper' -require 'aws' +require 'aws-sdk' class S3Test < Test::Unit::TestCase def rails_env(env) @@ -9,11 +9,11 @@ def rails_env(env) end def setup - AWS.config(:access_key_id => "TESTKEY", :secret_access_key => "TESTSECRET", :stub_requests => true) + Aws.config.update(:access_key_id => "TESTKEY", :secret_access_key => "TESTSECRET", :stub_responses => true) end def teardown - AWS.config(:access_key_id => nil, :secret_access_key => nil, :stub_requests => nil) + Aws.config.update(:access_key_id => nil, :secret_access_key => nil, :stub_responses => nil) end context "Parsing S3 credentials" do @@ -360,9 +360,14 @@ def counter @dummy = Dummy.new @dummy.avatar = StringIO.new(".") - object = stub - @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:url_for).with(:read, :expires => 3600, :secure => true) + presigner = stub + Aws::S3::Presigner.stubs(:new).returns(presigner) + presigner.expects(:presigned_url).with( + :get_object, + :bucket => "prod_bucket", + :key => "avatars/stringio.txt", + :expires_in => 3600 + ) @dummy.avatar.expiring_url end @@ -391,16 +396,26 @@ def counter end should "should generate a url for the thumb" do - object = stub - @dummy.avatar.stubs(:s3_object).with(:thumb).returns(object) - object.expects(:url_for).with(:read, :expires => 1800, :secure => true) + presigner = stub + Aws::S3::Presigner.stubs(:new).returns(presigner) + presigner.expects(:presigned_url).with( + :get_object, + :bucket => "prod_bucket", + :key => "avatars/thumb/stringio.txt", + :expires_in => 1800 + ) @dummy.avatar.expiring_url(1800, :thumb) end should "should generate a url for the default style" do - object = stub - @dummy.avatar.stubs(:s3_object).with(:original).returns(object) - object.expects(:url_for).with(:read, :expires => 1800, :secure => true) + presigner = stub + Aws::S3::Presigner.stubs(:new).returns(presigner) + presigner.expects(:presigned_url).with( + :get_object, + :bucket => "prod_bucket", + :key => "avatars/original/stringio.txt", + :expires_in => 1800 + ) @dummy.avatar.expiring_url(1800) end end @@ -517,11 +532,13 @@ def counter context "and saved without a bucket" do setup do - AWS::S3::BucketCollection.any_instance.expects(:create).with("testing") - AWS::S3::S3Object.any_instance.stubs(:write). - raises(AWS::S3::Errors::NoSuchBucket.new(stub, - stub(:status => 404, - :body => ""))). + s3_resource = stub + @dummy.avatar.stubs(:s3_interface).returns(s3_resource) + s3_resource.expects(:create_bucket).with(:bucket => "testing") + object = stub + @dummy.avatar.stubs(:s3_object).returns(object) + object.stubs(:put). + raises(Aws::S3::Errors::NoSuchBucket.new(stub, "no such bucket")). then.returns(nil) @dummy.save end @@ -533,8 +550,14 @@ def counter context "and remove" do setup do - AWS::S3::S3Object.any_instance.stubs(:exists?).returns(true) - AWS::S3::S3Object.any_instance.stubs(:delete) + object = stub + @dummy.avatar.stubs(:s3_object).returns(object) + object.stubs(:exists?).returns(true) + bucket = stub + @dummy.avatar.stubs(:s3_bucket).returns(bucket) + s3_obj = stub + bucket.stubs(:object).returns(s3_obj) + s3_obj.stubs(:delete) @dummy.destroy_attached_files end @@ -545,7 +568,9 @@ def counter context 'that the file were missing' do setup do - AWS::S3::S3Object.any_instance.stubs(:exists?).raises(AWS::Errors::Base) + object = stub + @dummy.avatar.stubs(:s3_object).returns(object) + object.stubs(:exists?).raises(Aws::Errors::ServiceError.new(stub, "error")) end should 'return false on exists?' do @@ -874,9 +899,10 @@ def counter setup do object = stub @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :public_read) + :acl => "public-read") @dummy.save end @@ -912,9 +938,10 @@ def counter setup do object = stub @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :private) + :acl => "private") @dummy.save end @@ -957,9 +984,10 @@ def counter [:thumb, :original].each do |style| object = stub @dummy.avatar.stubs(:s3_object).with(style).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => style == :thumb ? :public_read : :private) + :acl => style == :thumb ? "public-read" : "private") end @dummy.save end @@ -1005,9 +1033,10 @@ def counter [:thumb, :original].each do |style| object = stub @dummy.avatar.stubs(:s3_object).with(style).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => style == :thumb ? :public_read : :private) + :acl => style == :thumb ? "public-read" : "private") end @dummy.save end From fa0e51d9e98e648ee893136317cf43907c6fd653 Mon Sep 17 00:00:00 2001 From: Bartosz Rak Date: Thu, 9 Apr 2026 13:40:38 +0200 Subject: [PATCH 4/9] fix: address second round of PR review comments [no-ado] - Fix s3_interface endpoint: only set custom :endpoint for non-AWS hosts (match *.amazonaws.com) instead of just s3.amazonaws.com, preventing signature/redirect issues with AWS regional hostnames - Fix s3_protocol default proc: normalize permission to string before comparing, so both :public_read and "public-read" resolve correctly - Remove Gemfile.lock from tracking (already in .gitignore) since it still pinned aws-sdk v1 - Update test s3_endpoint/config assertions to use s3_host_name and s3_credentials instead of v1 s3_bucket.config accessors - Update all remaining .write expectations to .put with :body and string ACLs across 8 additional test locations Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- Gemfile.lock | 137 ------------------------------------ lib/paperclip/storage/s3.rb | 8 ++- test/storage/s3_test.rb | 53 +++++++------- 3 files changed, 34 insertions(+), 164 deletions(-) delete mode 100644 Gemfile.lock diff --git a/Gemfile.lock b/Gemfile.lock deleted file mode 100644 index 1fcd104f2..000000000 --- a/Gemfile.lock +++ /dev/null @@ -1,137 +0,0 @@ -PATH - remote: . - specs: - paperclip (2.7.0) - activerecord (>= 2.3.0) - activesupport (>= 2.3.2) - cocaine (>= 0.0.2) - mime-types - -GEM - remote: http://rubygems.org/ - specs: - activemodel (3.2.1) - activesupport (= 3.2.1) - builder (~> 3.0.0) - activerecord (3.2.1) - activemodel (= 3.2.1) - activesupport (= 3.2.1) - arel (~> 3.0.0) - tzinfo (~> 0.3.29) - activesupport (3.2.1) - i18n (~> 0.6) - multi_json (~> 1.0) - appraisal (0.4.1) - bundler - rake - arel (3.0.2) - aruba (0.4.6) - bcat (>= 0.6.1) - childprocess (>= 0.2.0) - cucumber (>= 1.0.2) - rdiscount (>= 1.6.8) - rspec (>= 2.6.0) - aws-sdk (1.3.4) - httparty (~> 0.7) - json (~> 1.4) - nokogiri (>= 1.4.4) - uuidtools (~> 2.1) - bcat (0.6.2) - rack (~> 1.0) - builder (3.0.0) - capybara (1.1.1) - mime-types (>= 1.16) - nokogiri (>= 1.3.3) - rack (>= 1.0.0) - rack-test (>= 0.5.4) - selenium-webdriver (~> 2.0) - xpath (~> 0.1.4) - childprocess (0.2.2) - ffi (~> 1.0.6) - cocaine (0.2.0) - cucumber (1.1.4) - builder (>= 2.1.2) - diff-lcs (>= 1.1.2) - gherkin (~> 2.7.1) - json (>= 1.4.6) - term-ansicolor (>= 1.0.6) - diff-lcs (1.1.3) - excon (0.6.6) - fakeweb (1.3.0) - ffi (1.0.9) - fog (0.11.0) - builder - excon (~> 0.6.5) - formatador (~> 0.2.0) - mime-types - multi_json (~> 1.0.3) - net-scp (~> 1.0.4) - net-ssh (~> 2.1.4) - nokogiri (~> 1.5.0) - ruby-hmac - formatador (0.2.1) - gherkin (2.7.1) - json (>= 1.4.6) - httparty (0.8.1) - multi_json - multi_xml - i18n (0.6.0) - json (1.6.5) - json_pure (1.6.1) - metaclass (0.0.1) - mime-types (1.16) - mocha (0.10.0) - metaclass (~> 0.0.1) - multi_json (1.0.4) - multi_xml (0.4.1) - net-scp (1.0.4) - net-ssh (>= 1.99.1) - net-ssh (2.1.4) - nokogiri (1.5.0) - rack (1.3.3) - rack-test (0.6.1) - rack (>= 1.0) - rake (0.9.2.2) - rdiscount (1.6.8) - rspec (2.6.0) - rspec-core (~> 2.6.0) - rspec-expectations (~> 2.6.0) - rspec-mocks (~> 2.6.0) - rspec-core (2.6.4) - rspec-expectations (2.6.0) - diff-lcs (~> 1.1.2) - rspec-mocks (2.6.0) - ruby-hmac (0.4.0) - rubyzip (0.9.4) - selenium-webdriver (2.7.0) - childprocess (>= 0.2.1) - ffi (>= 1.0.7) - json_pure - rubyzip - shoulda (2.11.3) - sqlite3 (1.3.4) - term-ansicolor (1.0.7) - tzinfo (0.3.31) - uuidtools (2.1.2) - xpath (0.1.4) - nokogiri (~> 1.3) - -PLATFORMS - ruby - -DEPENDENCIES - appraisal (~> 0.4.0) - aruba - aws-sdk - bundler - capybara - cocaine (~> 0.2) - cucumber (~> 1.1.0) - fakeweb - fog - jruby-openssl - mocha - paperclip! - rake - shoulda - sqlite3 (~> 1.3.4) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index 0d58bc300..c08087670 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -97,7 +97,7 @@ def self.extended base Proc.new do |style, attachment| permission = (@s3_permissions[style.to_sym] || @s3_permissions[:default]) permission = permission.call(attachment, style) if permission.is_a?(Proc) - (permission == :public_read) ? 'http' : 'https' + (permission.to_s.tr('_', '-') == 'public-read') ? 'http' : 'https' end @s3_metadata = @options[:s3_metadata] || {} @s3_headers = @options[:s3_headers] || {} @@ -177,9 +177,11 @@ def s3_interface config[:region] = s3_credentials[:region] || ENV['AWS_REGION'] || 'us-east-1' # Use_ssl is implied by the endpoint URI scheme. Build a custom endpoint - # only when not using the default AWS S3 host (e.g. MinIO in development). + # only when the host is not an AWS S3 hostname (e.g. MinIO in development). + # AWS regional hostnames (s3-.amazonaws.com) should not be set as + # a custom endpoint — the SDK resolves them from :region automatically. host = s3_host_name - unless host == 's3.amazonaws.com' + unless host =~ /\.amazonaws\.com\z/i use_ssl = !@s3_options.key?(:use_ssl) || @s3_options[:use_ssl] config[:endpoint] = "#{use_ssl ? 'https' : 'http'}://#{host}" end diff --git a/test/storage/s3_test.rb b/test/storage/s3_test.rb index d617265c2..5e3d21a9d 100644 --- a/test/storage/s3_test.rb +++ b/test/storage/s3_test.rb @@ -172,7 +172,7 @@ def teardown end should "use the S3 bucket with the correct host name" do - assert_equal "s3-ap-northeast-1.amazonaws.com", @dummy.avatar.s3_bucket.config.s3_endpoint + assert_equal "s3-ap-northeast-1.amazonaws.com", @dummy.avatar.s3_host_name end end @@ -457,19 +457,16 @@ def counter should "get the right s3_host_name in production" do rails_env("production") assert_match %r{^s3-world-end.amazonaws.com}, @dummy.avatar.s3_host_name - assert_match %r{^s3-world-end.amazonaws.com}, @dummy.avatar.s3_bucket.config.s3_endpoint end should "get the right s3_host_name in development" do rails_env("development") assert_match %r{^s3-ap-northeast-1.amazonaws.com}, @dummy.avatar.s3_host_name - assert_match %r{^s3-ap-northeast-1.amazonaws.com}, @dummy.avatar.s3_bucket.config.s3_endpoint end should "get the right s3_host_name if the key does not exist" do rails_env("test") assert_match %r{^s3.amazonaws.com}, @dummy.avatar.s3_host_name - assert_match %r{^s3.amazonaws.com}, @dummy.avatar.s3_bucket.config.s3_endpoint end end @@ -511,9 +508,10 @@ def counter setup do object = stub @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :public_read) + :acl => "public-read") @dummy.save end @@ -620,9 +618,10 @@ def counter setup do object = stub @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :public_read, + :acl => "public-read", :cache_control => 'max-age=31557600') @dummy.save end @@ -659,9 +658,10 @@ def counter setup do object = stub @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :public_read, + :acl => "public-read", :metadata => { "color" => "red" }) @dummy.save end @@ -698,9 +698,10 @@ def counter setup do object = stub @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :public_read, + :acl => "public-read", :metadata => { "color" => "red" }) @dummy.save end @@ -737,9 +738,10 @@ def counter setup do object = stub @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :public_read, + :acl => "public-read", :storage_class => "reduced_redundancy") @dummy.save end @@ -776,9 +778,10 @@ def counter setup do object = stub @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :public_read, + :acl => "public-read", :server_side_encryption => :aes256) @dummy.save end @@ -815,9 +818,10 @@ def counter setup do object = stub @dummy.avatar.stubs(:s3_object).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :public_read, + :acl => "public-read", :storage_class => :reduced_redundancy) @dummy.save end @@ -846,8 +850,8 @@ def counter should "parse the credentials" do assert_equal 'pathname_bucket', @dummy.avatar.bucket_name - assert_equal 'pathname_key', @dummy.avatar.s3_bucket.config.access_key_id - assert_equal 'pathname_secret', @dummy.avatar.s3_bucket.config.secret_access_key + assert_equal 'pathname_key', @dummy.avatar.s3_credentials[:access_key_id] + assert_equal 'pathname_secret', @dummy.avatar.s3_credentials[:secret_access_key] end end @@ -869,8 +873,8 @@ def counter should "run the file through ERB" do assert_equal 'env_bucket', @dummy.avatar.bucket_name - assert_equal 'env_key', @dummy.avatar.s3_bucket.config.access_key_id - assert_equal 'env_secret', @dummy.avatar.s3_bucket.config.secret_access_key + assert_equal 'env_key', @dummy.avatar.s3_credentials[:access_key_id] + assert_equal 'env_secret', @dummy.avatar.s3_credentials[:secret_access_key] end end @@ -1085,9 +1089,10 @@ def counter [:thumb, :original].each do |style| object = stub @dummy.avatar.stubs(:s3_object).with(style).returns(object) - object.expects(:write).with(anything, + object.expects(:put).with( + :body => anything, :content_type => "image/png", - :acl => :public_read, + :acl => "public-read", :content_disposition => 'attachment; filename="Custom Avatar Name.png"') end @dummy.save From bb14e7b43f6dcb7f247701c50e3e3e3da03c8900 Mon Sep 17 00:00:00 2001 From: Bartosz Rak Date: Thu, 9 Apr 2026 15:37:46 +0200 Subject: [PATCH 5/9] fix: address third round of PR review comments [no-ado] - Infer :region from s3_host_name when it matches an AWS regional pattern (e.g. s3-ap-northeast-1.amazonaws.com) and no explicit region is provided, preventing wrong-region requests - Translate legacy :s3_endpoint option to :endpoint in translate_s3_options to avoid ArgumentError from aws-sdk v2 - Preserve s3_protocol behavior in expiring_url by applying protocol override to presigned URLs via apply_s3_protocol_to_url helper Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/paperclip/storage/s3.rb | 43 +++++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index c08087670..bcafe7838 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -142,12 +142,13 @@ def self.extended base def expiring_url(time = 3600, style_name = default_style) if path(style_name) presigner = Aws::S3::Presigner.new(:client => s3_interface.client) - presigner.presigned_url( + url = presigner.presigned_url( :get_object, :bucket => bucket_name, :key => path(style_name).sub(%r{^/}, ''), :expires_in => time ) + apply_s3_protocol_to_url(url, style_name) end end @@ -181,7 +182,15 @@ def s3_interface # AWS regional hostnames (s3-.amazonaws.com) should not be set as # a custom endpoint — the SDK resolves them from :region automatically. host = s3_host_name - unless host =~ /\.amazonaws\.com\z/i + if host =~ /\.amazonaws\.com\z/i + # Infer region from the hostname when no explicit region was provided, + # so that s3_host_name like "s3-ap-northeast-1.amazonaws.com" routes + # to the correct region instead of falling back to us-east-1. + if !s3_credentials[:region] && !ENV['AWS_REGION'] + inferred = infer_region_from_host(host) + config[:region] = inferred if inferred + end + else use_ssl = !@s3_options.key?(:use_ssl) || @s3_options[:use_ssl] config[:endpoint] = "#{use_ssl ? 'https' : 'http'}://#{host}" end @@ -361,12 +370,42 @@ def translate_s3_options(opts) if translated.key?(:s3_force_path_style) translated[:force_path_style] = translated.delete(:s3_force_path_style) end + # v1: :s3_endpoint => v2: :endpoint + if translated.key?(:s3_endpoint) + translated[:endpoint] = translated.delete(:s3_endpoint) + end # v1: :use_ssl is handled via the endpoint URI scheme; drop it here # so it is not passed as an unknown option to Aws::S3::Resource.new. translated.delete(:use_ssl) translated end private :translate_s3_options + + # Replace the scheme of a presigned URL to match s3_protocol, preserving + # backward-compatible http/https behavior for expiring_url. + def apply_s3_protocol_to_url(url, style_name) + protocol = s3_protocol(style_name).to_s.sub(%r{://\z}, '') + if protocol == 'http' || protocol == 'https' + url.sub(%r{\Ahttps?://}, "#{protocol}://") + else + url + end + end + private :apply_s3_protocol_to_url + + # Extract region from an AWS S3 hostname. + # Supports "s3-.amazonaws.com" and "s3..amazonaws.com". + # Returns nil for "s3.amazonaws.com" (the us-east-1 default). + def infer_region_from_host(host) + case host + when /\As3[.-]([^.]+)\.amazonaws\.com\z/i + region = $1 + region == 'amazonaws' ? nil : region + else + nil + end + end + private :infer_region_from_host end end end From e4ddd76a6532725fd626594a9f07312692f76b79 Mon Sep 17 00:00:00 2001 From: Bartosz Rak Date: Thu, 9 Apr 2026 15:54:08 +0200 Subject: [PATCH 6/9] fix: validate inferred region against AWS region pattern [no-ado] Add aws_region_token? guard so infer_region_from_host only returns a region for tokens matching the AWS pattern (e.g. us-east-1, ap-northeast-1), preventing invalid regions from hostnames like s3-world-end.amazonaws.com. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/paperclip/storage/s3.rb | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index bcafe7838..8892937f7 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -395,17 +395,23 @@ def apply_s3_protocol_to_url(url, style_name) # Extract region from an AWS S3 hostname. # Supports "s3-.amazonaws.com" and "s3..amazonaws.com". - # Returns nil for "s3.amazonaws.com" (the us-east-1 default). + # Returns nil for "s3.amazonaws.com" (the us-east-1 default) and + # any hostname token that is not a valid AWS region identifier. def infer_region_from_host(host) case host when /\As3[.-]([^.]+)\.amazonaws\.com\z/i - region = $1 - region == 'amazonaws' ? nil : region + region = $1.downcase + aws_region_token?(region) ? region : nil else nil end end private :infer_region_from_host + + def aws_region_token?(token) + token.match?(/\A[a-z]{2}(?:-[a-z]+)+-\d+\z/) + end + private :aws_region_token? end end end From 8ffea226b0f58fdb9670f1147918f00fb108998c Mon Sep 17 00:00:00 2001 From: Bartosz Rak Date: Thu, 9 Apr 2026 16:02:23 +0200 Subject: [PATCH 7/9] fix: use :http_proxy instead of v1 :proxy_uri for aws-sdk v2 [no-ado] Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/paperclip/storage/s3.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index 8892937f7..a34b8c37a 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -203,7 +203,7 @@ def s3_interface userinfo += ":#{http_proxy_password}" if http_proxy_password proxy_opts[:userinfo] = userinfo end - config[:proxy_uri] = URI::HTTP.build(proxy_opts) + config[:http_proxy] = URI::HTTP.build(proxy_opts).to_s end [:access_key_id, :secret_access_key].each do |opt| From 265b74302df5ba5fd15a158a7c0ffcdcc3006180 Mon Sep 17 00:00:00 2001 From: Bartosz Rak Date: Thu, 9 Apr 2026 16:13:19 +0200 Subject: [PATCH 8/9] fix: normalize storage_class/server_side_encryption to v2 strings and fix presigner test returns [no-ado] - Upcase storage_class and server_side_encryption in flush_writes for v2 compat (e.g. :reduced_redundancy -> REDUCED_REDUNDANCY, :aes256 -> AES256) - Add .returns() to presigner expectations so apply_s3_protocol_to_url doesn't raise NoMethodError on nil - Update test assertions to expect uppercase enum strings Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/paperclip/storage/s3.rb | 4 ++++ test/storage/s3_test.rb | 12 ++++++------ 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index a34b8c37a..257dd9c1c 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -323,6 +323,10 @@ def flush_writes #:nodoc: write_options[:server_side_encryption] = @s3_server_side_encryption end write_options.merge!(@s3_headers) + # SDK v2 expects enum values as uppercase strings + # (e.g. "REDUCED_REDUNDANCY" not :reduced_redundancy, "AES256" not :aes256) + write_options[:storage_class] = write_options[:storage_class].to_s.upcase if write_options[:storage_class] + write_options[:server_side_encryption] = write_options[:server_side_encryption].to_s.upcase if write_options[:server_side_encryption] s3_object(style).put(write_options) rescue Aws::S3::Errors::NoSuchBucket create_bucket diff --git a/test/storage/s3_test.rb b/test/storage/s3_test.rb index 5e3d21a9d..e4a7a8205 100644 --- a/test/storage/s3_test.rb +++ b/test/storage/s3_test.rb @@ -367,7 +367,7 @@ def counter :bucket => "prod_bucket", :key => "avatars/stringio.txt", :expires_in => 3600 - ) + ).returns("https://prod_bucket.s3.amazonaws.com/avatars/stringio.txt?X-Amz-Expires=3600") @dummy.avatar.expiring_url end @@ -403,7 +403,7 @@ def counter :bucket => "prod_bucket", :key => "avatars/thumb/stringio.txt", :expires_in => 1800 - ) + ).returns("https://prod_bucket.s3.amazonaws.com/avatars/thumb/stringio.txt?X-Amz-Expires=1800") @dummy.avatar.expiring_url(1800, :thumb) end @@ -415,7 +415,7 @@ def counter :bucket => "prod_bucket", :key => "avatars/original/stringio.txt", :expires_in => 1800 - ) + ).returns("https://prod_bucket.s3.amazonaws.com/avatars/original/stringio.txt?X-Amz-Expires=1800") @dummy.avatar.expiring_url(1800) end end @@ -742,7 +742,7 @@ def counter :body => anything, :content_type => "image/png", :acl => "public-read", - :storage_class => "reduced_redundancy") + :storage_class => "REDUCED_REDUNDANCY") @dummy.save end @@ -782,7 +782,7 @@ def counter :body => anything, :content_type => "image/png", :acl => "public-read", - :server_side_encryption => :aes256) + :server_side_encryption => "AES256") @dummy.save end @@ -822,7 +822,7 @@ def counter :body => anything, :content_type => "image/png", :acl => "public-read", - :storage_class => :reduced_redundancy) + :storage_class => "REDUCED_REDUNDANCY") @dummy.save end From 3a940b3b2169f006a7a483a914acfc7c3549d9ae Mon Sep 17 00:00:00 2001 From: Bartosz Rak Date: Thu, 9 Apr 2026 16:32:43 +0200 Subject: [PATCH 9/9] fix: replace match? with =~ for broader Ruby compatibility [no-ado] String#match? was added in Ruby 2.4; use =~ instead since aws-sdk v2 supports Ruby >= 1.9.3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- lib/paperclip/storage/s3.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/paperclip/storage/s3.rb b/lib/paperclip/storage/s3.rb index 257dd9c1c..b4a0d08b7 100644 --- a/lib/paperclip/storage/s3.rb +++ b/lib/paperclip/storage/s3.rb @@ -413,7 +413,7 @@ def infer_region_from_host(host) private :infer_region_from_host def aws_region_token?(token) - token.match?(/\A[a-z]{2}(?:-[a-z]+)+-\d+\z/) + !!(token =~ /\A[a-z]{2}(?:-[a-z]+)+-\d+\z/) end private :aws_region_token? end