Skip to content

Commit

Permalink
Fixes from changed spec and new MPU behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
mullermp committed Oct 10, 2024
1 parent eb14386 commit 2b20f4c
Show file tree
Hide file tree
Showing 11 changed files with 57 additions and 112 deletions.
2 changes: 1 addition & 1 deletion gems/aws-sdk-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ Unreleased Changes

* Feature - Always calculate request checksums for operations that support or require it. Supported config options are `WHEN_SUPPORTED` and `WHEN_REQUIRED`. The default value is `WHEN_SUPPORTED`. This option is configured in code with `:request_checksum_calculation`, in the shared config file as `request_checksum_calculation`, and in the ENV as `ENV['AWS_REQUEST_CHECKSUM_CALCULATION']`.

* Feature - Always validate response checksums for operations that support or require it. Supported config options are `WHEN_SUPPORTED` and `WHEN_REQUIRED`. The default value is `WHEN_SUPPORTED`. This option is configured in code with `:response_checksum_calculation`, in the shared config file as `response_checksum_calculation`, and in the ENV as `ENV['AWS_RESPONSE_CHECKSUM_CALCULATION']`.
* Feature - Always validate response checksums for operations that support or require it. Supported config options are `WHEN_SUPPORTED` and `WHEN_REQUIRED`. The default value is `WHEN_SUPPORTED`. This option is configured in code with `:response_checksum_validation`, in the shared config file as `response_checksum_validation`, and in the ENV as `ENV['AWS_response_checksum_validation']`.

3.209.1 (2024-09-25)
------------------
Expand Down
18 changes: 9 additions & 9 deletions gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin
resolve_request_checksum_calculation(cfg)
end

