Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions gems/aws-sdk-s3/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Unreleased Changes
------------------

* Issue - Fix `download_file` single-request mode not writing to a temporary file when given a String/Pathname destination.

1.225.0 (2026-06-02)
------------------

Expand Down
4 changes: 3 additions & 1 deletion gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,9 @@ def resolve_temp_path(opts)
end

def single_request(opts)
params = get_opts(opts[:params]).merge(response_target: opts[:destination])
resolve_temp_path(opts)
target = opts[:temp_path] || opts[:destination]
params = get_opts(opts[:params]).merge(response_target: target)
params[:on_chunk_received] = single_part_progress(opts) if opts[:progress_callback]
resp = @client.get_object(params)
return resp unless opts[:on_checksum_validated]
Expand Down
24 changes: 19 additions & 5 deletions gems/aws-sdk-s3/spec/file_downloader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ module S3
end

it 'downloads a single object using Client#get_object' do
expect(client).to receive(:get_object).with(single_params.merge(response_target: path)).exactly(1).times
client.stub_responses(:get_object, { body: 'body' })
subject.download(path, single_params)
expect(File.read(path)).to eq('body')
end

it 'downloads a large object in parts' do
Expand Down Expand Up @@ -73,7 +74,10 @@ module S3

it 'supports download object with version_id' do
params = single_params.merge(version_id: 'foo')
expect(client).to receive(:get_object).with(params.merge(response_target: path)).exactly(1).times
client.stub_responses(:get_object, lambda do |ctx|
expect(ctx.params[:version_id]).to eq('foo')
{ body: 'body' }
end)

subject.download(path, params)
end
Expand Down Expand Up @@ -140,9 +144,8 @@ module S3
context 'multipart progress' do
it 'reports progress for single object' do
small_file_size = 1024
expect(client)
.to receive(:get_object)
.with(single_params.merge(response_target: path, on_chunk_received: instance_of(Proc))) do |args|
allow(File).to receive(:rename) # mocked get_object writes no temp file
expect(client).to receive(:get_object).exactly(1).times do |args|
args[:on_chunk_received].call(Tempfile.new('small-file'), small_file_size, small_file_size)
end

Expand Down Expand Up @@ -239,6 +242,17 @@ module S3
.to raise_error(Aws::S3::MultipartDownloadError)
end

it 'does not overwrite existing file when single object download fails mid-stream' do
File.write(path, 'existing content')
expect(client).to receive(:get_object) do |params|
File.write(params[:response_target], 'partial')
raise Aws::S3::Errors::InternalError.new(nil, 'connection lost')
end

expect { subject.download(path, single_params) }.to raise_error(Aws::S3::Errors::InternalError)
expect(File.read(path)).to eq('existing content')
end

it 'does not overwrite existing file when download fails' do
File.write(path, 'existing content')

Expand Down
5 changes: 4 additions & 1 deletion gems/aws-sdk-s3/spec/object/download_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,10 @@ module S3
it 'calls progress callback when given' do
n_calls = 0
callback = proc { |_b, _p, _t| n_calls += 1 }
expect(client).to receive(:get_object) { |args| args[:on_chunk_received]&.call('chunk', 1024, 1024) }
expect(client).to receive(:get_object) do |args|
File.write(args[:response_target], 'data')
args[:on_chunk_received]&.call('chunk', 1024, 1024)
end

subject.download_file(path, progress_callback: callback)
expect(n_calls).to eq(1)
Expand Down
5 changes: 4 additions & 1 deletion gems/aws-sdk-s3/spec/transfer_manager_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,10 @@ module S3
it 'calls progress callback when given' do
n_calls = 0
callback = proc { |_b, _p, _t| n_calls += 1 }
expect(client).to receive(:get_object) { |args| args[:on_chunk_received]&.call('chunk', 1024, 1024) }
expect(client).to receive(:get_object) do |args|
File.write(args[:response_target], 'data')
args[:on_chunk_received]&.call('chunk', 1024, 1024)
end

subject.download_file(path, bucket: 'bucket', key: 'key', progress_callback: callback)
expect(n_calls).to eq(1)
Expand Down