option(:response_checksum_calculation,
option(:response_checksum_validation,
doc_default: 'WHEN_SUPPORTED',
doc_type: 'String',
docstring: <<~DOCS) do |cfg|
Expand All @@ -69,7 +69,7 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin
response payloads of operations unless the checksum algorithm is supported and
the `requestValidationModeMember` member is set to `ENABLED`.
DOCS
resolve_response_checksum_calculation(cfg)
resolve_response_checksum_validation(cfg)
end

class << self
Expand Down Expand Up @@ -113,15 +113,15 @@ def resolve_request_checksum_calculation(cfg)
mode
end

def resolve_response_checksum_calculation(cfg)
mode = ENV['AWS_RESPONSE_CHECKSUM_CALCULATION'] ||
Aws.shared_config.response_checksum_calculation(profile: cfg.profile) ||
def resolve_response_checksum_validation(cfg)
mode = ENV['AWS_response_checksum_validation'] ||
Aws.shared_config.response_checksum_validation(profile: cfg.profile) ||
'WHEN_SUPPORTED'
mode = mode.upcase
unless %w[WHEN_SUPPORTED WHEN_REQUIRED].include?(mode)
raise ArgumentError,
'expected :response_checksum_calculation or' \
" ENV['AWS_RESPONSE_CHECKSUM_CALCULATION'] to be " \
'expected :response_checksum_validation or' \
" ENV['AWS_response_checksum_validation'] to be " \
'`WHEN_SUPPORTED` or `WHEN_REQUIRED`.'
end
mode
Expand Down Expand Up @@ -160,7 +160,7 @@ def call(context)
context[:http_checksum] ||= {}

# Set validation mode to enabled when supported.
if context.config.response_checksum_calculation == 'WHEN_SUPPORTED'
if context.config.response_checksum_validation == 'WHEN_SUPPORTED'
enable_request_validation_mode(context)
end

Expand Down Expand Up @@ -229,7 +229,7 @@ def add_request_config_metric(config, metrics)
end

def add_response_config_metric(config, metrics)
case config.response_checksum_calculation
case config.response_checksum_validation
when 'WHEN_SUPPORTED'
metrics << 'FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED'
when 'WHEN_REQUIRED'
Expand Down
2 changes: 1 addition & 1 deletion gems/aws-sdk-core/lib/aws-sdk-core/shared_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ def self.config_reader(*attrs)
:adaptive_retry_wait_to_fill,
:correct_clock_skew,
:request_checksum_calculation,
:response_checksum_calculation,
:response_checksum_validation,
:csm_client_id,
:csm_enabled,
:csm_host,
Expand Down
22 changes: 11 additions & 11 deletions gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -127,31 +127,31 @@ module Plugins
end
end

describe 'response_checksum_calculation' do
describe 'response_checksum_validation' do
it 'is configured to always verify by default' do
expect(client.config.response_checksum_calculation)
expect(client.config.response_checksum_validation)
.to eq('WHEN_SUPPORTED')
end

it 'can be configured using shared config' do
allow_any_instance_of(Aws::SharedConfig)
.to receive(:response_checksum_calculation)
.to receive(:response_checksum_validation)
.and_return('WHEN_REQUIRED')
expect(client.config.response_checksum_calculation)
expect(client.config.response_checksum_validation)
.to eq('WHEN_REQUIRED')
end

it 'can be configured using ENV with precedence over shared config' do
allow_any_instance_of(Aws::SharedConfig)
.to receive(:response_checksum_calculation)
.to receive(:response_checksum_validation)
.and_return('WHEN_SUPPORTED')
ENV['AWS_RESPONSE_CHECKSUM_CALCULATION'] = 'WHEN_REQUIRED'
expect(client.config.response_checksum_calculation)
ENV['AWS_response_checksum_validation'] = 'WHEN_REQUIRED'
expect(client.config.response_checksum_validation)
.to eq('WHEN_REQUIRED')
end

it 'raises when response_checksum_calculation is not valid' do
ENV['AWS_RESPONSE_CHECKSUM_CALCULATION'] = 'peccy'
it 'raises when response_checksum_validation is not valid' do
ENV['AWS_response_checksum_validation'] = 'peccy'
expect { client }.to raise_error(ArgumentError, /WHEN_SUPPORTED/)
end
end
Expand Down Expand Up @@ -373,7 +373,7 @@ def stub_client(client)
it 'WHEN_REQUIRED; not ENABLED; does not validate the checksum' do
client = checksum_client.new(
stub_responses: true,
response_checksum_calculation: 'WHEN_REQUIRED'
response_checksum_validation: 'WHEN_REQUIRED'
)
stub_client(client)
resp = client.http_checksum_operation
Expand All @@ -383,7 +383,7 @@ def stub_client(client)
it 'WHEN_REQUIRED; ENABLED; validates the checksum' do
client = checksum_client.new(
stub_responses: true,
response_checksum_calculation: 'WHEN_REQUIRED'
response_checksum_validation: 'WHEN_REQUIRED'
)
stub_client(client)
resp = client.http_checksum_operation(validation_mode: 'ENABLED')
Expand Down
2 changes: 1 addition & 1 deletion gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ class Client < Seahorse::Client::Base
# where server-side-encryption is used with customer-provided keys.
# This should only be disabled for local testing.
#
# @option options [String] :response_checksum_calculation ("WHEN_SUPPORTED")
# @option options [String] :response_checksum_validation ("WHEN_SUPPORTED")
# Determines when checksum validation will be performed on response payloads. Values are:
#
# * `WHEN_SUPPORTED` - (default) When set, checksum validation is performed on all
Expand Down
23 changes: 3 additions & 20 deletions gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,7 @@ def download(destination, options = {})
key: options[:key],
}
@params[:version_id] = options[:version_id] if options[:version_id]

# checksum_mode only supports the value "ENABLED"
# falsey values (false/nil) or "DISABLED" should be considered
# disabled and the api parameter should be unset.
if (checksum_mode = options.fetch(:checksum_mode, 'ENABLED'))
@params[:checksum_mode] = checksum_mode unless checksum_mode.upcase == 'DISABLED'
end
@on_checksum_validated = options[:on_checksum_validated]

@progress_callback = options[:progress_callback]

validate!
Expand Down Expand Up @@ -67,11 +59,6 @@ def download(destination, options = {})
private

def validate!
if @on_checksum_validated && @params[:checksum_mode] != 'ENABLED'
raise ArgumentError, "You must set checksum_mode: 'ENABLED' " +
"when providing a on_checksum_validated callback"
end

if @on_checksum_validated && !@on_checksum_validated.respond_to?(:call)
raise ArgumentError, 'on_checksum_validated must be callable'
end
Expand Down Expand Up @@ -164,9 +151,7 @@ def multithreaded_get_by_parts(n_parts, total_size)

def download_in_threads(pending, total_size)
threads = []
if @progress_callback
progress = MultipartProgress.new(pending, total_size, @progress_callback)
end
progress = MultipartProgress.new(pending, total_size, @progress_callback) if @progress_callback
@thread_count.times do
thread = Thread.new do
begin
Expand Down Expand Up @@ -208,9 +193,7 @@ def single_request

return resp unless @on_checksum_validated

if resp.checksum_validated
@on_checksum_validated.call(resp.checksum_validated, resp)
end
@on_checksum_validated.call(resp.checksum_validated, resp) if resp.checksum_validated

resp
end
Expand Down Expand Up @@ -251,7 +234,7 @@ def each(&block)
end

# @api private
class MultipartProgress
class MultipartProgress
def initialize(parts, total_size, progress_callback)
@bytes_received = Array.new(parts.size, 0)
@part_sizes = parts.map(&:size)
Expand Down
11 changes: 5 additions & 6 deletions gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -162,12 +162,11 @@ def upload_in_threads(pending, completed, options)
completed_part = {
etag: resp.etag,
part_number: part[:part_number]
}.tap do |h|
# get checksum algorithm from request params (default or part)
algorithm = resp.context.params[:checksum_algorithm]
k = "checksum_#{algorithm.downcase}".to_sym
# get the requested checksum from the response
h[k] = resp.send(k)
}
# get the requested checksum from the response
if part[:checksum_algorithm]
k = "checksum_#{part[:checksum_algorithm].downcase}".to_sym
completed_part[k] = resp[k]
end
completed.push(completed_part)
end
Expand Down
17 changes: 0 additions & 17 deletions gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/checksum_algorithm.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,29 +18,12 @@ def call(context)
end
end

# create_multipart_upload is not modeled with httpChecksum. For
# multipart requests, we must set a default for the checksum algorithm
# for it to f unction.
class DefaultMultipartChecksumHandler < Seahorse::Client::Handler
def call(context)
default = Aws::Plugins::ChecksumAlgorithm::DEFAULT_CHECKSUM
context.params[:checksum_algorithm] ||= default
@handler.call(context)
end
end

def add_handlers(handlers, _config)
handlers.add(
SkipWholeMultipartGetChecksumsHandler,
step: :initialize,
operations: [:get_object]
)

handlers.add(
DefaultMultipartChecksumHandler,
step: :initialize,
operations: [:create_multipart_upload]
)
end
end
end
Expand Down
10 changes: 0 additions & 10 deletions gems/aws-sdk-s3/spec/object/download_file_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ module S3
allow(client).to receive(:head_object).with({
bucket: 'bucket',
key: 'small',
checksum_mode: 'ENABLED',
part_number: 1
}).and_return(
client.stub_data(
Expand All @@ -62,7 +61,6 @@ module S3
bucket: 'bucket',
key: 'small',
part_number: 1,
checksum_mode: 'ENABLED',
version_id: version_id
}).and_return(
client.stub_data(
Expand All @@ -75,7 +73,6 @@ module S3
allow(client).to receive(:head_object).with({
bucket: 'bucket',
key: 'large',
checksum_mode: 'ENABLED',
part_number: 1
}).and_return(
client.stub_data(
Expand All @@ -87,7 +84,6 @@ module S3

allow(client).to receive(:head_object).with({
bucket: 'bucket',
checksum_mode: 'ENABLED',
key: 'large'
}).and_return(
client.stub_data(
Expand All @@ -99,7 +95,6 @@ module S3
allow(client).to receive(:head_object).with({
bucket: 'bucket',
key: 'single',
checksum_mode: 'ENABLED',
part_number: 1
}).and_return(
client.stub_data(
Expand All @@ -114,7 +109,6 @@ module S3
expect(client).to receive(:get_object).with({
bucket: 'bucket',
key: 'small',
checksum_mode: 'ENABLED',
response_target: path
}).exactly(1).times

Expand All @@ -126,7 +120,6 @@ module S3
expect(client).to receive(:get_object).with({
bucket: 'bucket',
key: 'small',
checksum_mode: 'ENABLED',
response_target: path,
on_chunk_received: instance_of(Proc)
}) do |args|
Expand All @@ -149,7 +142,6 @@ module S3
expect(client).to receive(:head_object).with({
bucket: 'bucket',
key: 'large',
checksum_mode: 'ENABLED',
part_number: 1
}).exactly(1).times

Expand Down Expand Up @@ -189,7 +181,6 @@ module S3
expect(client).to receive(:head_object).with({
bucket: 'bucket',
key: 'single',
checksum_mode: 'ENABLED',
part_number: 1
}).exactly(1).times

Expand All @@ -204,7 +195,6 @@ module S3
expect(client).to receive(:get_object).with({
bucket: 'bucket',
key: 'small',
checksum_mode: 'ENABLED',
version_id: version_id,
response_target: path
}).exactly(1).times
Expand Down
Loading

0 comments on commit 2b20f4c

Please sign in to comment.