From 2fe918517ec15980c110d16689fece0afc5cf5ce Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Mon, 19 Aug 2024 16:51:35 -0400 Subject: [PATCH 01/18] Default checksum calculation for httpChecksum to CRC32 --- build_tools/services.rb | 2 +- gems/aws-sdk-core/CHANGELOG.md | 4 + .../plugins/checksum_algorithm.rb | 417 ++++++++++------ .../lib/aws-sdk-core/plugins/http_checksum.rb | 2 +- .../lib/aws-sdk-core/shared_config.rb | 2 + .../aws/plugins/checksum_algorithm_spec.rb | 463 ++++++++++++------ .../spec/aws/plugins/checksum_request.json | 47 ++ .../spec/aws/plugins/checksum_response.json | 107 ++++ .../plugins/checksum_streaming_request.json | 62 +++ .../spec/aws/plugins/http_checksum_spec.rb | 18 +- gems/aws-sdk-s3/CHANGELOG.md | 2 + .../lib/aws-sdk-s3/multipart_file_uploader.rb | 15 +- .../plugins/express_session_auth.rb | 31 +- .../aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb | 85 +--- .../aws-sdk-s3/spec/encryption/client_spec.rb | 1 - gems/aws-sdk-s3/spec/plugins/md5s_spec.rb | 158 ------ 16 files changed, 839 insertions(+), 577 deletions(-) create mode 100644 gems/aws-sdk-core/spec/aws/plugins/checksum_request.json create mode 100644 gems/aws-sdk-core/spec/aws/plugins/checksum_response.json create mode 100644 gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json delete mode 100644 gems/aws-sdk-s3/spec/plugins/md5s_spec.rb diff --git a/build_tools/services.rb b/build_tools/services.rb index 0a89ffd6fe2..43dddb88608 100644 --- a/build_tools/services.rb +++ b/build_tools/services.rb @@ -13,7 +13,7 @@ class ServiceEnumerator MINIMUM_CORE_VERSION = "3.201.0" # Minimum `aws-sdk-core` version for new S3 gem builds - MINIMUM_CORE_VERSION_S3 = "3.201.0" + MINIMUM_CORE_VERSION_S3 = "3.202.0" EVENTSTREAM_PLUGIN = "Aws::Plugins::EventStreamConfiguration" diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index dacef83d581..d9f1f725eab 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -1,6 +1,10 @@ Unreleased Changes ------------------ +* Feature - Always calculate 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 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']`. + 3.201.4 (2024-08-08) ------------------ diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index 2c14e819aac..5bf6bd9eddc 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -14,13 +14,15 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin require 'aws-crt' supported << 'CRC32C' rescue LoadError + # Ignored end supported end.freeze - # priority order of checksum algorithms to validate responses against - # Remove any algorithms not supported by client (ie, depending on CRT availability) - CHECKSUM_ALGORITHM_PRIORITIES = %w[CRC32C SHA1 CRC32 SHA256] & CLIENT_ALGORITHMS + # Priority order of checksum algorithms to validate responses against. + # Remove any algorithms not supported by client (ie, depending on CRT availability). + # This list was chosen based on average performance. + CHECKSUM_ALGORITHM_PRIORITIES = %w[CRC32 CRC32C CRC64NVME SHA1 SHA256] & CLIENT_ALGORITHMS # byte size of checksums, used in computing the trailer length CHECKSUM_SIZE = { @@ -28,7 +30,97 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin 'CRC32C' => 16, 'SHA1' => 36, 'SHA256' => 52 - } + }.freeze + + option(:request_checksum_calculation, + doc_default: 'WHEN_SUPPORTED', + doc_type: 'String', + docstring: <<~DOCS) do |cfg| + Determines when a checksum will be calculated for request payloads. Values are: + + * `WHEN_SUPPORTED` - (default) When set, a checksum will be + calculated for all request payloads of operations modeled with the + `httpChecksum` trait where `requestChecksumRequired` is `true` and/or a + `requestAlgorithmMember` is modeled. + * `WHEN_REQUIRED` - When set, a checksum will only be calculated for + request payloads of operations modeled with the `httpChecksum` trait where + `requestChecksumRequired` is `true` or where a requestAlgorithmMember + is modeled and supplied. + DOCS + resolve_request_checksum_calculation(cfg) + end + + option(:response_checksum_calculation, + doc_default: 'WHEN_SUPPORTED', + doc_type: 'String', + docstring: <<~DOCS) do |cfg| + Determines when checksum validation will be performed on response payloads. Values are: + + * `WHEN_SUPPORTED` - (default) When set, checksum validation is performed on all + response payloads of operations modeled with the `httpChecksum` trait where + `responseAlgorithms` is modeled, except when no modeled checksum algorithms + are supported. + * `WHEN_REQUIRED` - When set, checksum validation is not performed on + response payloads of operations unless the checksum algorithm is supported and + the `requestValidationModeMember` member is set to `ENABLED`. + DOCS + resolve_response_checksum_calculation(cfg) + end + + class << self + def digest_for_algorithm(algorithm) + case algorithm + when 'CRC32' + Digest32.new(Zlib.method(:crc32)) + when 'CRC32C' + # this will only be used if input algorithm is CRC32C AND client supports it (crt available) + Digest32.new(Aws::Crt::Checksums.method(:crc32c)) + when 'SHA1' + Digest::SHA1.new + when 'SHA256' + Digest::SHA256.new + else + raise ArgumentError, + "#{algorithm} is not a supported checksum algorithm." + end + end + + # The trailer size (in bytes) is the overhead + the trailer name + + # the length of the base64 encoded checksum + def trailer_length(algorithm, location_name) + CHECKSUM_SIZE[algorithm] + location_name.size + end + + private + + def resolve_request_checksum_calculation(cfg) + mode = ENV['AWS_REQUEST_CHECKSUM_CALCULATION'] || + Aws.shared_config.request_checksum_calculation(profile: cfg.profile) || + 'WHEN_SUPPORTED' + mode = mode.upcase + unless %w[WHEN_SUPPORTED WHEN_REQUIRED].include?(mode) + raise ArgumentError, + 'expected :request_checksum_calculation or' \ + " ENV['AWS_REQUEST_CHECKSUM_CALCULATION'] to be " \ + '`WHEN_SUPPORTED` or `WHEN_REQUIRED`.' + end + mode + end + + def resolve_response_checksum_calculation(cfg) + mode = ENV['AWS_RESPONSE_CHECKSUM_CALCULATION'] || + Aws.shared_config.response_checksum_calculation(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 " \ + '`WHEN_SUPPORTED` or `WHEN_REQUIRED`.' + end + mode + end + end # Interface for computing digests on request/response bodies # which may be files, strings or IO like objects @@ -55,79 +147,59 @@ def base64digest def add_handlers(handlers, _config) handlers.add(OptionHandler, step: :initialize) - # priority set low to ensure checksum is computed AFTER the request is - # built but before it is signed + # Priority is set low to ensure the checksum is computed AFTER the + # request is built but before it is signed. handlers.add(ChecksumHandler, priority: 15, step: :build) end - private - - def self.request_algorithm_selection(context) - return unless context.operation.http_checksum - - input_member = context.operation.http_checksum['requestAlgorithmMember'] - context.params[input_member.to_sym]&.upcase if input_member - end - - def self.request_validation_mode(context) - return unless context.operation.http_checksum - - input_member = context.operation.http_checksum['requestValidationModeMember'] - context.params[input_member.to_sym] if input_member - end - - def self.operation_response_algorithms(context) - return unless context.operation.http_checksum - - context.operation.http_checksum['responseAlgorithms'] - end - - - # @api private class OptionHandler < Seahorse::Client::Handler def call(context) - context[:http_checksum] ||= {} - - # validate request configuration - if (request_input = ChecksumAlgorithm.request_algorithm_selection(context)) - unless CLIENT_ALGORITHMS.include? request_input - if (request_input == 'CRC32C') - raise ArgumentError, "CRC32C requires crt support - install the aws-crt gem for support." - else - raise ArgumentError, "#{request_input} is not a supported checksum algorithm." - end - end + # Enable validation on the service in all cases + if context.config.response_checksum_calculation == 'WHEN_SUPPORTED' + enable_request_validation_mode(context) end - # validate response configuration - if (ChecksumAlgorithm.request_validation_mode(context)) - # Compute an ordered list as the union between priority supported and the - # operation's modeled response algorithms. - validation_list = CHECKSUM_ALGORITHM_PRIORITIES & - ChecksumAlgorithm.operation_response_algorithms(context) - context[:http_checksum][:validation_list] = validation_list + # Default checksum member to CRC32 if not set + default_request_algorithm_member(context) + # Not modeled with httpChecksum + if context.operation_name == :create_multipart_upload + context.params[:checksum_algorithm] ||= 'CRC32' end @handler.call(context) end + + private + + def enable_request_validation_mode(context) + return unless context.operation.http_checksum + + input_member = context.operation.http_checksum['requestValidationModeMember'] + context.params[input_member.to_sym] = 'ENABLED' if input_member + end + + def default_request_algorithm_member(context) + return unless context.operation.http_checksum + + input_member = context.operation.http_checksum['requestAlgorithmMember'] + context.params[input_member.to_sym] ||= 'CRC32' if input_member + end end - # @api private class ChecksumHandler < Seahorse::Client::Handler - def call(context) + context[:http_checksum] ||= {} + if should_calculate_request_checksum?(context) - request_algorithm_input = ChecksumAlgorithm.request_algorithm_selection(context) || - context[:default_request_checksum_algorithm] - context[:checksum_algorithms] = request_algorithm_input - - request_checksum_property = { - 'algorithm' => request_algorithm_input, - 'in' => checksum_request_in(context), - 'name' => "x-amz-checksum-#{request_algorithm_input.downcase}" + algorithm = choose_request_algorithm!(context) + request_algorithm = { + algorithm: algorithm, + in: checksum_request_in(context), + name: "x-amz-checksum-#{algorithm.downcase}" } - calculate_request_checksum(context, request_checksum_property) + context[:http_checksum][:request_algorithm] = request_algorithm + calculate_request_checksum(context, request_algorithm) end if should_verify_response_checksum?(context) @@ -139,32 +211,108 @@ def call(context) private + def request_algorithm_selection(context) + return unless context.operation.http_checksum + + input_member = context.operation.http_checksum['requestAlgorithmMember'] + context.params[input_member.to_sym]&.upcase if input_member + end + + def request_validation_mode(context) + return unless context.operation.http_checksum + + input_member = context.operation.http_checksum['requestValidationModeMember'] + context.params[input_member.to_sym] if input_member + end + + def operation_response_algorithms(context) + return unless context.operation.http_checksum + + context.operation.http_checksum['responseAlgorithms'] + end + + def checksum_required?(context) + (http_checksum = context.operation.http_checksum) && + (checksum_required = http_checksum['requestChecksumRequired']) && + (checksum_required && context.config.request_checksum_calculation == 'WHEN_REQUIRED') + end + + def checksum_optional?(context) + (http_checksum = context.operation.http_checksum) && + !http_checksum['requestChecksumRequired'].nil? && + context.config.request_checksum_calculation == 'WHEN_SUPPORTED' + end + + def checksum_provided_as_header?(headers) + headers.any? { |k, _| k.start_with?('x-amz-checksum-') } + end + def should_calculate_request_checksum?(context) - context.operation.http_checksum && - (ChecksumAlgorithm.request_algorithm_selection(context) || - context[:default_request_checksum_algorithm]) + (checksum_required?(context) || checksum_optional?(context)) && + !checksum_provided_as_header?(context.http_request.headers) end - def should_verify_response_checksum?(context) - context[:http_checksum][:validation_list] && !context[:http_checksum][:validation_list].empty? + def choose_request_algorithm!(context) + algorithm = request_algorithm_selection(context) + return algorithm if CLIENT_ALGORITHMS.include?(algorithm) + + if algorithm == 'CRC32C' + raise ArgumentError, + 'CRC32C requires CRT support - install the aws-crt gem' + end + + raise ArgumentError, + "#{algorithm} is not a supported checksum algorithm." + end + + def checksum_request_in(context) + if context.operation['unsignedPayload'] || + context.operation['authtype'] == 'v4-unsigned-body' + 'trailer' + else + 'header' + end end def calculate_request_checksum(context, checksum_properties) - case checksum_properties['in'] + case checksum_properties[:in] when 'header' - header_name = checksum_properties['name'] - body = context.http_request.body_contents - if body - context.http_request.headers[header_name] ||= - ChecksumAlgorithm.calculate_checksum(checksum_properties['algorithm'], body) + header_name = checksum_properties[:name] + headers = context.http_request.headers + unless headers[header_name] + body = context.http_request.body_contents + headers[header_name] = calculate_checksum( + checksum_properties[:algorithm], + body + ) end when 'trailer' apply_request_trailer_checksum(context, checksum_properties) end end + def calculate_checksum(algorithm, body) + digest = ChecksumAlgorithm.digest_for_algorithm(algorithm) + if body.respond_to?(:read) + update_in_chunks(digest, body) + else + digest.update(body) + end + digest.base64digest + end + + def update_in_chunks(digest, io) + loop do + chunk = io.read(CHUNK_SIZE) + break unless chunk + + digest.update(chunk) + end + io.rewind + end + def apply_request_trailer_checksum(context, checksum_properties) - location_name = checksum_properties['name'] + location_name = checksum_properties[:name] # set required headers headers = context.http_request.headers @@ -176,121 +324,88 @@ def apply_request_trailer_checksum(context, checksum_properties) # to set the Content-Length header (set by content_length plugin). # This means we cannot use Transfer-Encoding=chunked - if !context.http_request.body.respond_to?(:size) + unless context.http_request.body.respond_to?(:size) raise Aws::Errors::ChecksumError, 'Could not determine length of the body' end headers['X-Amz-Decoded-Content-Length'] = context.http_request.body.size context.http_request.body = AwsChunkedTrailerDigestIO.new( context.http_request.body, - checksum_properties['algorithm'], + checksum_properties[:algorithm], location_name ) end + def should_verify_response_checksum?(context) + request_validation_mode(context) == 'ENABLED' + end + # Add events to the http_response to verify the checksum as its read # This prevents the body from being read multiple times # verification is done only once a successful response has completed def add_verify_response_checksum_handlers(context) - http_response = context.http_response - checksum_context = { } - http_response.on_headers do |_status, headers| - header_name, algorithm = response_header_to_verify(headers, context[:http_checksum][:validation_list]) - if header_name - expected = headers[header_name] - - unless context[:http_checksum][:skip_on_suffix] && /-[\d]+$/.match(expected) - checksum_context[:algorithm] = algorithm - checksum_context[:header_name] = header_name - checksum_context[:digest] = ChecksumAlgorithm.digest_for_algorithm(algorithm) - checksum_context[:expected] = expected - end - end - end + checksum_context = {} + add_verify_response_headers_handler(context, checksum_context) + add_verify_response_data_handler(context, checksum_context) + add_verify_response_success_handler(context, checksum_context) + end - http_response.on_data do |chunk| - checksum_context[:digest].update(chunk) if checksum_context[:digest] + def add_verify_response_headers_handler(context, checksum_context) + validation_list = CHECKSUM_ALGORITHM_PRIORITIES & + operation_response_algorithms(context) + context[:http_checksum][:validation_list] = validation_list + + context.http_response.on_headers do |_status, headers| + header_name, algorithm = response_header_to_verify( + headers, + validation_list + ) + next unless header_name + + expected = headers[header_name] + next if context[:http_checksum][:skip_on_suffix] && /-\d+$/.match(expected) + + checksum_context[:algorithm] = algorithm + checksum_context[:header_name] = header_name + checksum_context[:digest] = ChecksumAlgorithm.digest_for_algorithm(algorithm) + checksum_context[:expected] = expected end + end - http_response.on_success do - if checksum_context[:digest] && - (computed = checksum_context[:digest].base64digest) + def add_verify_response_data_handler(context, checksum_context) + context.http_response.on_data do |chunk| + checksum_context[:digest]&.update(chunk) + end + end - if computed != checksum_context[:expected] - raise Aws::Errors::ChecksumError, - "Checksum validation failed on #{checksum_context[:header_name]} "\ - "computed: #{computed}, expected: #{checksum_context[:expected]}" - end + def add_verify_response_success_handler(context, checksum_context) + context.http_response.on_success do + next unless checksum_context[:digest] + computed = checksum_context[:digest].base64digest + if computed == checksum_context[:expected] context[:http_checksum][:validated] = checksum_context[:algorithm] + else + raise Aws::Errors::ChecksumError, + "Checksum validation failed on #{checksum_context[:header_name]} "\ + "computed: #{computed}, expected: #{checksum_context[:expected]}" end end end - # returns nil if no headers to verify def response_header_to_verify(headers, validation_list) validation_list.each do |algorithm| - header_name = "x-amz-checksum-#{algorithm}" + header_name = "x-amz-checksum-#{algorithm.downcase}" return [header_name, algorithm] if headers[header_name] end nil end - - # determine where (header vs trailer) a request checksum should be added - def checksum_request_in(context) - if context.operation['unsignedPayload'] || - context.operation['authtype'] == 'v4-unsigned-body' - 'trailer' - else - 'header' - end - end - - end - - def self.calculate_checksum(algorithm, body) - digest = ChecksumAlgorithm.digest_for_algorithm(algorithm) - if body.respond_to?(:read) - ChecksumAlgorithm.update_in_chunks(digest, body) - else - digest.update(body) - end - digest.base64digest - end - - def self.digest_for_algorithm(algorithm) - case algorithm - when 'CRC32' - Digest32.new(Zlib.method(:crc32)) - when 'CRC32C' - # this will only be used if input algorithm is CRC32C AND client supports it (crt available) - Digest32.new(Aws::Crt::Checksums.method(:crc32c)) - when 'SHA1' - Digest::SHA1.new - when 'SHA256' - Digest::SHA256.new - end - end - - # The trailer size (in bytes) is the overhead + the trailer name + - # the length of the base64 encoded checksum - def self.trailer_length(algorithm, location_name) - CHECKSUM_SIZE[algorithm] + location_name.size - end - - def self.update_in_chunks(digest, io) - loop do - chunk = io.read(CHUNK_SIZE) - break unless chunk - digest.update(chunk) - end - io.rewind end # Wrapper for request body that implements application-layer # chunking with Digest computed on chunks + added as a trailer class AwsChunkedTrailerDigestIO - CHUNK_SIZE = 16384 + CHUNK_SIZE = 16_384 def initialize(io, algorithm, location_name) @io = io diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/http_checksum.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/http_checksum.rb index 36134428bad..7eb33001f2a 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/http_checksum.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/http_checksum.rb @@ -12,7 +12,7 @@ class Handler < Seahorse::Client::Handler def call(context) if checksum_required?(context) && - !context[:checksum_algorithms] && # skip in favor of flexible checksum + !context[:http_checksum][:request_algorithm] && # skip in favor of flexible checksum !context[:s3_express_endpoint] # s3 express endpoints do not support md5 body = context.http_request.body context.http_request.headers['Content-Md5'] ||= md5(body) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/shared_config.rb b/gems/aws-sdk-core/lib/aws-sdk-core/shared_config.rb index 2dca19bb944..d1c67fad88e 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/shared_config.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/shared_config.rb @@ -211,6 +211,8 @@ def self.config_reader(*attrs) :retry_mode, :adaptive_retry_wait_to_fill, :correct_clock_skew, + :request_checksum_calculation, + :response_checksum_calculation, :csm_client_id, :csm_enabled, :csm_host, diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb index d93333d2f82..7074f4039df 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb @@ -6,174 +6,238 @@ module Aws module Plugins describe ChecksumAlgorithm do - let(:creds) { Aws::Credentials.new('akid', 'secret') } - let(:client_opts) { { credentials: creds, stub_responses: true } } - - # Don't include CRC32C in these definitions - increases complexity of testing - # with and without CRT - ChecksumClient = ApiHelper.sample_service( - metadata: { 'protocol' => 'rest-xml' }, - operations: { - 'SomeOperation' => { - 'http' => { 'method' => 'POST', 'requestUri' => '/' }, - 'httpChecksumRequired' => 'true', - 'input' => { 'shape' => 'SomeOperationRequest' }, - 'output' => { 'shape' => 'StructureShape' }, - 'httpChecksum' => { - "requestChecksumRequired" => true, - "requestAlgorithmMember" => "ChecksumAlgorithm", - "requestValidationModeMember" => "ChecksumMode", - "responseAlgorithms" => ["CRC32", "SHA256", "SHA1"] - } - }, - 'SomeOperationStreaming' => { - 'http' => { 'method' => 'POST', 'requestUri' => '/' }, - 'unsignedPayload' => true, - 'input' => { 'shape' => 'SomeOperationRequest' }, - 'output' => { 'shape' => 'StructureShape' }, - 'httpChecksum' => { - "requestChecksumRequired" => true, - "requestAlgorithmMember" => "ChecksumAlgorithm", - "requestValidationModeMember" => "ChecksumMode", - "responseAlgorithms" => ["CRC32", "SHA256", "SHA1"] + let(:checksum_client) do + ApiHelper.sample_service( + metadata: { 'protocol' => 'rest-xml' }, + operations: { + 'SomeOperation' => { + 'http' => { 'method' => 'POST', 'requestUri' => '/' }, + 'input' => { 'shape' => 'SomeInput' }, + 'output' => { 'shape' => 'SomeOutput' }, + 'httpChecksum' => { + 'requestChecksumRequired' => request_checksum_required, + 'requestAlgorithmMember' => 'ChecksumAlgorithm', + 'requestValidationModeMember' => 'ValidationMode', + 'responseAlgorithms' => response_algorithms + } + }, + 'SomeOperationStreaming' => { + 'http' => { 'method' => 'POST', 'requestUri' => '/' }, + 'unsignedPayload' => true, + 'input' => { 'shape' => 'SomeStreamingInput' }, + 'output' => { 'shape' => 'SomeStreamingOutput' }, + 'httpChecksum' => { + 'requestChecksumRequired' => request_checksum_required, + 'requestAlgorithmMember' => 'ChecksumAlgorithm', + 'requestValidationModeMember' => 'ValidationMode', + 'responseAlgorithms' => response_algorithms + } } }, - 'SomeOperationStreamingLegacyAuth' => { - 'http' => { 'method' => 'POST', 'requestUri' => '/' }, - 'authtype' => 'v4-unsigned-body', - 'input' => { 'shape' => 'SomeOperationRequest' }, - 'output' => { 'shape' => 'StructureShape' }, - 'httpChecksum' => { - "requestChecksumRequired" => true, - "requestAlgorithmMember" => "ChecksumAlgorithm", - "requestValidationModeMember" => "ChecksumMode", - "responseAlgorithms" => ["CRC32", "SHA256", "SHA1"] + shapes: { + 'Body' => { 'type' => 'blob' }, + 'ChecksumAlgorithm' => { + 'type' => 'string', + # SHA256 intentionally unmodeled for forwards compatibility test + 'enum' => ['CRC32', 'CRC32C', 'CRC64NVME', 'SHA1'] + }, + 'SomeInput' => { + 'type' => 'structure', + 'members' => { + 'ChecksumAlgorithm' => { + 'shape' => 'ChecksumAlgorithm', + 'location' => 'header', + 'locationName' => 'x-amz-request-algorithm' + }, + 'ValidationMode' => { 'shape' => 'ValidationMode' }, + 'Body' => { 'shape' => 'Body' } + }, + 'payload' => 'Body' + }, + 'SomeStreamingInput' => { + 'type' => 'structure', + 'members' => { + 'ChecksumAlgorithm' => { + 'shape' => 'ChecksumAlgorithm', + 'location' => 'header', + 'locationName' => 'x-amz-request-algorithm' + }, + 'ValidationMode' => { 'shape' => 'ValidationMode' }, + 'Body' => { 'shape' => 'Body', 'streaming' => true } + }, + 'payload' => 'Body' + }, + 'SomeOutput' => { + 'type' => 'structure', + 'members' => {} + }, + 'SomeStreamingOutput' => { + 'type' => 'structure', + 'members' => {} + }, + 'ValidationMode' => { + 'type' => 'string', + 'enum' => ['ENABLED'] } } - }, - shapes: { - 'StructureShape' => { - 'type' => 'structure', - 'members' => { - 'String' => { 'shape' => 'StringShape' } - } - }, - 'StringShape' => { 'type' => 'string' }, - 'ChecksumAlgorithmShape' => { - 'type' => 'string', - 'enum' => %w[CRC32 SHA1] - }, - 'ChecksumModeEnum' => { - 'type' => 'string', - 'enum' => %w[ENABLED] - }, - 'SomeOperationRequest' => { - 'type' => 'structure', - 'members' => { - 'ChecksumAlgorithm' => { 'shape' => 'ChecksumAlgorithmShape' }, - 'ChecksumMode' => { 'shape' => 'ChecksumModeEnum' }, - 'Body' => { 'shape' => 'Body', 'streaming' => true} - }, - 'payload' => 'Body' - }, - 'Body' => { 'type' => 'blob' } - } - ).const_get(:Client) - let(:client) { ChecksumClient.new(client_opts) } - - let(:default_algorithm_plugin) do - Class.new(Seahorse::Client::Plugin) do - class Handler < Seahorse::Client::Handler - def call(context) - context[:default_request_checksum_algorithm] = 'CRC32' - @handler.call(context) - end - end - handler(Handler) - end + ).const_get(:Client) end - context 'request algorithm selection' do - it 'uses crc32 in the header' do - resp = client.some_operation(checksum_algorithm: 'CRC32') - expect(resp.context.http_request.headers['x-amz-checksum-crc32']).to_not be_nil + let(:request_checksum_required) { true } + let(:response_algorithms) do + %w[CRC32 CRC32C CRC64NVME SHA1 SHA256] + end + + let(:client) { checksum_client.new(stub_responses: true) } + + describe 'request_checksum_calculation' do + it 'is configured to always compute by default' do + expect(client.config.request_checksum_calculation) + .to eq('WHEN_SUPPORTED') end - it 'uses crc32 in the trailer' do - resp = client.some_operation_streaming(checksum_algorithm: 'CRC32') - expect(resp.context.http_request.headers['x-amz-trailer']).to eq 'x-amz-checksum-crc32' + it 'can be configured using shared config' do + allow_any_instance_of(Aws::SharedConfig) + .to receive(:request_checksum_calculation) + .and_return('WHEN_REQUIRED') + expect(client.config.request_checksum_calculation) + .to eq('WHEN_REQUIRED') end - it 'uses crc32 in the trailer using legacy auth' do - resp = client.some_operation_streaming_legacy_auth(checksum_algorithm: 'CRC32') - expect(resp.context.http_request.headers['x-amz-trailer']).to eq 'x-amz-checksum-crc32' + it 'can be configured using ENV with precedence over shared config' do + allow_any_instance_of(Aws::SharedConfig) + .to receive(:request_checksum_calculation) + .and_return('WHEN_SUPPORTED') + ENV['AWS_REQUEST_CHECKSUM_CALCULATION'] = 'WHEN_REQUIRED' + expect(client.config.request_checksum_calculation) + .to eq('WHEN_REQUIRED') end - it 'uses un-modeled sha256 in the header' do - resp = client.some_operation(checksum_algorithm: 'sha256') - expect(resp.context.http_request.headers['x-amz-checksum-sha256']).to_not be_nil + it 'raises when request_checksum_calculation is not valid' do + ENV['AWS_REQUEST_CHECKSUM_CALCULATION'] = 'peccy' + expect { client }.to raise_error(ArgumentError, /WHEN_SUPPORTED/) end + end - it 'uses un-modeled sha256 in the trailer' do - resp = client.some_operation_streaming(checksum_algorithm: 'sha256') - expect(resp.context.http_request.headers['x-amz-trailer']).to eq 'x-amz-checksum-sha256' + describe 'response_checksum_calculation' do + it 'is configured to always verify by default' do + expect(client.config.response_checksum_calculation) + .to eq('WHEN_SUPPORTED') end - # Additional behavior tests + it 'can be configured using shared config' do + allow_any_instance_of(Aws::SharedConfig) + .to receive(:response_checksum_calculation) + .and_return('WHEN_REQUIRED') + expect(client.config.response_checksum_calculation) + .to eq('WHEN_REQUIRED') + end - it 'http_checksum_required; computes md5 when not configured' do - resp = client.some_operation - expect(resp.context.http_request.headers['Content-MD5']).to_not be_nil + it 'can be configured using ENV with precedence over shared config' do + allow_any_instance_of(Aws::SharedConfig) + .to receive(:response_checksum_calculation) + .and_return('WHEN_SUPPORTED') + ENV['AWS_RESPONSE_CHECKSUM_CALCULATION'] = 'WHEN_REQUIRED' + expect(client.config.response_checksum_calculation) + .to eq('WHEN_REQUIRED') end - it 'http_checksum_required; does not computes md5 when another checksum is computed' do - resp = client.some_operation(checksum_algorithm: 'sha256') - expect(resp.context.http_request.headers['Content-MD5']).to be_nil + it 'raises when response_checksum_calculation is not valid' do + ENV['AWS_RESPONSE_CHECKSUM_CALCULATION'] = 'peccy' + expect { client }.to raise_error(ArgumentError, /WHEN_SUPPORTED/) end + end + context 'request checksum calculation' do it 'raises when request algorithm is not supported by the client' do expect do client.some_operation(checksum_algorithm: 'no-such-algorithm') end.to raise_error(ArgumentError) end - it 'will use a default algorithm from the context' do - ChecksumClient.add_plugin(default_algorithm_plugin) + it 'will use a CRC32 as a default' do resp = client.some_operation - expect(resp.context.http_request.headers['x-amz-checksum-crc32']).to_not be_nil - ChecksumClient.remove_plugin(default_algorithm_plugin) + header = resp.context.http_request.headers['x-amz-checksum-crc32'] + expect(header).to eq('AAAAAA==') end - end - context 'response algorithm selection' do - let(:handler) { client.handlers.entries.find { |h| h.handler_class == Aws::Plugins::ChecksumAlgorithm::ChecksumHandler } } + file = File.expand_path('checksum_request.json', __dir__) + test_cases = JSON.load_file(file) - it 'skips response checksum when ChecksumMode is not set' do - expect(handler).not_to receive(:add_verify_response_checksum_handlers) - client.some_operation + test_cases.each do |test_case| + it "passes test: #{test_case['documentation']}" do + algorithm = test_case['checksumAlgorithm'].upcase + unless ChecksumAlgorithm::CLIENT_ALGORITHMS.include?(algorithm) + skip "Algorithm #{algorithm} not supported" + end + + resp = client.some_operation( + checksum_algorithm: algorithm, + body: test_case['requestPayload'] + ) + headers = resp.context.http_request.headers + test_case['expectHeaders'].each do |key, value| + expect(headers[key]).to eq(value) + end + end end + end + + context 'request streaming checksum calculation' do + file = File.expand_path('checksum_streaming_request.json', __dir__) + test_cases = JSON.load_file(file) - it 'computes validation_list when ChecksumMode is set' do - resp = client.some_operation(checksum_mode: 'ENABLED') - expect(resp.context[:http_checksum][:validation_list]).to eq %w[SHA1 CRC32 SHA256] + test_cases.each do |test_case| + it "passes test: #{test_case['documentation']}" do + algorithm = test_case['checksumAlgorithm'].upcase + unless ChecksumAlgorithm::CLIENT_ALGORITHMS.include?(algorithm) + skip "Algorithm #{algorithm} not supported" + end + + body = test_case['requestPayload'] + client.stub_responses(:some_operation_streaming, lambda do |context| + headers = context.http_request.headers + + expect(headers['x-amz-content-sha256']) + .to eq('STREAMING-UNSIGNED-PAYLOAD-TRAILER') + test_case['expectHeaders'].each do |key, value| + expect(headers[key]).to eq(value) + end + + # capture the body by reading it into a new IO object + new_body = StringIO.new + # IO.copy_stream is the same method used by Net::Http to write our body to the socket + IO.copy_stream(context.http_request.body, new_body) + new_body.rewind + + expect(headers['Content-Length'].to_i).to eq(new_body.size) + read_body = new_body.read + test_case['expectTrailers'].each do |key, value| + expect(read_body).to include("#{key}:#{value}") + end + context + end) + + client.some_operation_streaming( + checksum_algorithm: algorithm, + body: body + ) + end end + end - it 'validation_list does not include unsupported algorithms' do - expect(ChecksumAlgorithm).to receive(:operation_response_algorithms).and_return(%w[CRC64 CRC32]) - resp = client.some_operation(checksum_mode: 'ENABLED') - expect(resp.context[:http_checksum][:validation_list]).to eq %w[CRC32] + context 'response checksum calculation' do + it 'computes a validation_list' do + resp = client.some_operation + expect(resp.context[:http_checksum][:validation_list]) + .to include(*%w[SHA1 CRC32 SHA256]) end - it 'validates a matched header' do - client.stub_responses( - :some_operation, - [{ - body: '', - headers: {'x-amz-checksum-sha256' => '47DEQpj8HBSa+/TImW+5JCeuQeRkm5NMpJWZG3hSuFU='}, - status_code: 200 - }]) - resp = client.some_operation(checksum_mode: 'ENABLED') - expect(resp.context[:http_checksum][:validated]).to eq 'SHA256' + it 'validation_list does not include unknown algorithms' do + expect_any_instance_of(ChecksumAlgorithm::ChecksumHandler) + .to receive(:operation_response_algorithms).and_return(%w[UNKNOWN CRC32]) + resp = client.some_operation + expect(resp.context[:http_checksum][:validation_list]).to eq %w[CRC32] end it 'validates the first matched header by priority' do @@ -187,54 +251,135 @@ def call(context) }, status_code: 200 }]) - resp = client.some_operation(checksum_mode: 'ENABLED') + resp = client.some_operation expect(resp.context[:http_checksum][:validated]).to eq 'CRC32' end - it 'does not validate an unmodeled header' do + it 'does not validate unknown checksums' do client.stub_responses( :some_operation, [{ body: '', - headers: {'x-amz-checksum-crc64' => 'crc64_value'}, + headers: {'x-amz-checksum-unknown' => 'unknown'}, status_code: 200 }]) - resp = client.some_operation(checksum_mode: 'ENABLED') + resp = client.some_operation expect(resp.context[:http_checksum][:validated]).to be_nil end + + file = File.expand_path('checksum_response.json', __dir__) + test_cases = JSON.load_file(file) + + test_cases.each do |test_case| + it "passes test: #{test_case['documentation']}" do + algorithm = test_case['checksumAlgorithm'].upcase + unless ChecksumAlgorithm::CLIENT_ALGORITHMS.include?(algorithm) + skip "Algorithm #{algorithm} not supported" + end + + expect = test_case['expect'] + client.stub_responses( + :some_operation, + [{ + body: test_case['responsePayload'], + headers: test_case['responseHeaders'], + status_code: 200 + }] + ) + case expect['kind'] + when 'success' + client.some_operation(validation_mode: 'ENABLED') + when 'failure' + expect do + client.some_operation(validation_mode: 'ENABLED') + end.to raise_error(Aws::Errors::ChecksumError, /#{expect['responseHeaders']}/) + else + raise 'Unsupported test kind' + end + end + end end - context 'PUT requests' do + context 'when checksums are not required' do + let(:request_checksum_required) { false } - it 'handles unsigned-payload auth with checksum in trailer' do - client.stub_responses(:some_operation_streaming, -> (context) do - headers = context.http_request.headers + it 'WHEN_SUPPORTED; no algorithm; includes a checksum' do + resp = client.some_operation(checksum_algorithm: 'CRC32') + expect(resp.context.http_request.headers['x-amz-checksum-crc32']) + .to eq('AAAAAA==') + end - expect(headers['x-amz-content-sha256']).to eq('STREAMING-UNSIGNED-PAYLOAD-TRAILER') - expect(headers['x-amz-trailer']).to eq('x-amz-checksum-sha256') - expect(headers['content-encoding']).to eq('aws-chunked') - expect(headers['x-amz-decoded-content-length']).to eq('11') - # capture the body by reading it into a new IO object - body = StringIO.new - # IO.copy_stream is the same method used by Net::Http to write our body to the socket - IO.copy_stream(context.http_request.body, body) - body.rewind - expect(body.read).to eq "b\r\nHello World\r\n0\r\nx-amz-checksum-sha256:pZGm1Av0IEBKARczz7exkNYsZb8LzaMrV7J32a2fFG4=\r\n\r\n" - end) - client.some_operation_streaming(checksum_algorithm: 'sha256', body: 'Hello World') + it 'WHEN_REQUIRED; no algorithm; does not include a checksum' do + client = checksum_client.new( + stub_responses: true, + request_checksum_calculation: 'WHEN_REQUIRED' + ) + resp = client.some_operation + expect(resp.context.http_request.headers['x-amz-checksum-crc32']) + .to be_nil + end + end + + context 'when checksums are required' do + let(:request_checksum_required) { true } + + it 'WHEN_SUPPORTED; no algorithm; includes a checksum' do + resp = client.some_operation + expect(resp.context.http_request.headers['x-amz-checksum-crc32']) + .to eq('AAAAAA==') + end + + it 'WHEN_REQUIRED; no algorithm; includes a checksum' do + client = checksum_client.new( + stub_responses: true, + request_checksum_calculation: 'WHEN_REQUIRED' + ) + resp = client.some_operation + expect(resp.context.http_request.headers['x-amz-checksum-crc32']) + .to eq('AAAAAA==') + end + end + + context 'when a response may be validated' do + let(:response_algorithms) { %w[CRC32] } + def stub_client(client) + client.stub_responses( + :some_operation, + [{ + body: '', + headers: { 'x-amz-checksum-crc32' => 'AAAAAA==' }, + status_code: 200 + }] + ) end - it 'handles header-based auth with checksum in header' do - client.stub_responses(:some_operation, -> (context) do - expect(context.http_request.headers['x-amz-checksum-sha256']).to eq('pZGm1Av0IEBKARczz7exkNYsZb8LzaMrV7J32a2fFG4=') - expect(context.http_request.body.read).to eq('Hello World') - end) - client.some_operation(checksum_algorithm: 'sha256', body: 'Hello World') + it 'WHEN_SUPPORTED; not ENABLED; validates the checksum' do + stub_client(client) + resp = client.some_operation + expect(resp.context[:http_checksum][:validated]).to eq('CRC32') + # This needs to be set by the plugin in this case + expect(resp.context.params[:validation_mode]).to eq('ENABLED') + end + + it 'WHEN_REQUIRED; not ENABLED; does not validate the checksum' do + client = checksum_client.new( + stub_responses: true, + response_checksum_calculation: 'WHEN_REQUIRED' + ) + stub_client(client) + resp = client.some_operation + expect(resp.context[:http_checksum][:validated]).to be_nil end - it 'handles sigv4-streaming auth with checksum in trailer' do - skip("sigv4 streaming is not supported") + it 'WHEN_REQUIRED; ENABLED; validates the checksum' do + client = checksum_client.new( + stub_responses: true, + response_checksum_calculation: 'WHEN_REQUIRED' + ) + stub_client(client) + resp = client.some_operation(validation_mode: 'ENABLED') + expect(resp.context[:http_checksum][:validated]).to eq('CRC32') end end end diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json new file mode 100644 index 00000000000..75ac0df6344 --- /dev/null +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json @@ -0,0 +1,47 @@ +[ + { + "documentation": "CRC32 checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc32", + "expectHeaders": { + "x-amz-request-algorithm": "CRC32", + "x-amz-checksum-crc32": "i9aeUg==" + } + }, + { + "documentation": "CRC32C checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc32C", + "expectHeaders": { + "x-amz-request-algorithm": "CRC32C", + "x-amz-checksum-crc32c": "crUfeA==" + } + }, + { + "documentation": "CRC64NVME checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc64Nvme", + "expectHeaders": { + "x-amz-request-algorithm": "CRC64NVME", + "x-amz-checksum-crc64nvme": "uc8X9yrZrD4=" + } + }, + { + "documentation": "SHA1 checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Sha1", + "expectHeaders": { + "x-amz-request-algorithm": "SHA1", + "x-amz-checksum-sha1": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" + } + }, + { + "documentation": "SHA256 checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Sha256", + "expectHeaders": { + "x-amz-request-algorithm": "SHA256", + "x-amz-checksum-sha256": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" + } + } +] \ No newline at end of file diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_response.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_response.json new file mode 100644 index 00000000000..27e896ec3ac --- /dev/null +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_response.json @@ -0,0 +1,107 @@ +[ + { + "documentation": "Successful payload validation with CRC32 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc32", + "responseHeaders": { + "x-amz-checksum-crc32": "i9aeUg==" + }, + "expect": { "kind": "success" } + }, + { + "documentation": "Successful payload validation with CRC32C checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc32C", + "responseHeaders": { + "x-amz-checksum-crc32c": "crUfeA==" + }, + "expect": { "kind": "success" } + }, + { + "documentation": "Successful payload validation with CRC64NVME checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc64Nvme", + "responseHeaders": { + "x-amz-checksum-crc64nvme": "uc8X9yrZrD4=" + }, + "expect": { "kind": "success" } + }, + { + "documentation": "Successful payload validation with SHA1 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Sha1", + "responseHeaders": { + "x-amz-checksum-sha1": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" + }, + "expect": { "kind": "success" } + }, + { + "documentation": "Successful payload validation with SHA256 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Sha256", + "responseHeaders": { + "x-amz-checksum-sha256": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" + }, + "expect": { "kind": "success" } + }, + { + "documentation": "Failed payload validation with CRC32 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc32", + "responseHeaders": { + "x-amz-checksum-crc32": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "i9aeUg==" + } + }, + { + "documentation": "Failed payload validation with CRC32C checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc32C", + "responseHeaders": { + "x-amz-checksum-crc32c": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "crUfeA==" + } + }, + { + "documentation": "Failed payload validation with CRC64NVME checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc64Nvme", + "responseHeaders": { + "x-amz-checksum-crc64nvme": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "uc8X9yrZrD4=" + } + }, + { + "documentation": "Failed payload validation with SHA1 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Sha1", + "responseHeaders": { + "x-amz-checksum-sha1": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" + } + }, + { + "documentation": "Failed payload validation with SHA256 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Sha256", + "responseHeaders": { + "x-amz-checksum-sha256": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" + } + } + ] \ No newline at end of file diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json new file mode 100644 index 00000000000..5c9ce933618 --- /dev/null +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json @@ -0,0 +1,62 @@ +[ + { + "documentation": "CRC32 streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc32", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-crc32" + }, + "expectTrailers": { + "x-amz-checksum-crc32": "i9aeUg==" + } + }, + { + "documentation": "CRC32C streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc32C", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-crc32c" + }, + "expectTrailers": { + "x-amz-checksum-crc32c": "crUfeA==" + } + }, + { + "documentation": "CRC64NVME streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc64Nvme", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-crc64nvme" + }, + "expectTrailers": { + "x-amz-checksum-crc64nvme": "uc8X9yrZrD4=" + } + }, + { + "documentation": "SHA1 streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Sha1", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-sha1" + }, + "expectTrailers": { + "x-amz-checksum-sha1": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" + } + }, + { + "documentation": "SHA256 streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Sha256", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-sha256" + }, + "expectTrailers": { + "x-amz-checksum-sha256": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" + } + } + ] \ No newline at end of file diff --git a/gems/aws-sdk-core/spec/aws/plugins/http_checksum_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/http_checksum_spec.rb index 458cd523a6e..ef9b71cfbc1 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/http_checksum_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/http_checksum_spec.rb @@ -43,7 +43,14 @@ module Plugins HttpChecksumClient.new(credentials: creds, stub_responses: true) end - context 'checksum required operations' do + context 'checksum not required' do + it 'does not compute MD5 and does not send the content-md5 header' do + resp = client.operation(string: 'i am just a string') + expect(resp.context.http_request.headers['content-md5']).to be_nil + end + end + + context 'httpChecksumRequired operations' do it 'computes MD5 of the http body and sends as content-md5 header' do resp = client.checksum_operation(string: 'md5 me captain') expect(resp.context.http_request.headers['content-md5']).to eq( @@ -62,15 +69,8 @@ module Plugins end end - context 'checksum not required operations' do - it 'does not compute MD5 and does not send the content-md5 header' do - resp = client.operation(string: 'i am just a string') - expect(resp.context.http_request.headers['content-md5']).to be_nil - end - end - context 'httpChecksum operation' do - it 'computes the MD5 when another checksum has not been computed' do + it 'computes the MD5 when another checksum has not been selected' do resp = client.checksum_algorithm_operation(string: 'md5 me captain') expect(resp.context.http_request.headers['content-md5']).to eq( 'rqd/0N8H2GgZWzmo3oY9tA==' diff --git a/gems/aws-sdk-s3/CHANGELOG.md b/gems/aws-sdk-s3/CHANGELOG.md index 85acf38af87..74ed07792d6 100644 --- a/gems/aws-sdk-s3/CHANGELOG.md +++ b/gems/aws-sdk-s3/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Feature - Default to using `CRC32` checksum validation for S3 uploads and downloads. + 1.157.0 (2024-08-01) ------------------ diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb index adbd5a936be..4ca8e1984be 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb @@ -159,14 +159,15 @@ def upload_in_threads(pending, completed, options) end resp = @client.upload_part(part) part[:body].close - completed_part = {etag: resp.etag, part_number: part[:part_number]} - - # get the requested checksum from the response - if part[:checksum_algorithm] - k = "checksum_#{part[:checksum_algorithm].downcase}".to_sym - completed_part[k] = resp[k] + completed_part = { + etag: resp.etag, + part_number: part[:part_number] + }.tap do |h| + # get the requested checksum from the response + algorithm = resp.context.params[:checksum_algorithm] + k = "checksum_#{algorithm.downcase}".to_sym + h[k] = resp.send(k) end - completed.push(completed_part) end nil diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/express_session_auth.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/express_session_auth.rb index 54ae89d36ef..ce23f18021a 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/express_session_auth.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/express_session_auth.rb @@ -29,24 +29,17 @@ class ExpressSessionAuth < Seahorse::Client::Plugin # @api private class Handler < Seahorse::Client::Handler def call(context) - if (props = context[:endpoint_properties]) - # S3 Express endpoint - turn off md5 and enable crc32 default - if props['backend'] == 'S3Express' - if context.operation_name == :put_object || checksum_required?(context) - context[:default_request_checksum_algorithm] = 'CRC32' - end - context[:s3_express_endpoint] = true - end + context[:s3_express_endpoint] = true if s3_express_endpoint?(context) - # if s3 express auth, use new credentials and sign additional header - if context[:auth_scheme]['name'] == 'sigv4-s3express' && - !context.config.disable_s3_express_session_auth - bucket = context.params[:bucket] - credentials_provider = context.config.express_credentials_provider - credentials = credentials_provider.express_credentials_for(bucket) - context[:sigv4_credentials] = credentials # Sign will use this - end + # if s3 express auth, use new credentials and sign additional header + if context[:auth_scheme]['name'] == 'sigv4-s3express' && + !context.config.disable_s3_express_session_auth + bucket = context.params[:bucket] + credentials_provider = context.config.express_credentials_provider + credentials = credentials_provider.express_credentials_for(bucket) + context[:sigv4_credentials] = credentials # Sign will use this end + with_metric(credentials) { @handler.call(context) } end @@ -58,10 +51,8 @@ def with_metric(credentials, &block) Aws::Plugins::UserAgent.metric('S3_EXPRESS_BUCKET', &block) end - def checksum_required?(context) - context.operation.http_checksum_required || - (context.operation.http_checksum && - context.operation.http_checksum['requestChecksumRequired']) + def s3_express_endpoint?(context) + context[:endpoint_properties]['backend'] == 'S3Express' end end diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb index b88e4e917da..cec90b7252c 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb @@ -6,81 +6,26 @@ module Aws module S3 module Plugins # @api private - # This plugin is effectively deprecated in favor of modeled + # This plugin is deprecated in favor of modeled # httpChecksumRequired traits. class Md5s < Seahorse::Client::Plugin - # These operations allow Content MD5 but are not required by - # httpChecksumRequired. This list should not grow. - OPTIONAL_OPERATIONS = [ - :put_object, - :upload_part - ] - - # @api private - class Handler < Seahorse::Client::Handler - - CHUNK_SIZE = 1 * 1024 * 1024 # one MB - - def call(context) - if !context[:checksum_algorithms] && # skip in favor of flexible checksum - !context[:s3_express_endpoint] # s3 express endpoints do not support md5 - body = context.http_request.body - if body.respond_to?(:size) && body.size > 0 - context.http_request.headers['Content-Md5'] ||= md5(body) - end - end - @handler.call(context) - end - - private - - # @param [File, Tempfile, IO#read, String] value - # @return [String] - def md5(value) - if (File === value || Tempfile === value) && !value.path.nil? && File.exist?(value.path) - OpenSSL::Digest::MD5.file(value).base64digest - elsif value.respond_to?(:read) - md5 = OpenSSL::Digest::MD5.new - update_in_chunks(md5, value) - md5.base64digest + option(:compute_checksums, + default: true, + doc_type: 'Boolean', + docstring: <<~DOCS) + This option is deprecated. Please use `:request_checksum_calculation` + instead. When `true`, `request_checksum_calculation` is set to `WHEN_SUPPORTED`, + and if `false` it is set to `WHEN_REQUIRED`. + DOCS + + def after_initialize(client) + client.config.request_checksum_calculation = + if client.config.compute_checksums + 'WHEN_SUPPORTED' else - OpenSSL::Digest::MD5.digest(value).base64digest + 'WHEN_REQUIRED' end - end - - def update_in_chunks(digest, io) - loop do - chunk = io.read(CHUNK_SIZE) - break unless chunk - digest.update(chunk) - end - io.rewind - end - end - - option(:compute_checksums, - default: true, - doc_type: 'Boolean', - docstring: <<-DOCS) -When `true` a MD5 checksum will be computed and sent in the Content Md5 -header for :put_object and :upload_part. When `false`, MD5 checksums -will not be computed for these operations. Checksums are still computed -for operations requiring them. Checksum errors returned by Amazon S3 are -automatically retried up to `:retry_limit` times. - DOCS - - def add_handlers(handlers, config) - if config.compute_checksums - # priority set low to ensure md5 is computed AFTER the request is - # built but before it is signed - handlers.add( - Handler, - priority: 10, step: :build, operations: OPTIONAL_OPERATIONS - ) - end - end - end end end diff --git a/gems/aws-sdk-s3/spec/encryption/client_spec.rb b/gems/aws-sdk-s3/spec/encryption/client_spec.rb index 6888f0fc921..f1662582b08 100644 --- a/gems/aws-sdk-s3/spec/encryption/client_spec.rb +++ b/gems/aws-sdk-s3/spec/encryption/client_spec.rb @@ -149,7 +149,6 @@ module Encryption body: encrypted_body, headers: { 'Content-Length' => '16', - 'Content-Md5' => 'l0B7VfMeJ/9UqZlxWo2uEw==', # key is encrypted here with the master encryption key, # then base64 encoded 'X-Amz-Meta-X-Amz-Key' => 'gX+a4JQYj7FP0y5TAAvxTz4e'\ diff --git a/gems/aws-sdk-s3/spec/plugins/md5s_spec.rb b/gems/aws-sdk-s3/spec/plugins/md5s_spec.rb deleted file mode 100644 index 7a1981ce77c..00000000000 --- a/gems/aws-sdk-s3/spec/plugins/md5s_spec.rb +++ /dev/null @@ -1,158 +0,0 @@ -# frozen_string_literal: true - -require_relative '../spec_helper' -require 'stringio' - -module Aws - module S3 - module Plugins - describe Md5s do - it 'has a :compute_checksums option that defaults to true' do - client = Client.new(stub_responses: true) - expect(client.config.compute_checksums).to be(true) - end - - it 'can disabled' do - client = Client.new(stub_responses: true, compute_checksums: false) - expect(client.config.compute_checksums).to be(false) - end - - it 'retries invalid content MD5s', rbs_test: :skip do - stub_request( - :put, - 'https://test.s3.us-west-2.amazonaws.com/test' - ).to_return( - status: 400, - headers: {}, - body: <<-XML - - -BadDigest -The Content-MD5 you specified did not match what we received. -123 -456== -' -XML - ) - - s3 = Client.new( - region: 'us-west-2', - credentials: Aws::Credentials.new('akid', 'secret'), - raise_response_errors: false, - retry_backoff: proc {}, - retry_limit: 5 - ) - - resp = s3.put_object(bucket: 'test', key: 'test', content_md5: '456==') - expect(resp.error).to be_a(Aws::S3::Errors::BadDigest) - expect(resp.error.context.retries).to eq(5) - end - - describe '#put_object' do - it 'computes MD5 of the body and sends it with content-md5 header' do - client = Client.new(stub_responses: true) - resp = client.put_object( - bucket: 'bucket-name', - key: 'object-key', - body: 'Hello World!' - ) - expect(resp.context.http_request.headers['content-md5']).to eq( - '7Qdih1MuhjZehB6Sv8UNjA==' - ) - end - - it 'does not compute the MD5 when :compute_checksums is false' do - client = Client.new(stub_responses: true, compute_checksums: false) - resp = client.put_object( - bucket: 'bucket-name', - key: 'object-key', - body: 'Hello World!' - ) - expect(resp.context.http_request.headers['content-md5']).to be(nil) - end - - it 'skips the md5 when the body is empty' do - client = Client.new(stub_responses: true) - resp = client.put_object( - bucket: 'bucket-name', - key: 'object-key', - body: '' - ) - expect(resp.context.http_request.headers['content-md5']).to be(nil) - end - - it 'computes the MD5 by reading the body 1MB at a time' do - body = StringIO.new('.' * 5 * 1024 * 1024) # 5MB - allow(body).to receive(:read) - .with(1024 * 1024, any_args).and_call_original - client = Client.new(stub_responses: true) - resp = client.put_object( - bucket: 'bucket-name', - key: 'object-key', - body: body # an io-like object - ) - expect(resp.context.http_request.headers['content-md5']).to eq( - '+kDD2/74SZx+Rz+/Dw7I1Q==' - ) - end - end - - describe '#upload_part' do - it 'computes MD5 of the body and sends it with content-md5 header' do - client = Client.new(stub_responses: true) - resp = client.upload_part( - bucket: 'bucket-name', - key: 'object-key', - upload_id: 'id', - part_number: 1, - body: 'Hello World!' - ) - expect(resp.context.http_request.headers['content-md5']).to eq( - '7Qdih1MuhjZehB6Sv8UNjA==' - ) - end - - it 'does not compute the MD5 when :compute_checksums is false' do - client = Client.new(stub_responses: true, compute_checksums: false) - resp = client.upload_part( - bucket: 'bucket-name', - key: 'object-key', - upload_id: 'id', - part_number: 1, - body: 'Hello World!' - ) - expect(resp.context.http_request.headers['content-md5']).to be(nil) - end - - it 'skips the md5 when the body is empty' do - client = Client.new(stub_responses: true) - resp = client.upload_part( - bucket: 'bucket-name', - key: 'object-key', - upload_id: 'id', - part_number: 1 - ) - expect(resp.context.http_request.headers['content-md5']).to be(nil) - end - - it 'computes the MD5 by reading the body 1MB at a time' do - body = StringIO.new('.' * 5 * 1024 * 1024) # 5MB - allow(body).to receive(:read) - .with(1024 * 1024, any_args).and_call_original - client = Client.new(stub_responses: true) - resp = client.upload_part( - bucket: 'bucket-name', - key: 'object-key', - upload_id: 'id', - part_number: 1, - body: body # an io-like object - ) - expect(resp.context.http_request.headers['content-md5']).to eq( - '+kDD2/74SZx+Rz+/Dw7I1Q==' - ) - end - end - end - end - end -end From 64539c67c38ddbbb47353a5040f1e9ca3bee9f0a Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Tue, 20 Aug 2024 16:42:31 -0400 Subject: [PATCH 02/18] Fix test cases --- .../plugins/checksum_algorithm.rb | 7 +- .../aws/plugins/checksum_algorithm_spec.rb | 64 +++++++++++-------- .../spec/aws/plugins/http_checksum_spec.rb | 63 +++++++----------- .../lib/aws-sdk-s3/multipart_file_uploader.rb | 3 +- gems/aws-sdk-s3/lib/aws-sdk-s3/presigner.rb | 9 ++- .../spec/object/upload_file_spec.rb | 56 ++++++++-------- 6 files changed, 103 insertions(+), 99 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index 5bf6bd9eddc..a9406badefe 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -28,10 +28,13 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin CHECKSUM_SIZE = { 'CRC32' => 16, 'CRC32C' => 16, + 'CRC64NVME' => 32, 'SHA1' => 36, 'SHA256' => 52 }.freeze + DEFAULT_CHECKSUM = 'CRC32' + option(:request_checksum_calculation, doc_default: 'WHEN_SUPPORTED', doc_type: 'String', @@ -163,7 +166,7 @@ def call(context) default_request_algorithm_member(context) # Not modeled with httpChecksum if context.operation_name == :create_multipart_upload - context.params[:checksum_algorithm] ||= 'CRC32' + context.params[:checksum_algorithm] ||= DEFAULT_CHECKSUM end @handler.call(context) @@ -253,7 +256,7 @@ def should_calculate_request_checksum?(context) end def choose_request_algorithm!(context) - algorithm = request_algorithm_selection(context) + algorithm = request_algorithm_selection(context) || DEFAULT_CHECKSUM return algorithm if CLIENT_ALGORITHMS.include?(algorithm) if algorithm == 'CRC32C' diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb index 7074f4039df..3eccb833f43 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb @@ -10,7 +10,7 @@ module Plugins ApiHelper.sample_service( metadata: { 'protocol' => 'rest-xml' }, operations: { - 'SomeOperation' => { + 'HttpChecksumOperation' => { 'http' => { 'method' => 'POST', 'requestUri' => '/' }, 'input' => { 'shape' => 'SomeInput' }, 'output' => { 'shape' => 'SomeOutput' }, @@ -21,7 +21,7 @@ module Plugins 'responseAlgorithms' => response_algorithms } }, - 'SomeOperationStreaming' => { + 'HttpChecksumStreamingOperation' => { 'http' => { 'method' => 'POST', 'requestUri' => '/' }, 'unsignedPayload' => true, 'input' => { 'shape' => 'SomeStreamingInput' }, @@ -32,6 +32,14 @@ module Plugins 'requestValidationModeMember' => 'ValidationMode', 'responseAlgorithms' => response_algorithms } + }, + 'ChecksumRequiredOperation' => { + 'http' => { 'method' => 'POST', 'requestUri' => '/' }, + 'input' => { 'shape' => 'SomeInput' }, + 'output' => { 'shape' => 'SomeOutput' }, + 'httpChecksum' => { + 'requestChecksumRequired' => request_checksum_required, + } } }, shapes: { @@ -151,16 +159,22 @@ module Plugins context 'request checksum calculation' do it 'raises when request algorithm is not supported by the client' do expect do - client.some_operation(checksum_algorithm: 'no-such-algorithm') + client.http_checksum_operation(checksum_algorithm: 'no-such-algorithm') end.to raise_error(ArgumentError) end - it 'will use a CRC32 as a default' do - resp = client.some_operation + it 'with requestAlgorithmMember; will use a CRC32 as a default' do + resp = client.http_checksum_operation header = resp.context.http_request.headers['x-amz-checksum-crc32'] expect(header).to eq('AAAAAA==') end + it 'without requestAlgorithmMember; will use a CRC32 as a default' do + resp = client.checksum_required_operation(body: 'crc32 me captain') + header = resp.context.http_request.headers['x-amz-checksum-crc32'] + expect(header).to eq('nqtcGg==') + end + file = File.expand_path('checksum_request.json', __dir__) test_cases = JSON.load_file(file) @@ -171,7 +185,7 @@ module Plugins skip "Algorithm #{algorithm} not supported" end - resp = client.some_operation( + resp = client.http_checksum_operation( checksum_algorithm: algorithm, body: test_case['requestPayload'] ) @@ -195,7 +209,7 @@ module Plugins end body = test_case['requestPayload'] - client.stub_responses(:some_operation_streaming, lambda do |context| + client.stub_responses(:http_checksum_streaming_operation, lambda do |context| headers = context.http_request.headers expect(headers['x-amz-content-sha256']) @@ -218,7 +232,7 @@ module Plugins context end) - client.some_operation_streaming( + client.http_checksum_streaming_operation( checksum_algorithm: algorithm, body: body ) @@ -228,7 +242,7 @@ module Plugins context 'response checksum calculation' do it 'computes a validation_list' do - resp = client.some_operation + resp = client.http_checksum_operation expect(resp.context[:http_checksum][:validation_list]) .to include(*%w[SHA1 CRC32 SHA256]) end @@ -236,13 +250,13 @@ module Plugins it 'validation_list does not include unknown algorithms' do expect_any_instance_of(ChecksumAlgorithm::ChecksumHandler) .to receive(:operation_response_algorithms).and_return(%w[UNKNOWN CRC32]) - resp = client.some_operation + resp = client.http_checksum_operation expect(resp.context[:http_checksum][:validation_list]).to eq %w[CRC32] end it 'validates the first matched header by priority' do client.stub_responses( - :some_operation, + :http_checksum_operation, [{ body: '', headers: { @@ -251,19 +265,19 @@ module Plugins }, status_code: 200 }]) - resp = client.some_operation + resp = client.http_checksum_operation expect(resp.context[:http_checksum][:validated]).to eq 'CRC32' end it 'does not validate unknown checksums' do client.stub_responses( - :some_operation, + :http_checksum_operation, [{ body: '', headers: {'x-amz-checksum-unknown' => 'unknown'}, status_code: 200 }]) - resp = client.some_operation + resp = client.http_checksum_operation expect(resp.context[:http_checksum][:validated]).to be_nil end @@ -279,7 +293,7 @@ module Plugins expect = test_case['expect'] client.stub_responses( - :some_operation, + :http_checksum_operation, [{ body: test_case['responsePayload'], headers: test_case['responseHeaders'], @@ -288,10 +302,10 @@ module Plugins ) case expect['kind'] when 'success' - client.some_operation(validation_mode: 'ENABLED') + client.http_checksum_operation(validation_mode: 'ENABLED') when 'failure' expect do - client.some_operation(validation_mode: 'ENABLED') + client.http_checksum_operation(validation_mode: 'ENABLED') end.to raise_error(Aws::Errors::ChecksumError, /#{expect['responseHeaders']}/) else raise 'Unsupported test kind' @@ -304,7 +318,7 @@ module Plugins let(:request_checksum_required) { false } it 'WHEN_SUPPORTED; no algorithm; includes a checksum' do - resp = client.some_operation(checksum_algorithm: 'CRC32') + resp = client.http_checksum_operation(checksum_algorithm: 'CRC32') expect(resp.context.http_request.headers['x-amz-checksum-crc32']) .to eq('AAAAAA==') end @@ -314,7 +328,7 @@ module Plugins stub_responses: true, request_checksum_calculation: 'WHEN_REQUIRED' ) - resp = client.some_operation + resp = client.http_checksum_operation expect(resp.context.http_request.headers['x-amz-checksum-crc32']) .to be_nil end @@ -324,7 +338,7 @@ module Plugins let(:request_checksum_required) { true } it 'WHEN_SUPPORTED; no algorithm; includes a checksum' do - resp = client.some_operation + resp = client.http_checksum_operation expect(resp.context.http_request.headers['x-amz-checksum-crc32']) .to eq('AAAAAA==') end @@ -334,7 +348,7 @@ module Plugins stub_responses: true, request_checksum_calculation: 'WHEN_REQUIRED' ) - resp = client.some_operation + resp = client.http_checksum_operation expect(resp.context.http_request.headers['x-amz-checksum-crc32']) .to eq('AAAAAA==') end @@ -345,7 +359,7 @@ module Plugins def stub_client(client) client.stub_responses( - :some_operation, + :http_checksum_operation, [{ body: '', headers: { 'x-amz-checksum-crc32' => 'AAAAAA==' }, @@ -356,7 +370,7 @@ def stub_client(client) it 'WHEN_SUPPORTED; not ENABLED; validates the checksum' do stub_client(client) - resp = client.some_operation + resp = client.http_checksum_operation expect(resp.context[:http_checksum][:validated]).to eq('CRC32') # This needs to be set by the plugin in this case expect(resp.context.params[:validation_mode]).to eq('ENABLED') @@ -368,7 +382,7 @@ def stub_client(client) response_checksum_calculation: 'WHEN_REQUIRED' ) stub_client(client) - resp = client.some_operation + resp = client.http_checksum_operation expect(resp.context[:http_checksum][:validated]).to be_nil end @@ -378,7 +392,7 @@ def stub_client(client) response_checksum_calculation: 'WHEN_REQUIRED' ) stub_client(client) - resp = client.some_operation(validation_mode: 'ENABLED') + resp = client.http_checksum_operation(validation_mode: 'ENABLED') expect(resp.context[:http_checksum][:validated]).to eq('CRC32') end end diff --git a/gems/aws-sdk-core/spec/aws/plugins/http_checksum_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/http_checksum_spec.rb index ef9b71cfbc1..227337077fb 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/http_checksum_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/http_checksum_spec.rb @@ -6,41 +6,33 @@ module Aws module Plugins describe HttpChecksum do - HttpChecksumClient = ApiHelper.sample_service( - metadata: { 'protocol' => 'rest-xml' }, - operations: { - 'Operation' => { - 'http' => { 'method' => 'POST', 'requestUri' => '/' }, - 'input' => { 'shape' => 'StructureShape' }, - 'output' => { 'shape' => 'StructureShape' } - }, - 'ChecksumOperation' => { - 'http' => { 'method' => 'POST', 'requestUri' => '/' }, - 'input' => { 'shape' => 'StructureShape' }, - 'output' => { 'shape' => 'StructureShape' }, - 'httpChecksumRequired' => { 'required' => 'true' } - }, - 'ChecksumStreamingOperation' => { - 'http' => { 'method' => 'POST', 'requestUri' => '/' }, - 'input' => { 'shape' => 'PayloadStructureShape' }, - 'output' => { 'shape' => 'PayloadStructureShape' }, - 'httpChecksumRequired' => { 'required' => 'true' } - }, - 'ChecksumAlgorithmOperation' => { - 'http' => { 'method' => 'POST', 'requestUri' => '/' }, - 'input' => { 'shape' => 'StructureShape' }, - 'output' => { 'shape' => 'StructureShape' }, - 'httpChecksum' => { - "requestChecksumRequired" => true, + let(:http_checksum_client) do + ApiHelper.sample_service( + metadata: { 'protocol' => 'rest-xml' }, + operations: { + 'Operation' => { + 'http' => { 'method' => 'POST', 'requestUri' => '/' }, + 'input' => { 'shape' => 'StructureShape' }, + 'output' => { 'shape' => 'StructureShape' } + }, + 'ChecksumOperation' => { + 'http' => { 'method' => 'POST', 'requestUri' => '/' }, + 'input' => { 'shape' => 'StructureShape' }, + 'output' => { 'shape' => 'StructureShape' }, + 'httpChecksumRequired' => { 'required' => 'true' } + }, + 'ChecksumStreamingOperation' => { + 'http' => { 'method' => 'POST', 'requestUri' => '/' }, + 'input' => { 'shape' => 'PayloadStructureShape' }, + 'output' => { 'shape' => 'PayloadStructureShape' }, + 'httpChecksumRequired' => { 'required' => 'true' } } } - } - ).const_get(:Client) - - let(:creds) { Aws::Credentials.new('akid', 'secret') } + ).const_get(:Client) + end let(:client) do - HttpChecksumClient.new(credentials: creds, stub_responses: true) + http_checksum_client.new(stub_responses: true) end context 'checksum not required' do @@ -68,15 +60,6 @@ module Plugins ) end end - - context 'httpChecksum operation' do - it 'computes the MD5 when another checksum has not been selected' do - resp = client.checksum_algorithm_operation(string: 'md5 me captain') - expect(resp.context.http_request.headers['content-md5']).to eq( - 'rqd/0N8H2GgZWzmo3oY9tA==' - ) - end - end end end end diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb index 4ca8e1984be..5b2274e89f3 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb @@ -163,9 +163,10 @@ def upload_in_threads(pending, completed, options) etag: resp.etag, part_number: part[:part_number] }.tap do |h| - # get the requested checksum from the response + # 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) end completed.push(completed_part) diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/presigner.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/presigner.rb index fa15076a421..78c2ce9ac87 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/presigner.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/presigner.rb @@ -193,15 +193,14 @@ def sign_but_dont_send( req, expires_in, secure, time, unsigned_headers, hoist = true ) x_amz_headers = {} - http_req = req.context.http_request - - req.handlers.remove(Aws::S3::Plugins::S3Signer::LegacyHandler) - req.handlers.remove(Aws::Plugins::Sign::Handler) req.handlers.remove(Seahorse::Client::Plugins::ContentLength::Handler) req.handlers.remove(Aws::Rest::ContentTypeHandler) + req.handlers.remove(Aws::Plugins::ChecksumAlgorithm::OptionHandler) + req.handlers.remove(Aws::Plugins::ChecksumAlgorithm::ChecksumHandler) req.handlers.remove(Aws::Plugins::InvocationId::Handler) - + req.handlers.remove(Aws::Plugins::Sign::Handler) + req.handlers.remove(Aws::S3::Plugins::S3Signer::LegacyHandler) req.handle(step: :send) do |context| # if an endpoint was not provided, force secure or insecure if context.config.regional_endpoint diff --git a/gems/aws-sdk-s3/spec/object/upload_file_spec.rb b/gems/aws-sdk-s3/spec/object/upload_file_spec.rb index 0e147d43775..a894d1782a0 100644 --- a/gems/aws-sdk-s3/spec/object/upload_file_spec.rb +++ b/gems/aws-sdk-s3/spec/object/upload_file_spec.rb @@ -131,37 +131,37 @@ module S3 context 'large objects' do it 'uses multipart APIs for objects >= 100MB' do client.stub_responses(:create_multipart_upload, upload_id: 'id') - client.stub_responses(:upload_part, etag: 'etag') + client.stub_responses(:upload_part, etag: 'etag', checksum_crc32: 'checksum') expect(client).to receive(:complete_multipart_upload).with( bucket: 'bucket', key: 'key', upload_id: 'id', multipart_upload: { parts: [ - { etag: 'etag', part_number: 1 }, - { etag: 'etag', part_number: 2 }, - { etag: 'etag', part_number: 3 }, - { etag: 'etag', part_number: 4 }, - { etag: 'etag', part_number: 5 }, - { etag: 'etag', part_number: 6 }, - { etag: 'etag', part_number: 7 }, - { etag: 'etag', part_number: 8 }, - { etag: 'etag', part_number: 9 }, - { etag: 'etag', part_number: 10 }, - { etag: 'etag', part_number: 11 }, - { etag: 'etag', part_number: 12 }, - { etag: 'etag', part_number: 13 }, - { etag: 'etag', part_number: 14 }, - { etag: 'etag', part_number: 15 }, - { etag: 'etag', part_number: 16 }, - { etag: 'etag', part_number: 17 }, - { etag: 'etag', part_number: 18 }, - { etag: 'etag', part_number: 19 }, - { etag: 'etag', part_number: 20 }, - { etag: 'etag', part_number: 21 }, - { etag: 'etag', part_number: 22 }, - { etag: 'etag', part_number: 23 }, - { etag: 'etag', part_number: 24 } + { checksum_crc32: 'checksum', etag: 'etag', part_number: 1 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 2 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 3 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 4 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 5 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 6 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 7 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 8 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 9 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 10 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 11 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 12 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 13 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 14 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 15 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 16 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 17 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 18 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 19 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 20 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 21 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 22 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 23 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 24 } ] } ) @@ -176,7 +176,11 @@ module S3 client.stub_responses(:complete_multipart_upload) expect(client).to receive(:upload_part).exactly(24).times do |args| args[:on_chunk_sent].call(args[:body], args[:body].size, args[:body].size) - double(etag: 'etag') + double( + context: double(params: { checksum_algorithm: 'crc32' }), + checksum_crc32: 'checksum', + etag: 'etag' + ) end callback = proc do |bytes, totals| expect(bytes.size).to eq(24) From 00ddaabb3f9c96bba126df56346e4a18929f7ca8 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Fri, 23 Aug 2024 12:13:17 -0400 Subject: [PATCH 03/18] PR feedback --- gems/aws-sdk-core/CHANGELOG.md | 4 +- .../plugins/checksum_algorithm.rb | 61 ++++++++++++++++--- .../lib/aws-sdk-core/plugins/user_agent.rb | 19 +++++- .../aws-sdk-s3/plugins/checksum_algorithm.rb | 48 +++++++++++++++ .../skip_whole_multipart_get_checksums.rb | 31 ---------- services.json | 2 +- 6 files changed, 120 insertions(+), 45 deletions(-) create mode 100644 gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/checksum_algorithm.rb delete mode 100644 gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/skip_whole_multipart_get_checksums.rb diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index 71da2daa54d..d7f9ecc60c6 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -1,9 +1,9 @@ Unreleased Changes ------------------ -* Feature - Always calculate 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 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 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_calculation`, in the shared config file as `response_checksum_calculation`, and in the ENV as `ENV['AWS_RESPONSE_CHECKSUM_CALCULATION']`. 3.201.5 (2024-08-15) ------------------ diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index a9406badefe..4997e9189b8 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -157,17 +157,13 @@ def add_handlers(handlers, _config) class OptionHandler < Seahorse::Client::Handler def call(context) - # Enable validation on the service in all cases + # Set validation mode to enabled when supported. if context.config.response_checksum_calculation == 'WHEN_SUPPORTED' enable_request_validation_mode(context) end # Default checksum member to CRC32 if not set default_request_algorithm_member(context) - # Not modeled with httpChecksum - if context.operation_name == :create_multipart_upload - context.params[:checksum_algorithm] ||= DEFAULT_CHECKSUM - end @handler.call(context) end @@ -178,21 +174,20 @@ def enable_request_validation_mode(context) return unless context.operation.http_checksum input_member = context.operation.http_checksum['requestValidationModeMember'] - context.params[input_member.to_sym] = 'ENABLED' if input_member + context.params[input_member.to_sym] ||= 'ENABLED' if input_member end def default_request_algorithm_member(context) return unless context.operation.http_checksum input_member = context.operation.http_checksum['requestAlgorithmMember'] - context.params[input_member.to_sym] ||= 'CRC32' if input_member + context.params[input_member.to_sym] ||= DEFAULT_CHECKSUM if input_member end end class ChecksumHandler < Seahorse::Client::Handler def call(context) - context[:http_checksum] ||= {} - + algorithm = nil if should_calculate_request_checksum?(context) algorithm = choose_request_algorithm!(context) request_algorithm = { @@ -201,6 +196,7 @@ def call(context) name: "x-amz-checksum-#{algorithm.downcase}" } + context[:http_checksum] ||= {} context[:http_checksum][:request_algorithm] = request_algorithm calculate_request_checksum(context, request_algorithm) end @@ -209,11 +205,56 @@ def call(context) add_verify_response_checksum_handlers(context) end - @handler.call(context) + with_request_config_metric(context.config) do + with_response_config_metric(context.config) do + with_request_checksum_metrics(algorithm) do + @handler.call(context) + end + end + end end private + def with_request_config_metric(config, &block) + case config.request_checksum_calculation + when 'WHEN_SUPPORTED' + Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_WHEN_SUPPORTED', &block) + when 'WHEN_REQUIRED' + Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED', &block) + else + block.call + end + end + + def with_response_config_metric(config, &block) + case config.response_checksum_calculation + when 'WHEN_SUPPORTED' + Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED', &block) + when 'WHEN_REQUIRED' + Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED', &block) + else + block.call + end + end + + def with_request_checksum_metrics(algorithm, &block) + case algorithm + when 'CRC32' + Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_CRC32', &block) + when 'CRC32C' + Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_CRC32C', &block) + when 'CRC64NVME' + Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_CRC64', &block) + when 'SHA1' + Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_SHA1', &block) + when 'SHA256' + Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_SHA256', &block) + else + block.call + end + end + def request_algorithm_selection(context) return unless context.operation.http_checksum diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/user_agent.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/user_agent.rb index df7d94e1bd4..81db50643fa 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/user_agent.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/user_agent.rb @@ -17,7 +17,24 @@ class UserAgent < Seahorse::Client::Plugin "S3_CRYPTO_V2": "I", "S3_EXPRESS_BUCKET": "J", "S3_ACCESS_GRANTS": "K", - "GZIP_REQUEST_COMPRESSION": "L" + "GZIP_REQUEST_COMPRESSION": "L", + "PROTOCOL_RPC_V2_CBOR": "M", + "ENDPOINT_OVERRIDE": "N", + "ACCOUNT_ID_ENDPOINT": "O", + "ACCOUNT_ID_MODE_PREFERRED": "P", + "ACCOUNT_ID_MODE_DISABLED": "Q", + "ACCOUNT_ID_MODE_REQUIRED": "R", + "SIGV4A_SIGNING": "S", + "RESOLVED_ACCOUNT_ID": "T", + "FLEXIBLE_CHECKSUMS_REQ_CRC32" : "U", + "FLEXIBLE_CHECKSUMS_REQ_CRC32C" : "V", + "FLEXIBLE_CHECKSUMS_REQ_CRC64" : "W", + "FLEXIBLE_CHECKSUMS_REQ_SHA1" : "X", + "FLEXIBLE_CHECKSUMS_REQ_SHA256" : "Y", + "FLEXIBLE_CHECKSUMS_REQ_WHEN_SUPPORTED" : "Z", + "FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED" : "a", + "FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED" : "b", + "FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED" : "c" } METRICS diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/checksum_algorithm.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/checksum_algorithm.rb new file mode 100644 index 00000000000..15116c6e11e --- /dev/null +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/checksum_algorithm.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Aws + module S3 + module Plugins + # @api private + class ChecksumAlgorithm < Seahorse::Client::Plugin + + # S3 GetObject results for whole Multipart Objects contain a checksum + # that cannot be validated. These should be skipped by the + # ChecksumAlgorithm plugin. + class SkipWholeMultipartGetChecksumsHandler < Seahorse::Client::Handler + def call(context) + context[:http_checksum] ||= {} + context[:http_checksum][:skip_on_suffix] = true + + @handler.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 + end +end diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/skip_whole_multipart_get_checksums.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/skip_whole_multipart_get_checksums.rb deleted file mode 100644 index 4d274bcff42..00000000000 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/skip_whole_multipart_get_checksums.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -module Aws - module S3 - module Plugins - - # S3 GetObject results for whole Multipart Objects contain a checksum - # that cannot be validated. These should be skipped by the - # ChecksumAlgorithm plugin. - class SkipWholeMultipartGetChecksums < Seahorse::Client::Plugin - - class Handler < Seahorse::Client::Handler - - def call(context) - context[:http_checksum] ||= {} - context[:http_checksum][:skip_on_suffix] = true - - @handler.call(context) - end - - end - - handler( - Handler, - step: :initialize, - operations: [:get_object] - ) - end - end - end -end diff --git a/services.json b/services.json index 27ec3954203..a994fb928a6 100644 --- a/services.json +++ b/services.json @@ -1010,6 +1010,7 @@ "Aws::S3::Plugins::ARN", "Aws::S3::Plugins::BucketDns", "Aws::S3::Plugins::BucketNameRestrictions", + "Aws::S3::Plugins::ChecksumAlgorithm", "Aws::S3::Plugins::Dualstack", "Aws::S3::Plugins::Expect100Continue", "Aws::S3::Plugins::ExpressSessionAuth", @@ -1022,7 +1023,6 @@ "Aws::S3::Plugins::S3HostId", "Aws::S3::Plugins::S3Signer", "Aws::S3::Plugins::SseCpk", - "Aws::S3::Plugins::SkipWholeMultipartGetChecksums", "Aws::S3::Plugins::StreamingRetry", "Aws::S3::Plugins::UrlEncodedKeys" ] From 0499320c1310588d584d58132cd5080d3947bb4f Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Fri, 23 Aug 2024 12:35:27 -0400 Subject: [PATCH 04/18] Fix s3 specs --- gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb | 35 +++++++++++++++---- ...ums_spec.rb => checksum_algorithm_spec.rb} | 8 ++++- 2 files changed, 35 insertions(+), 8 deletions(-) rename gems/aws-sdk-s3/spec/plugins/{skip_whole_multipart_checksums_spec.rb => checksum_algorithm_spec.rb} (89%) diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb index a7057a69bce..fd78389f0d6 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb @@ -39,6 +39,7 @@ require 'aws-sdk-s3/plugins/arn.rb' require 'aws-sdk-s3/plugins/bucket_dns.rb' require 'aws-sdk-s3/plugins/bucket_name_restrictions.rb' +require 'aws-sdk-s3/plugins/checksum_algorithm.rb' require 'aws-sdk-s3/plugins/dualstack.rb' require 'aws-sdk-s3/plugins/expect_100_continue.rb' require 'aws-sdk-s3/plugins/express_session_auth.rb' @@ -51,7 +52,6 @@ require 'aws-sdk-s3/plugins/s3_host_id.rb' require 'aws-sdk-s3/plugins/s3_signer.rb' require 'aws-sdk-s3/plugins/sse_cpk.rb' -require 'aws-sdk-s3/plugins/skip_whole_multipart_get_checksums.rb' require 'aws-sdk-s3/plugins/streaming_retry.rb' require 'aws-sdk-s3/plugins/url_encoded_keys.rb' require 'aws-sdk-core/plugins/event_stream_configuration.rb' @@ -111,6 +111,7 @@ class Client < Seahorse::Client::Base add_plugin(Aws::S3::Plugins::ARN) add_plugin(Aws::S3::Plugins::BucketDns) add_plugin(Aws::S3::Plugins::BucketNameRestrictions) + add_plugin(Aws::S3::Plugins::ChecksumAlgorithm) add_plugin(Aws::S3::Plugins::Dualstack) add_plugin(Aws::S3::Plugins::Expect100Continue) add_plugin(Aws::S3::Plugins::ExpressSessionAuth) @@ -123,7 +124,6 @@ class Client < Seahorse::Client::Base add_plugin(Aws::S3::Plugins::S3HostId) add_plugin(Aws::S3::Plugins::S3Signer) add_plugin(Aws::S3::Plugins::SseCpk) - add_plugin(Aws::S3::Plugins::SkipWholeMultipartGetChecksums) add_plugin(Aws::S3::Plugins::StreamingRetry) add_plugin(Aws::S3::Plugins::UrlEncodedKeys) add_plugin(Aws::Plugins::EventStreamConfiguration) @@ -236,11 +236,9 @@ class Client < Seahorse::Client::Base # will use the Client Side Monitoring Agent Publisher. # # @option options [Boolean] :compute_checksums (true) - # When `true` a MD5 checksum will be computed and sent in the Content Md5 - # header for :put_object and :upload_part. When `false`, MD5 checksums - # will not be computed for these operations. Checksums are still computed - # for operations requiring them. Checksum errors returned by Amazon S3 are - # automatically retried up to `:retry_limit` times. + # This option is deprecated. Please use `:request_checksum_calculation` + # instead. When `true`, `request_checksum_calculation` is set to `WHEN_SUPPORTED`, + # and if `false` it is set to `WHEN_REQUIRED`. # # @option options [Boolean] :convert_params (true) # When `true`, an attempt is made to coerce request parameters into @@ -336,6 +334,18 @@ class Client < Seahorse::Client::Base # Used when loading credentials from the shared credentials file # at HOME/.aws/credentials. When not specified, 'default' is used. # + # @option options [String] :request_checksum_calculation ("WHEN_SUPPORTED") + # Determines when a checksum will be calculated for request payloads. Values are: + # + # * `WHEN_SUPPORTED` - (default) When set, a checksum will be + # calculated for all request payloads of operations modeled with the + # `httpChecksum` trait where `requestChecksumRequired` is `true` and/or a + # `requestAlgorithmMember` is modeled. + # * `WHEN_REQUIRED` - When set, a checksum will only be calculated for + # request payloads of operations modeled with the `httpChecksum` trait where + # `requestChecksumRequired` is `true` or where a requestAlgorithmMember + # is modeled and supplied. + # # @option options [Integer] :request_min_compression_size_bytes (10240) # The minimum size in bytes that triggers compression for request # bodies. The value must be non-negative integer value between 0 @@ -346,6 +356,17 @@ 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") + # Determines when checksum validation will be performed on response payloads. Values are: + # + # * `WHEN_SUPPORTED` - (default) When set, checksum validation is performed on all + # response payloads of operations modeled with the `httpChecksum` trait where + # `responseAlgorithms` is modeled, except when no modeled checksum algorithms + # are supported. + # * `WHEN_REQUIRED` - When set, checksum validation is not performed on + # response payloads of operations unless the checksum algorithm is supported and + # the `requestValidationModeMember` member is set to `ENABLED`. + # # @option options [Proc] :retry_backoff # A proc or lambda used for backoff. Defaults to 2**retries * retry_base_delay. # This option is only used in the `legacy` retry mode. diff --git a/gems/aws-sdk-s3/spec/plugins/skip_whole_multipart_checksums_spec.rb b/gems/aws-sdk-s3/spec/plugins/checksum_algorithm_spec.rb similarity index 89% rename from gems/aws-sdk-s3/spec/plugins/skip_whole_multipart_checksums_spec.rb rename to gems/aws-sdk-s3/spec/plugins/checksum_algorithm_spec.rb index ecba994055b..245dae36a2d 100644 --- a/gems/aws-sdk-s3/spec/plugins/skip_whole_multipart_checksums_spec.rb +++ b/gems/aws-sdk-s3/spec/plugins/checksum_algorithm_spec.rb @@ -5,7 +5,7 @@ module Aws module S3 module Plugins - describe SkipWholeMultipartGetChecksums do + describe ChecksumAlgorithm do let(:creds) { Aws::Credentials.new('akid', 'secret') } let(:client) { S3::Client.new(stub_responses: true) } let(:bucket) { 'bucket' } @@ -77,6 +77,12 @@ module Plugins resp = client.get_object(bucket: bucket, key: key, checksum_mode: 'ENABLED') expect(resp.context[:http_checksum][:validated]).to be_nil end + + it 'sets a default checksum algorithm for create_multipart_upload' do + resp = client.create_multipart_upload(bucket: bucket, key: key) + expect(resp.context.params[:checksum_algorithm]) + .to eq Aws::Plugins::ChecksumAlgorithm::DEFAULT_CHECKSUM + end end end end From ae4a90be73ec826c64afdc9960f2ac9d30acbf42 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Fri, 23 Aug 2024 12:50:52 -0400 Subject: [PATCH 05/18] Fix missing context --- .../lib/aws-sdk-core/plugins/checksum_algorithm.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index 4997e9189b8..fb952a6ca1f 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -157,6 +157,8 @@ def add_handlers(handlers, _config) class OptionHandler < Seahorse::Client::Handler def call(context) + context[:http_checksum] ||= {} + # Set validation mode to enabled when supported. if context.config.response_checksum_calculation == 'WHEN_SUPPORTED' enable_request_validation_mode(context) @@ -196,7 +198,6 @@ def call(context) name: "x-amz-checksum-#{algorithm.downcase}" } - context[:http_checksum] ||= {} context[:http_checksum][:request_algorithm] = request_algorithm calculate_request_checksum(context, request_algorithm) end From 573e34ba3eda66606a1a3145770889384ed17101 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Fri, 23 Aug 2024 12:58:20 -0400 Subject: [PATCH 06/18] Remove metric tests --- .../aws/plugins/request_compression_spec.rb | 14 -------------- .../aws/plugins/retry_errors_legacy_spec.rb | 9 --------- .../spec/aws/plugins/retry_errors_spec.rb | 18 ------------------ .../spec/plugins/express_session_auth_spec.rb | 7 ------- 4 files changed, 48 deletions(-) diff --git a/gems/aws-sdk-core/spec/aws/plugins/request_compression_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/request_compression_spec.rb index 897b1a7ef8a..48f61ae884b 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/request_compression_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/request_compression_spec.rb @@ -121,13 +121,6 @@ def expect_uncompressed_body(resp, body) end context 'request compression operation' do - it 'sets user-agent metric for the operation' do - resp = client.some_operation(body: uncompressed_body) - metric = Aws::Plugins::UserAgent::METRICS['GZIP_REQUEST_COMPRESSION'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end - it 'compresses the body and sets the content-encoding header' do resp = client.some_operation(body: uncompressed_body) expect(resp.context.http_request.headers['Content-Encoding']).to eq('gzip') @@ -165,13 +158,6 @@ def expect_uncompressed_body(resp, body) expect_uncompressed_body(resp, small_body) end - it 'sets user-agent metric for a streaming operation' do - resp = client.operation_streaming(body: 'body') - metric = Aws::Plugins::UserAgent::METRICS['GZIP_REQUEST_COMPRESSION'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end - it 'compresses a large streaming body' do large_body = StringIO.new('.' * 16_385) client.stub_responses(:operation_streaming, -> (context) do diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb index 13f66c5ab3f..067231e8004 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_legacy_spec.rb @@ -32,15 +32,6 @@ module Plugins client = RetryErrorsSvc::Client.new(retry_mode: 'legacy', region: 'us-west-2') expect(client.handlers.entries.map(&:handler_class)).to include(RetryErrors::LegacyHandler) end - - it 'sets user-agent metric' do - client = RetryErrorsSvc::Client.new(retry_mode: 'legacy', region: 'us-west-2') - stub_request(:post, client.config.endpoint) - resp = client.example_operation - metric = Aws::Plugins::UserAgent::METRICS['RETRY_MODE_LEGACY'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end end describe RetryErrors::LegacyHandler do diff --git a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb index 82838e29bdd..9b56972f352 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/retry_errors_spec.rb @@ -106,24 +106,6 @@ module Plugins .to receive(:correct_clock_skew).and_return('true') expect(client.config.correct_clock_skew).to eq(true) end - - it 'sets user-agent metric for standard' do - client = RetryErrorsSvc::Client.new(retry_mode: 'standard', region: 'us-west-2') - stub_request(:post, client.config.endpoint) - resp = client.example_operation - metric = Aws::Plugins::UserAgent::METRICS['RETRY_MODE_STANDARD'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end - - it 'sets user-agent metric for adaptive' do - client = RetryErrorsSvc::Client.new(retry_mode: 'adaptive', region: 'us-west-2') - stub_request(:post, client.config.endpoint) - resp = client.example_operation - metric = Aws::Plugins::UserAgent::METRICS['RETRY_MODE_ADAPTIVE'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end end describe RetryErrors::Handler do diff --git a/gems/aws-sdk-s3/spec/plugins/express_session_auth_spec.rb b/gems/aws-sdk-s3/spec/plugins/express_session_auth_spec.rb index f2ffd887b86..60d165a3237 100644 --- a/gems/aws-sdk-s3/spec/plugins/express_session_auth_spec.rb +++ b/gems/aws-sdk-s3/spec/plugins/express_session_auth_spec.rb @@ -128,13 +128,6 @@ module S3 end client.get_object(bucket: bucket, key: 'key') end - - it 'sets user-agent metric' do - resp = client.get_object(bucket: 'bucket--use1-az2--x-s3', key: 'key') - metric = Aws::Plugins::UserAgent::METRICS['S3_EXPRESS_BUCKET'] - expect(resp.context.http_request.headers['User-Agent']) - .to include("m/#{metric}") - end end # does not have http checksum trait, but requires a checksum (md5) From c18c340b8bebdcf1caf2c66973b7bd06dde6e09a Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Fri, 20 Sep 2024 16:31:27 -0400 Subject: [PATCH 07/18] Design updates and CRC64 usage --- .../plugins/checksum_algorithm.rb | 91 +++---- .../lib/aws-sdk-core/plugins/http_checksum.rb | 8 +- .../aws/plugins/checksum_algorithm_spec.rb | 8 +- .../spec/aws/plugins/checksum_request.json | 2 +- .../spec/aws/plugins/checksum_response.json | 222 +++++++++--------- .../plugins/checksum_streaming_request.json | 122 +++++----- 6 files changed, 226 insertions(+), 227 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index fb952a6ca1f..f000e4aacde 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -13,6 +13,7 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin begin require 'aws-crt' supported << 'CRC32C' + supported << 'CRC64NVME' if Aws::Crt::GEM_VERSION >= '0.3.0' rescue LoadError # Ignored end @@ -28,7 +29,7 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin CHECKSUM_SIZE = { 'CRC32' => 16, 'CRC32C' => 16, - 'CRC64NVME' => 32, + 'CRC64NVME' => 20, 'SHA1' => 36, 'SHA256' => 52 }.freeze @@ -74,14 +75,15 @@ class << self def digest_for_algorithm(algorithm) case algorithm when 'CRC32' - Digest32.new(Zlib.method(:crc32)) + Digest.new(Zlib.method(:crc32), 'N') when 'CRC32C' - # this will only be used if input algorithm is CRC32C AND client supports it (crt available) - Digest32.new(Aws::Crt::Checksums.method(:crc32c)) + Digest.new(Aws::Crt::Checksums.method(:crc32c), 'N') + when 'CRC64NVME' + Digest.new(Aws::Crt::Checksums.method(:crc64nvme), 'Q>') when 'SHA1' - Digest::SHA1.new + ::Digest::SHA1.new when 'SHA256' - Digest::SHA256.new + ::Digest::SHA256.new else raise ArgumentError, "#{algorithm} is not a supported checksum algorithm." @@ -126,16 +128,16 @@ def resolve_response_checksum_calculation(cfg) end # Interface for computing digests on request/response bodies - # which may be files, strings or IO like objects - # Applies only to digest functions that produce 32 bit integer checksums - # (eg CRC32) - class Digest32 - + # which may be files, strings or IO like objects. + # Applies only to digest functions that produce 32 or 64 bit + # integer checksums (eg CRC32 or CRC64). + class Digest attr_reader :value # @param [Object] digest_fn - def initialize(digest_fn) + def initialize(digest_fn, directive) @digest_fn = digest_fn + @directive = directive @value = 0 end @@ -144,7 +146,7 @@ def update(chunk) end def base64digest - Base64.encode64([@value].pack('N')).chomp + Base64.encode64([@value].pack(@directive)).chomp end end @@ -206,53 +208,49 @@ def call(context) add_verify_response_checksum_handlers(context) end - with_request_config_metric(context.config) do - with_response_config_metric(context.config) do - with_request_checksum_metrics(algorithm) do - @handler.call(context) - end - end - end + with_metrics(context.config, algorithm) { @handler.call(context) } end private - def with_request_config_metric(config, &block) + def with_metrics(config, algorithm, &block) + metrics = [] + add_request_config_metric(config, metrics) + add_response_config_metric(config, metrics) + add_request_checksum_metrics(algorithm, metrics) + Aws::Plugins::UserAgent.metric(*metrics, &block) + end + + def add_request_config_metric(config, metrics) case config.request_checksum_calculation when 'WHEN_SUPPORTED' - Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_WHEN_SUPPORTED', &block) + metrics << 'FLEXIBLE_CHECKSUMS_REQ_WHEN_SUPPORTED' when 'WHEN_REQUIRED' - Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED', &block) - else - block.call + metrics << 'FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED' end end - def with_response_config_metric(config, &block) + def add_response_config_metric(config, metrics) case config.response_checksum_calculation when 'WHEN_SUPPORTED' - Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED', &block) + metrics << 'FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED' when 'WHEN_REQUIRED' - Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED', &block) - else - block.call + metrics << 'FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED' end end - def with_request_checksum_metrics(algorithm, &block) + def add_request_checksum_metrics(algorithm, metrics) case algorithm when 'CRC32' - Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_CRC32', &block) + metrics << 'FLEXIBLE_CHECKSUMS_REQ_CRC32' when 'CRC32C' - Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_CRC32C', &block) + metrics << 'FLEXIBLE_CHECKSUMS_REQ_CRC32C' when 'CRC64NVME' - Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_CRC64', &block) + metrics << 'FLEXIBLE_CHECKSUMS_REQ_CRC64' when 'SHA1' - Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_SHA1', &block) + metrics << 'FLEXIBLE_CHECKSUMS_REQ_SHA1' when 'SHA256' - Aws::Plugins::UserAgent.metric('FLEXIBLE_CHECKSUMS_REQ_SHA256', &block) - else - block.call + metrics << 'FLEXIBLE_CHECKSUMS_REQ_SHA256' end end @@ -283,8 +281,7 @@ def checksum_required?(context) end def checksum_optional?(context) - (http_checksum = context.operation.http_checksum) && - !http_checksum['requestChecksumRequired'].nil? && + context.operation.http_checksum && context.config.request_checksum_calculation == 'WHEN_SUPPORTED' end @@ -293,17 +290,21 @@ def checksum_provided_as_header?(headers) end def should_calculate_request_checksum?(context) - (checksum_required?(context) || checksum_optional?(context)) && - !checksum_provided_as_header?(context.http_request.headers) + # requestAlgorithmMember must be present on the model - guaranteed + # a default from OptionHandler + request_algorithm_selection(context) && + !checksum_provided_as_header?(context.http_request.headers) && + (checksum_required?(context) || checksum_optional?(context)) end def choose_request_algorithm!(context) - algorithm = request_algorithm_selection(context) || DEFAULT_CHECKSUM + algorithm = request_algorithm_selection(context) return algorithm if CLIENT_ALGORITHMS.include?(algorithm) - if algorithm == 'CRC32C' + if %w[CRC32C CRC64NVME].include?(algorithm) raise ArgumentError, - 'CRC32C requires CRT support - install the aws-crt gem' + 'CRC32C and CRC64NVME requires CRT support ' \ + '- install the aws-crt gem' end raise ArgumentError, diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/http_checksum.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/http_checksum.rb index 7eb33001f2a..0fa5b91e2a8 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/http_checksum.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/http_checksum.rb @@ -11,7 +11,7 @@ class Handler < Seahorse::Client::Handler CHUNK_SIZE = 1 * 1024 * 1024 # one MB def call(context) - if checksum_required?(context) && + if context.operation.http_checksum_required && !context[:http_checksum][:request_algorithm] && # skip in favor of flexible checksum !context[:s3_express_endpoint] # s3 express endpoints do not support md5 body = context.http_request.body @@ -22,12 +22,6 @@ def call(context) private - def checksum_required?(context) - context.operation.http_checksum_required || - (context.operation.http_checksum && - context.operation.http_checksum['requestChecksumRequired']) - end - # @param [File, Tempfile, IO#read, String] value # @return [String] def md5(value) diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb index 3eccb833f43..43109d54618 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb @@ -169,12 +169,6 @@ module Plugins expect(header).to eq('AAAAAA==') end - it 'without requestAlgorithmMember; will use a CRC32 as a default' do - resp = client.checksum_required_operation(body: 'crc32 me captain') - header = resp.context.http_request.headers['x-amz-checksum-crc32'] - expect(header).to eq('nqtcGg==') - end - file = File.expand_path('checksum_request.json', __dir__) test_cases = JSON.load_file(file) @@ -306,7 +300,7 @@ module Plugins when 'failure' expect do client.http_checksum_operation(validation_mode: 'ENABLED') - end.to raise_error(Aws::Errors::ChecksumError, /#{expect['responseHeaders']}/) + end.to raise_error(Aws::Errors::ChecksumError, include(expect['calculatedChecksum'])) else raise 'Unsupported test kind' end diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json index 75ac0df6344..713e7a4b416 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json @@ -23,7 +23,7 @@ "checksumAlgorithm": "Crc64Nvme", "expectHeaders": { "x-amz-request-algorithm": "CRC64NVME", - "x-amz-checksum-crc64nvme": "uc8X9yrZrD4=" + "x-amz-checksum-crc64nvme": "OOJZ0D8xKts=" } }, { diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_response.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_response.json index 27e896ec3ac..89f6ce8d09b 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_response.json +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_response.json @@ -1,107 +1,117 @@ [ - { - "documentation": "Successful payload validation with CRC32 checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Crc32", - "responseHeaders": { - "x-amz-checksum-crc32": "i9aeUg==" - }, - "expect": { "kind": "success" } - }, - { - "documentation": "Successful payload validation with CRC32C checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Crc32C", - "responseHeaders": { - "x-amz-checksum-crc32c": "crUfeA==" - }, - "expect": { "kind": "success" } - }, - { - "documentation": "Successful payload validation with CRC64NVME checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Crc64Nvme", - "responseHeaders": { - "x-amz-checksum-crc64nvme": "uc8X9yrZrD4=" - }, - "expect": { "kind": "success" } - }, - { - "documentation": "Successful payload validation with SHA1 checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Sha1", - "responseHeaders": { - "x-amz-checksum-sha1": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" - }, - "expect": { "kind": "success" } - }, - { - "documentation": "Successful payload validation with SHA256 checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Sha256", - "responseHeaders": { - "x-amz-checksum-sha256": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" - }, - "expect": { "kind": "success" } - }, - { - "documentation": "Failed payload validation with CRC32 checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Crc32", - "responseHeaders": { - "x-amz-checksum-crc32": "bm90LWEtY2hlY2tzdW0=" - }, - "expect": { - "kind": "failure", - "calculatedChecksum": "i9aeUg==" - } - }, - { - "documentation": "Failed payload validation with CRC32C checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Crc32C", - "responseHeaders": { - "x-amz-checksum-crc32c": "bm90LWEtY2hlY2tzdW0=" - }, - "expect": { - "kind": "failure", - "calculatedChecksum": "crUfeA==" - } - }, - { - "documentation": "Failed payload validation with CRC64NVME checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Crc64Nvme", - "responseHeaders": { - "x-amz-checksum-crc64nvme": "bm90LWEtY2hlY2tzdW0=" - }, - "expect": { - "kind": "failure", - "calculatedChecksum": "uc8X9yrZrD4=" - } - }, - { - "documentation": "Failed payload validation with SHA1 checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Sha1", - "responseHeaders": { - "x-amz-checksum-sha1": "bm90LWEtY2hlY2tzdW0=" - }, - "expect": { - "kind": "failure", - "calculatedChecksum": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" - } - }, - { - "documentation": "Failed payload validation with SHA256 checksum.", - "responsePayload": "Hello world", - "checksumAlgorithm": "Sha256", - "responseHeaders": { - "x-amz-checksum-sha256": "bm90LWEtY2hlY2tzdW0=" - }, - "expect": { - "kind": "failure", - "calculatedChecksum": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" - } - } - ] \ No newline at end of file + { + "documentation": "Successful payload validation with CRC32 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc32", + "responseHeaders": { + "x-amz-checksum-crc32": "i9aeUg==" + }, + "expect": { + "kind": "success" + } + }, + { + "documentation": "Successful payload validation with CRC32C checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc32C", + "responseHeaders": { + "x-amz-checksum-crc32c": "crUfeA==" + }, + "expect": { + "kind": "success" + } + }, + { + "documentation": "Successful payload validation with CRC64NVME checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc64Nvme", + "responseHeaders": { + "x-amz-checksum-crc64nvme": "OOJZ0D8xKts=" + }, + "expect": { + "kind": "success" + } + }, + { + "documentation": "Successful payload validation with SHA1 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Sha1", + "responseHeaders": { + "x-amz-checksum-sha1": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" + }, + "expect": { + "kind": "success" + } + }, + { + "documentation": "Successful payload validation with SHA256 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Sha256", + "responseHeaders": { + "x-amz-checksum-sha256": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" + }, + "expect": { + "kind": "success" + } + }, + { + "documentation": "Failed payload validation with CRC32 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc32", + "responseHeaders": { + "x-amz-checksum-crc32": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "i9aeUg==" + } + }, + { + "documentation": "Failed payload validation with CRC32C checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc32C", + "responseHeaders": { + "x-amz-checksum-crc32c": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "crUfeA==" + } + }, + { + "documentation": "Failed payload validation with CRC64NVME checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Crc64Nvme", + "responseHeaders": { + "x-amz-checksum-crc64nvme": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "OOJZ0D8xKts=" + } + }, + { + "documentation": "Failed payload validation with SHA1 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Sha1", + "responseHeaders": { + "x-amz-checksum-sha1": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" + } + }, + { + "documentation": "Failed payload validation with SHA256 checksum.", + "responsePayload": "Hello world", + "checksumAlgorithm": "Sha256", + "responseHeaders": { + "x-amz-checksum-sha256": "bm90LWEtY2hlY2tzdW0=" + }, + "expect": { + "kind": "failure", + "calculatedChecksum": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" + } + } +] \ No newline at end of file diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json index 5c9ce933618..14750563d62 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json @@ -1,62 +1,62 @@ [ - { - "documentation": "CRC32 streaming checksum calculation works.", - "requestPayload": "Hello world", - "checksumAlgorithm": "Crc32", - "expectHeaders": { - "content-encoding": "aws-chunked", - "x-amz-trailer": "x-amz-checksum-crc32" - }, - "expectTrailers": { - "x-amz-checksum-crc32": "i9aeUg==" - } - }, - { - "documentation": "CRC32C streaming checksum calculation works.", - "requestPayload": "Hello world", - "checksumAlgorithm": "Crc32C", - "expectHeaders": { - "content-encoding": "aws-chunked", - "x-amz-trailer": "x-amz-checksum-crc32c" - }, - "expectTrailers": { - "x-amz-checksum-crc32c": "crUfeA==" - } - }, - { - "documentation": "CRC64NVME streaming checksum calculation works.", - "requestPayload": "Hello world", - "checksumAlgorithm": "Crc64Nvme", - "expectHeaders": { - "content-encoding": "aws-chunked", - "x-amz-trailer": "x-amz-checksum-crc64nvme" - }, - "expectTrailers": { - "x-amz-checksum-crc64nvme": "uc8X9yrZrD4=" - } - }, - { - "documentation": "SHA1 streaming checksum calculation works.", - "requestPayload": "Hello world", - "checksumAlgorithm": "Sha1", - "expectHeaders": { - "content-encoding": "aws-chunked", - "x-amz-trailer": "x-amz-checksum-sha1" - }, - "expectTrailers": { - "x-amz-checksum-sha1": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" - } - }, - { - "documentation": "SHA256 streaming checksum calculation works.", - "requestPayload": "Hello world", - "checksumAlgorithm": "Sha256", - "expectHeaders": { - "content-encoding": "aws-chunked", - "x-amz-trailer": "x-amz-checksum-sha256" - }, - "expectTrailers": { - "x-amz-checksum-sha256": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" - } - } - ] \ No newline at end of file + { + "documentation": "CRC32 streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc32", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-crc32" + }, + "expectTrailers": { + "x-amz-checksum-crc32": "i9aeUg==" + } + }, + { + "documentation": "CRC32C streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc32C", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-crc32c" + }, + "expectTrailers": { + "x-amz-checksum-crc32c": "crUfeA==" + } + }, + { + "documentation": "CRC64NVME streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc64Nvme", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-crc64nvme" + }, + "expectTrailers": { + "x-amz-checksum-crc64nvme": "OOJZ0D8xKts=" + } + }, + { + "documentation": "SHA1 streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Sha1", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-sha1" + }, + "expectTrailers": { + "x-amz-checksum-sha1": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" + } + }, + { + "documentation": "SHA256 streaming checksum calculation works.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Sha256", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-trailer": "x-amz-checksum-sha256" + }, + "expectTrailers": { + "x-amz-checksum-sha256": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" + } + } +] \ No newline at end of file From 761b87227edb40e6dae8be5bcf8f4cb3471da999 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Fri, 20 Sep 2024 18:34:46 -0400 Subject: [PATCH 08/18] Improve readability with trailer sizes --- .../plugins/checksum_algorithm.rb | 22 +++++++++---------- 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index f000e4aacde..3c585f31894 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -27,11 +27,12 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin # byte size of checksums, used in computing the trailer length CHECKSUM_SIZE = { - 'CRC32' => 16, - 'CRC32C' => 16, - 'CRC64NVME' => 20, - 'SHA1' => 36, - 'SHA256' => 52 + 'CRC32' => 9, + 'CRC32C' => 9, + 'CRC64NVME' => 13, + # SHA functions need 1 byte padding because of how they are encoded + 'SHA1' => 28 + 1, + 'SHA256' => 44 + 1 }.freeze DEFAULT_CHECKSUM = 'CRC32' @@ -90,10 +91,10 @@ def digest_for_algorithm(algorithm) end end - # The trailer size (in bytes) is the overhead + the trailer name + - # the length of the base64 encoded checksum + # The trailer size (in bytes) is the overhead (0, \r, \n) + the trailer + # name + the bytesize of the base64 encoded checksum. def trailer_length(algorithm, location_name) - CHECKSUM_SIZE[algorithm] + location_name.size + 7 + location_name.size + CHECKSUM_SIZE[algorithm] end private @@ -132,9 +133,6 @@ def resolve_response_checksum_calculation(cfg) # Applies only to digest functions that produce 32 or 64 bit # integer checksums (eg CRC32 or CRC64). class Digest - attr_reader :value - - # @param [Object] digest_fn def initialize(digest_fn, directive) @digest_fn = digest_fn @directive = directive @@ -492,7 +490,7 @@ def read(length, buf = nil) else trailers = {} trailers[@location_name] = @digest.base64digest - trailers = trailers.map { |k,v| "#{k}:#{v}"}.join("\r\n") + trailers = trailers.map { |k,v| "#{k}:#{v}" }.join("\r\n") @trailer_io = StringIO.new("0\r\n#{trailers}\r\n\r\n") chunk = @trailer_io.read(length, buf) end From eb1438671632ced1b3f62ac6ad63b10c0f67c439 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Thu, 10 Oct 2024 10:11:07 -0400 Subject: [PATCH 09/18] Update submodule --- tasks/benchmark | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tasks/benchmark b/tasks/benchmark index f3d048c1b5c..0db61c82f12 160000 --- a/tasks/benchmark +++ b/tasks/benchmark @@ -1 +1 @@ -Subproject commit f3d048c1b5c94b4a017756a38783557728c3aca7 +Subproject commit 0db61c82f125ad6df410e0662af11f9500336e2f From 2b20f4c8f5b01eaf8812440589d667e518663b0d Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Thu, 10 Oct 2024 11:46:11 -0400 Subject: [PATCH 10/18] Fixes from changed spec and new MPU behavior --- gems/aws-sdk-core/CHANGELOG.md | 2 +- .../plugins/checksum_algorithm.rb | 18 +++--- .../lib/aws-sdk-core/shared_config.rb | 2 +- .../aws/plugins/checksum_algorithm_spec.rb | 22 ++++---- gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb | 2 +- .../lib/aws-sdk-s3/file_downloader.rb | 23 +------- .../lib/aws-sdk-s3/multipart_file_uploader.rb | 11 ++-- .../aws-sdk-s3/plugins/checksum_algorithm.rb | 17 ------ .../spec/object/download_file_spec.rb | 10 ---- .../spec/object/upload_file_spec.rb | 56 +++++++++---------- .../spec/plugins/checksum_algorithm_spec.rb | 6 -- 11 files changed, 57 insertions(+), 112 deletions(-) diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index 707b0354e78..be353df1180 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -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) ------------------ diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index 3c585f31894..bbb80b4e799 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -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| @@ -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 @@ -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 @@ -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 @@ -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' diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/shared_config.rb b/gems/aws-sdk-core/lib/aws-sdk-core/shared_config.rb index 8dfe8cf73fa..1fea8bdbccd 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/shared_config.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/shared_config.rb @@ -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, diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb index 43109d54618..9d5025a8dfa 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb @@ -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 @@ -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 @@ -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') diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb index 2951635a939..f76b9554f32 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb @@ -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 diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb index 9e5b3b91b73..beb7351c836 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb @@ -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! @@ -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 @@ -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 @@ -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 @@ -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) diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb index 5b2274e89f3..331d283af2d 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb @@ -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 diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/checksum_algorithm.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/checksum_algorithm.rb index 15116c6e11e..c8a5ed4ba70 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/checksum_algorithm.rb @@ -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 diff --git a/gems/aws-sdk-s3/spec/object/download_file_spec.rb b/gems/aws-sdk-s3/spec/object/download_file_spec.rb index be61d31f9b1..d22bae4752b 100644 --- a/gems/aws-sdk-s3/spec/object/download_file_spec.rb +++ b/gems/aws-sdk-s3/spec/object/download_file_spec.rb @@ -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( @@ -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( @@ -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( @@ -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( @@ -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( @@ -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 @@ -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| @@ -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 @@ -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 @@ -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 diff --git a/gems/aws-sdk-s3/spec/object/upload_file_spec.rb b/gems/aws-sdk-s3/spec/object/upload_file_spec.rb index a894d1782a0..0e147d43775 100644 --- a/gems/aws-sdk-s3/spec/object/upload_file_spec.rb +++ b/gems/aws-sdk-s3/spec/object/upload_file_spec.rb @@ -131,37 +131,37 @@ module S3 context 'large objects' do it 'uses multipart APIs for objects >= 100MB' do client.stub_responses(:create_multipart_upload, upload_id: 'id') - client.stub_responses(:upload_part, etag: 'etag', checksum_crc32: 'checksum') + client.stub_responses(:upload_part, etag: 'etag') expect(client).to receive(:complete_multipart_upload).with( bucket: 'bucket', key: 'key', upload_id: 'id', multipart_upload: { parts: [ - { checksum_crc32: 'checksum', etag: 'etag', part_number: 1 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 2 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 3 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 4 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 5 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 6 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 7 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 8 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 9 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 10 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 11 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 12 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 13 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 14 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 15 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 16 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 17 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 18 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 19 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 20 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 21 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 22 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 23 }, - { checksum_crc32: 'checksum', etag: 'etag', part_number: 24 } + { etag: 'etag', part_number: 1 }, + { etag: 'etag', part_number: 2 }, + { etag: 'etag', part_number: 3 }, + { etag: 'etag', part_number: 4 }, + { etag: 'etag', part_number: 5 }, + { etag: 'etag', part_number: 6 }, + { etag: 'etag', part_number: 7 }, + { etag: 'etag', part_number: 8 }, + { etag: 'etag', part_number: 9 }, + { etag: 'etag', part_number: 10 }, + { etag: 'etag', part_number: 11 }, + { etag: 'etag', part_number: 12 }, + { etag: 'etag', part_number: 13 }, + { etag: 'etag', part_number: 14 }, + { etag: 'etag', part_number: 15 }, + { etag: 'etag', part_number: 16 }, + { etag: 'etag', part_number: 17 }, + { etag: 'etag', part_number: 18 }, + { etag: 'etag', part_number: 19 }, + { etag: 'etag', part_number: 20 }, + { etag: 'etag', part_number: 21 }, + { etag: 'etag', part_number: 22 }, + { etag: 'etag', part_number: 23 }, + { etag: 'etag', part_number: 24 } ] } ) @@ -176,11 +176,7 @@ module S3 client.stub_responses(:complete_multipart_upload) expect(client).to receive(:upload_part).exactly(24).times do |args| args[:on_chunk_sent].call(args[:body], args[:body].size, args[:body].size) - double( - context: double(params: { checksum_algorithm: 'crc32' }), - checksum_crc32: 'checksum', - etag: 'etag' - ) + double(etag: 'etag') end callback = proc do |bytes, totals| expect(bytes.size).to eq(24) diff --git a/gems/aws-sdk-s3/spec/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-s3/spec/plugins/checksum_algorithm_spec.rb index 245dae36a2d..f338fcb0317 100644 --- a/gems/aws-sdk-s3/spec/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-s3/spec/plugins/checksum_algorithm_spec.rb @@ -77,12 +77,6 @@ module Plugins resp = client.get_object(bucket: bucket, key: key, checksum_mode: 'ENABLED') expect(resp.context[:http_checksum][:validated]).to be_nil end - - it 'sets a default checksum algorithm for create_multipart_upload' do - resp = client.create_multipart_upload(bucket: bucket, key: key) - expect(resp.context.params[:checksum_algorithm]) - .to eq Aws::Plugins::ChecksumAlgorithm::DEFAULT_CHECKSUM - end end end end From 2a9453782f1aabe8bfb8a11bff33db2476fd071c Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Mon, 14 Oct 2024 13:19:53 -0400 Subject: [PATCH 11/18] Move create defaults to transfer manager --- gems/aws-sdk-core/CHANGELOG.md | 4 +- .../plugins/checksum_algorithm.rb | 50 ++++++++--------- .../aws/plugins/checksum_algorithm_spec.rb | 50 ++++++++--------- gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb | 35 +++--------- .../lib/aws-sdk-s3/file_downloader.rb | 2 +- .../lib/aws-sdk-s3/multipart_file_uploader.rb | 12 ++-- .../aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb | 8 +-- .../spec/object/upload_file_spec.rb | 56 ++++++++++--------- 8 files changed, 100 insertions(+), 117 deletions(-) diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index be353df1180..af1ac935fb5 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -1,9 +1,9 @@ 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 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_validation`, in the shared config file as `response_checksum_validation`, and in the ENV as `ENV['AWS_response_checksum_validation']`. +* 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) ------------------ diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index bbb80b4e799..334df6d5285 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -38,16 +38,16 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin DEFAULT_CHECKSUM = 'CRC32' option(:request_checksum_calculation, - doc_default: 'WHEN_SUPPORTED', + doc_default: 'when_supported', doc_type: 'String', docstring: <<~DOCS) do |cfg| Determines when a checksum will be calculated for request payloads. Values are: - * `WHEN_SUPPORTED` - (default) When set, a checksum will be + * `when_supported` - (default) When set, a checksum will be calculated for all request payloads of operations modeled with the `httpChecksum` trait where `requestChecksumRequired` is `true` and/or a `requestAlgorithmMember` is modeled. - * `WHEN_REQUIRED` - When set, a checksum will only be calculated for + * `when_required` - When set, a checksum will only be calculated for request payloads of operations modeled with the `httpChecksum` trait where `requestChecksumRequired` is `true` or where a requestAlgorithmMember is modeled and supplied. @@ -56,16 +56,16 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin end option(:response_checksum_validation, - doc_default: 'WHEN_SUPPORTED', + doc_default: 'when_supported', doc_type: 'String', docstring: <<~DOCS) do |cfg| Determines when checksum validation will be performed on response payloads. Values are: - * `WHEN_SUPPORTED` - (default) When set, checksum validation is performed on all + * `when_supported` - (default) When set, checksum validation is performed on all response payloads of operations modeled with the `httpChecksum` trait where `responseAlgorithms` is modeled, except when no modeled checksum algorithms are supported. - * `WHEN_REQUIRED` - When set, checksum validation is not performed on + * `when_required` - When set, checksum validation is not performed on response payloads of operations unless the checksum algorithm is supported and the `requestValidationModeMember` member is set to `ENABLED`. DOCS @@ -102,13 +102,13 @@ def trailer_length(algorithm, location_name) def resolve_request_checksum_calculation(cfg) mode = ENV['AWS_REQUEST_CHECKSUM_CALCULATION'] || Aws.shared_config.request_checksum_calculation(profile: cfg.profile) || - 'WHEN_SUPPORTED' - mode = mode.upcase - unless %w[WHEN_SUPPORTED WHEN_REQUIRED].include?(mode) + 'when_supported' + mode = mode.downcase + unless %w[when_supported when_required].include?(mode) raise ArgumentError, 'expected :request_checksum_calculation or' \ " ENV['AWS_REQUEST_CHECKSUM_CALCULATION'] to be " \ - '`WHEN_SUPPORTED` or `WHEN_REQUIRED`.' + '`when_supported` or `when_required`.' end mode end @@ -116,13 +116,13 @@ def resolve_request_checksum_calculation(cfg) 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) + 'when_supported' + mode = mode.downcase + unless %w[when_supported when_required].include?(mode) raise ArgumentError, 'expected :response_checksum_validation or' \ " ENV['AWS_response_checksum_validation'] to be " \ - '`WHEN_SUPPORTED` or `WHEN_REQUIRED`.' + '`when_supported` or `when_required`.' end mode end @@ -160,7 +160,7 @@ def call(context) context[:http_checksum] ||= {} # Set validation mode to enabled when supported. - if context.config.response_checksum_validation == 'WHEN_SUPPORTED' + if context.config.response_checksum_validation == 'when_supported' enable_request_validation_mode(context) end @@ -221,19 +221,19 @@ def with_metrics(config, algorithm, &block) def add_request_config_metric(config, metrics) case config.request_checksum_calculation - when 'WHEN_SUPPORTED' - metrics << 'FLEXIBLE_CHECKSUMS_REQ_WHEN_SUPPORTED' - when 'WHEN_REQUIRED' - metrics << 'FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED' + when 'when_supported' + metrics << 'FLEXIBLE_CHECKSUMS_REQ_when_supported' + when 'when_required' + metrics << 'FLEXIBLE_CHECKSUMS_REQ_when_required' end end def add_response_config_metric(config, metrics) case config.response_checksum_validation - when 'WHEN_SUPPORTED' - metrics << 'FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED' - when 'WHEN_REQUIRED' - metrics << 'FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED' + when 'when_supported' + metrics << 'FLEXIBLE_CHECKSUMS_RES_when_supported' + when 'when_required' + metrics << 'FLEXIBLE_CHECKSUMS_RES_when_required' end end @@ -275,12 +275,12 @@ def operation_response_algorithms(context) def checksum_required?(context) (http_checksum = context.operation.http_checksum) && (checksum_required = http_checksum['requestChecksumRequired']) && - (checksum_required && context.config.request_checksum_calculation == 'WHEN_REQUIRED') + (checksum_required && context.config.request_checksum_calculation == 'when_required') end def checksum_optional?(context) context.operation.http_checksum && - context.config.request_checksum_calculation == 'WHEN_SUPPORTED' + context.config.request_checksum_calculation == 'when_supported' end def checksum_provided_as_header?(headers) diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb index 9d5025a8dfa..c24ccadef88 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb @@ -101,58 +101,58 @@ module Plugins describe 'request_checksum_calculation' do it 'is configured to always compute by default' do expect(client.config.request_checksum_calculation) - .to eq('WHEN_SUPPORTED') + .to eq('when_supported') end it 'can be configured using shared config' do allow_any_instance_of(Aws::SharedConfig) .to receive(:request_checksum_calculation) - .and_return('WHEN_REQUIRED') + .and_return('when_required') expect(client.config.request_checksum_calculation) - .to eq('WHEN_REQUIRED') + .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(:request_checksum_calculation) - .and_return('WHEN_SUPPORTED') - ENV['AWS_REQUEST_CHECKSUM_CALCULATION'] = 'WHEN_REQUIRED' + .and_return('when_supported') + ENV['AWS_REQUEST_CHECKSUM_CALCULATION'] = 'when_required' expect(client.config.request_checksum_calculation) - .to eq('WHEN_REQUIRED') + .to eq('when_required') end it 'raises when request_checksum_calculation is not valid' do ENV['AWS_REQUEST_CHECKSUM_CALCULATION'] = 'peccy' - expect { client }.to raise_error(ArgumentError, /WHEN_SUPPORTED/) + expect { client }.to raise_error(ArgumentError, /when_supported/) end end describe 'response_checksum_validation' do it 'is configured to always verify by default' do expect(client.config.response_checksum_validation) - .to eq('WHEN_SUPPORTED') + .to eq('when_supported') end it 'can be configured using shared config' do allow_any_instance_of(Aws::SharedConfig) .to receive(:response_checksum_validation) - .and_return('WHEN_REQUIRED') + .and_return('when_required') expect(client.config.response_checksum_validation) - .to eq('WHEN_REQUIRED') + .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_validation) - .and_return('WHEN_SUPPORTED') - ENV['AWS_response_checksum_validation'] = 'WHEN_REQUIRED' + .and_return('when_supported') + ENV['AWS_response_checksum_validation'] = 'when_required' expect(client.config.response_checksum_validation) - .to eq('WHEN_REQUIRED') + .to eq('when_required') end it 'raises when response_checksum_validation is not valid' do ENV['AWS_response_checksum_validation'] = 'peccy' - expect { client }.to raise_error(ArgumentError, /WHEN_SUPPORTED/) + expect { client }.to raise_error(ArgumentError, /when_supported/) end end @@ -311,16 +311,16 @@ module Plugins context 'when checksums are not required' do let(:request_checksum_required) { false } - it 'WHEN_SUPPORTED; no algorithm; includes a checksum' do + it 'when_supported; no algorithm; includes a checksum' do resp = client.http_checksum_operation(checksum_algorithm: 'CRC32') expect(resp.context.http_request.headers['x-amz-checksum-crc32']) .to eq('AAAAAA==') end - it 'WHEN_REQUIRED; no algorithm; does not include a checksum' do + it 'when_required; no algorithm; does not include a checksum' do client = checksum_client.new( stub_responses: true, - request_checksum_calculation: 'WHEN_REQUIRED' + request_checksum_calculation: 'when_required' ) resp = client.http_checksum_operation expect(resp.context.http_request.headers['x-amz-checksum-crc32']) @@ -331,16 +331,16 @@ module Plugins context 'when checksums are required' do let(:request_checksum_required) { true } - it 'WHEN_SUPPORTED; no algorithm; includes a checksum' do + it 'when_supported; no algorithm; includes a checksum' do resp = client.http_checksum_operation expect(resp.context.http_request.headers['x-amz-checksum-crc32']) .to eq('AAAAAA==') end - it 'WHEN_REQUIRED; no algorithm; includes a checksum' do + it 'when_required; no algorithm; includes a checksum' do client = checksum_client.new( stub_responses: true, - request_checksum_calculation: 'WHEN_REQUIRED' + request_checksum_calculation: 'when_required' ) resp = client.http_checksum_operation expect(resp.context.http_request.headers['x-amz-checksum-crc32']) @@ -362,7 +362,7 @@ def stub_client(client) ) end - it 'WHEN_SUPPORTED; not ENABLED; validates the checksum' do + it 'when_supported; not ENABLED; validates the checksum' do stub_client(client) resp = client.http_checksum_operation expect(resp.context[:http_checksum][:validated]).to eq('CRC32') @@ -370,20 +370,20 @@ def stub_client(client) expect(resp.context.params[:validation_mode]).to eq('ENABLED') end - it 'WHEN_REQUIRED; not ENABLED; does not validate the checksum' do + it 'when_required; not ENABLED; does not validate the checksum' do client = checksum_client.new( stub_responses: true, - response_checksum_validation: 'WHEN_REQUIRED' + response_checksum_validation: 'when_required' ) stub_client(client) resp = client.http_checksum_operation expect(resp.context[:http_checksum][:validated]).to be_nil end - it 'WHEN_REQUIRED; ENABLED; validates the checksum' do + it 'when_required; ENABLED; validates the checksum' do client = checksum_client.new( stub_responses: true, - response_checksum_validation: 'WHEN_REQUIRED' + response_checksum_validation: 'when_required' ) stub_client(client) resp = client.http_checksum_operation(validation_mode: 'ENABLED') diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb index f76b9554f32..6b303f78d2f 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/client.rb @@ -40,7 +40,6 @@ require 'aws-sdk-s3/plugins/arn.rb' require 'aws-sdk-s3/plugins/bucket_dns.rb' require 'aws-sdk-s3/plugins/bucket_name_restrictions.rb' -require 'aws-sdk-s3/plugins/checksum_algorithm.rb' require 'aws-sdk-s3/plugins/dualstack.rb' require 'aws-sdk-s3/plugins/expect_100_continue.rb' require 'aws-sdk-s3/plugins/express_session_auth.rb' @@ -53,6 +52,7 @@ require 'aws-sdk-s3/plugins/s3_host_id.rb' require 'aws-sdk-s3/plugins/s3_signer.rb' require 'aws-sdk-s3/plugins/sse_cpk.rb' +require 'aws-sdk-s3/plugins/skip_whole_multipart_get_checksums.rb' require 'aws-sdk-s3/plugins/streaming_retry.rb' require 'aws-sdk-s3/plugins/url_encoded_keys.rb' require 'aws-sdk-core/plugins/event_stream_configuration.rb' @@ -111,7 +111,6 @@ class Client < Seahorse::Client::Base add_plugin(Aws::S3::Plugins::ARN) add_plugin(Aws::S3::Plugins::BucketDns) add_plugin(Aws::S3::Plugins::BucketNameRestrictions) - add_plugin(Aws::S3::Plugins::ChecksumAlgorithm) add_plugin(Aws::S3::Plugins::Dualstack) add_plugin(Aws::S3::Plugins::Expect100Continue) add_plugin(Aws::S3::Plugins::ExpressSessionAuth) @@ -124,6 +123,7 @@ class Client < Seahorse::Client::Base add_plugin(Aws::S3::Plugins::S3HostId) add_plugin(Aws::S3::Plugins::S3Signer) add_plugin(Aws::S3::Plugins::SseCpk) + add_plugin(Aws::S3::Plugins::SkipWholeMultipartGetChecksums) add_plugin(Aws::S3::Plugins::StreamingRetry) add_plugin(Aws::S3::Plugins::UrlEncodedKeys) add_plugin(Aws::Plugins::EventStreamConfiguration) @@ -240,9 +240,11 @@ class Client < Seahorse::Client::Base # will use the Client Side Monitoring Agent Publisher. # # @option options [Boolean] :compute_checksums (true) - # This option is deprecated. Please use `:request_checksum_calculation` - # instead. When `true`, `request_checksum_calculation` is set to `WHEN_SUPPORTED`, - # and if `false` it is set to `WHEN_REQUIRED`. + # When `true` a MD5 checksum will be computed and sent in the Content Md5 + # header for :put_object and :upload_part. When `false`, MD5 checksums + # will not be computed for these operations. Checksums are still computed + # for operations requiring them. Checksum errors returned by Amazon S3 are + # automatically retried up to `:retry_limit` times. # # @option options [Boolean] :convert_params (true) # When `true`, an attempt is made to coerce request parameters into @@ -338,18 +340,6 @@ class Client < Seahorse::Client::Base # Used when loading credentials from the shared credentials file # at HOME/.aws/credentials. When not specified, 'default' is used. # - # @option options [String] :request_checksum_calculation ("WHEN_SUPPORTED") - # Determines when a checksum will be calculated for request payloads. Values are: - # - # * `WHEN_SUPPORTED` - (default) When set, a checksum will be - # calculated for all request payloads of operations modeled with the - # `httpChecksum` trait where `requestChecksumRequired` is `true` and/or a - # `requestAlgorithmMember` is modeled. - # * `WHEN_REQUIRED` - When set, a checksum will only be calculated for - # request payloads of operations modeled with the `httpChecksum` trait where - # `requestChecksumRequired` is `true` or where a requestAlgorithmMember - # is modeled and supplied. - # # @option options [Integer] :request_min_compression_size_bytes (10240) # The minimum size in bytes that triggers compression for request # bodies. The value must be non-negative integer value between 0 @@ -360,17 +350,6 @@ 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_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 - # response payloads of operations modeled with the `httpChecksum` trait where - # `responseAlgorithms` is modeled, except when no modeled checksum algorithms - # are supported. - # * `WHEN_REQUIRED` - When set, checksum validation is not performed on - # response payloads of operations unless the checksum algorithm is supported and - # the `requestValidationModeMember` member is set to `ENABLED`. - # # @option options [Proc] :retry_backoff # A proc or lambda used for backoff. Defaults to 2**retries * retry_base_delay. # This option is only used in the `legacy` retry mode. diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb index beb7351c836..fd45b82ac66 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/file_downloader.rb @@ -28,7 +28,7 @@ def download(destination, options = {}) @chunk_size = options[:chunk_size] @params = { bucket: options[:bucket], - key: options[:key], + key: options[:key] } @params[:version_id] = options[:version_id] if options[:version_id] @on_checksum_validated = options[:on_checksum_validated] diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb index 331d283af2d..b2397c2c0bb 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/multipart_file_uploader.rb @@ -122,7 +122,9 @@ def compute_parts(upload_id, source, options) end def create_opts(options) - CREATE_OPTIONS.inject({}) do |hash, key| + default_checksum = Aws::Plugins::ChecksumAlgorithm::DEFAULT_CHECKSUM + opts = { checksum_algorithm: default_checksum } + CREATE_OPTIONS.inject(opts) do |hash, key| hash[key] = options[key] if options.key?(key) hash end @@ -163,11 +165,9 @@ def upload_in_threads(pending, completed, options) etag: resp.etag, part_number: part[:part_number] } - # 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 + algorithm = resp.context.params[:checksum_algorithm] + k = "checksum_#{algorithm.downcase}".to_sym + completed_part[k] = resp.send(k) completed.push(completed_part) end nil diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb index cec90b7252c..002a4f6faf5 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb @@ -14,16 +14,16 @@ class Md5s < Seahorse::Client::Plugin doc_type: 'Boolean', docstring: <<~DOCS) This option is deprecated. Please use `:request_checksum_calculation` - instead. When `true`, `request_checksum_calculation` is set to `WHEN_SUPPORTED`, - and if `false` it is set to `WHEN_REQUIRED`. + instead. When `true`, `request_checksum_calculation` is set to `when_supported`, + and if `false` it is set to `when_required`. DOCS def after_initialize(client) client.config.request_checksum_calculation = if client.config.compute_checksums - 'WHEN_SUPPORTED' + 'when_supported' else - 'WHEN_REQUIRED' + 'when_required' end end end diff --git a/gems/aws-sdk-s3/spec/object/upload_file_spec.rb b/gems/aws-sdk-s3/spec/object/upload_file_spec.rb index 0e147d43775..a894d1782a0 100644 --- a/gems/aws-sdk-s3/spec/object/upload_file_spec.rb +++ b/gems/aws-sdk-s3/spec/object/upload_file_spec.rb @@ -131,37 +131,37 @@ module S3 context 'large objects' do it 'uses multipart APIs for objects >= 100MB' do client.stub_responses(:create_multipart_upload, upload_id: 'id') - client.stub_responses(:upload_part, etag: 'etag') + client.stub_responses(:upload_part, etag: 'etag', checksum_crc32: 'checksum') expect(client).to receive(:complete_multipart_upload).with( bucket: 'bucket', key: 'key', upload_id: 'id', multipart_upload: { parts: [ - { etag: 'etag', part_number: 1 }, - { etag: 'etag', part_number: 2 }, - { etag: 'etag', part_number: 3 }, - { etag: 'etag', part_number: 4 }, - { etag: 'etag', part_number: 5 }, - { etag: 'etag', part_number: 6 }, - { etag: 'etag', part_number: 7 }, - { etag: 'etag', part_number: 8 }, - { etag: 'etag', part_number: 9 }, - { etag: 'etag', part_number: 10 }, - { etag: 'etag', part_number: 11 }, - { etag: 'etag', part_number: 12 }, - { etag: 'etag', part_number: 13 }, - { etag: 'etag', part_number: 14 }, - { etag: 'etag', part_number: 15 }, - { etag: 'etag', part_number: 16 }, - { etag: 'etag', part_number: 17 }, - { etag: 'etag', part_number: 18 }, - { etag: 'etag', part_number: 19 }, - { etag: 'etag', part_number: 20 }, - { etag: 'etag', part_number: 21 }, - { etag: 'etag', part_number: 22 }, - { etag: 'etag', part_number: 23 }, - { etag: 'etag', part_number: 24 } + { checksum_crc32: 'checksum', etag: 'etag', part_number: 1 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 2 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 3 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 4 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 5 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 6 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 7 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 8 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 9 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 10 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 11 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 12 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 13 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 14 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 15 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 16 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 17 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 18 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 19 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 20 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 21 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 22 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 23 }, + { checksum_crc32: 'checksum', etag: 'etag', part_number: 24 } ] } ) @@ -176,7 +176,11 @@ module S3 client.stub_responses(:complete_multipart_upload) expect(client).to receive(:upload_part).exactly(24).times do |args| args[:on_chunk_sent].call(args[:body], args[:body].size, args[:body].size) - double(etag: 'etag') + double( + context: double(params: { checksum_algorithm: 'crc32' }), + checksum_crc32: 'checksum', + etag: 'etag' + ) end callback = proc do |bytes, totals| expect(bytes.size).to eq(24) From 7d635e54aad0957fce90dc4a12de9760b0f69a08 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Sat, 2 Nov 2024 12:35:08 -0400 Subject: [PATCH 12/18] User provided checksum tests --- .../plugins/checksum_algorithm.rb | 20 +---- .../aws/plugins/checksum_algorithm_spec.rb | 67 ++++++++++++++++- .../spec/aws/plugins/checksum_provided.json | 74 +++++++++++++++++++ 3 files changed, 143 insertions(+), 18 deletions(-) create mode 100644 gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index 334df6d5285..da012a806c1 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -164,9 +164,6 @@ def call(context) enable_request_validation_mode(context) end - # Default checksum member to CRC32 if not set - default_request_algorithm_member(context) - @handler.call(context) end @@ -178,13 +175,6 @@ def enable_request_validation_mode(context) input_member = context.operation.http_checksum['requestValidationModeMember'] context.params[input_member.to_sym] ||= 'ENABLED' if input_member end - - def default_request_algorithm_member(context) - return unless context.operation.http_checksum - - input_member = context.operation.http_checksum['requestAlgorithmMember'] - context.params[input_member.to_sym] ||= DEFAULT_CHECKSUM if input_member - end end class ChecksumHandler < Seahorse::Client::Handler @@ -256,7 +246,7 @@ def request_algorithm_selection(context) return unless context.operation.http_checksum input_member = context.operation.http_checksum['requestAlgorithmMember'] - context.params[input_member.to_sym]&.upcase if input_member + context.params[input_member.to_sym] ||= DEFAULT_CHECKSUM if input_member end def request_validation_mode(context) @@ -288,15 +278,13 @@ def checksum_provided_as_header?(headers) end def should_calculate_request_checksum?(context) - # requestAlgorithmMember must be present on the model - guaranteed - # a default from OptionHandler - request_algorithm_selection(context) && - !checksum_provided_as_header?(context.http_request.headers) && + !checksum_provided_as_header?(context.http_request.headers) && + request_algorithm_selection(context) && (checksum_required?(context) || checksum_optional?(context)) end def choose_request_algorithm!(context) - algorithm = request_algorithm_selection(context) + algorithm = request_algorithm_selection(context).upcase return algorithm if CLIENT_ALGORITHMS.include?(algorithm) if %w[CRC32C CRC64NVME].include?(algorithm) diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb index c24ccadef88..b8d4f651631 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb @@ -44,10 +44,10 @@ module Plugins }, shapes: { 'Body' => { 'type' => 'blob' }, + 'String' => { 'type' => 'string' }, 'ChecksumAlgorithm' => { 'type' => 'string', - # SHA256 intentionally unmodeled for forwards compatibility test - 'enum' => ['CRC32', 'CRC32C', 'CRC64NVME', 'SHA1'] + 'enum' => ['CRC32', 'CRC32C', 'CRC64NVME', 'SHA1', 'SHA256'] }, 'SomeInput' => { 'type' => 'structure', @@ -58,6 +58,36 @@ module Plugins 'locationName' => 'x-amz-request-algorithm' }, 'ValidationMode' => { 'shape' => 'ValidationMode' }, + 'ChecksumCRC32' => { + 'shape' => 'String', + 'location' => 'header', + 'locationName' => 'x-amz-checksum-crc32' + }, + 'ChecksumCRC32C' => { + 'shape' => 'String', + 'location' => 'header', + 'locationName' => 'x-amz-checksum-crc32c' + }, + 'ChecksumCRC64NVME' => { + 'shape' => 'String', + 'location' => 'header', + 'locationName' => 'x-amz-checksum-crc64nvme' + }, + 'ChecksumSHA1' => { + 'shape' => 'String', + 'location' => 'header', + 'locationName' => 'x-amz-checksum-sha1' + }, + 'ChecksumSHA256' => { + 'shape' => 'String', + 'location' => 'header', + 'locationName' => 'x-amz-checksum-sha256' + }, + 'ChecksumFoo' => { + 'shape' => 'String', + 'location' => 'header', + 'locationName' => 'x-amz-checksum-foo' + }, 'Body' => { 'shape' => 'Body' } }, 'payload' => 'Body' @@ -308,6 +338,39 @@ module Plugins end end + context 'checksums are provided' do + file = File.expand_path('checksum_provided.json', __dir__) + test_cases = JSON.load_file(file) + + before do + algorithms = Aws::Plugins::ChecksumAlgorithm::CLIENT_ALGORITHMS.dup + algorithms << 'FOO' + algorithms.freeze + stub_const('Aws::Plugins::ChecksumAlgorithm::CLIENT_ALGORITHMS', algorithms) + end + + test_cases.each do |test_case| + it "passes test: #{test_case['documentation']}" do + algorithm = test_case['checksumAlgorithm'].upcase + unless ChecksumAlgorithm::CLIENT_ALGORITHMS.include?(algorithm) + skip "Algorithm #{algorithm} not supported" + end + + resp = client.http_checksum_operation( + "checksum_#{algorithm.downcase}".to_sym => test_case['checksumValue'], + body: test_case['requestPayload'] + ) + headers = resp.context.http_request.headers + test_case['expectHeaders'].each do |key, value| + expect(headers[key]).to eq(value) + end + test_case['expectNotPresentHeaders'].each do |key| + expect(headers[key]).to be_nil + end + end + end + end + context 'when checksums are not required' do let(:request_checksum_required) { false } diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json new file mode 100644 index 00000000000..59c95ef1463 --- /dev/null +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json @@ -0,0 +1,74 @@ +[ + { + "documentation": "CRC32 checksum provided by user.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc32", + "checksumValue": "i9aeUg==", + "expectHeaders": { + "x-amz-checksum-crc32": "i9aeUg==" + }, + "expectNotPresentHeaders": [ + "x-amz-request-algorithm" + ] + }, + { + "documentation": "CRC32C checksum provided by user.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc32C", + "checksumValue": "crUfeA==", + "expectHeaders": { + "x-amz-checksum-crc32c": "crUfeA==" + }, + "expectNotPresentHeaders": [ + "x-amz-request-algorithm" + ] + }, + { + "documentation": "CRC64NVME checksum provided by user.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Crc64Nvme", + "checksumValue": "OOJZ0D8xKts=", + "expectHeaders": { + "x-amz-checksum-crc64nvme": "OOJZ0D8xKts=" + }, + "expectNotPresentHeaders": [ + "x-amz-request-algorithm" + ] + }, + { + "documentation": "SHA1 checksum provided by user.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Sha1", + "checksumValue": "e1AsOh9IyGCa4hLN+2Od7jlnP14=", + "expectHeaders": { + "x-amz-checksum-sha1": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" + }, + "expectNotPresentHeaders": [ + "x-amz-request-algorithm" + ] + }, + { + "documentation": "SHA256 checksum provided by user.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Sha256", + "checksumValue": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=", + "expectHeaders": { + "x-amz-checksum-sha256": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" + }, + "expectNotPresentHeaders": [ + "x-amz-request-algorithm" + ] + }, + { + "documentation": "Unsupported checksum provided by user.", + "requestPayload": "Hello world", + "checksumAlgorithm": "Foo", + "checksumValue": "This-is-not-a-real-checksum", + "expectHeaders": { + "x-amz-checksum-foo": "This-is-not-a-real-checksum" + }, + "expectNotPresentHeaders": [ + "x-amz-request-algorithm" + ] + } +] \ No newline at end of file From 391d47c92602e836a8f04276c770c09095cacb43 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Sat, 2 Nov 2024 12:51:01 -0400 Subject: [PATCH 13/18] Remove algorithm check in test --- .../spec/aws/plugins/checksum_algorithm_spec.rb | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb index b8d4f651631..93f9b5b59de 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb @@ -351,11 +351,7 @@ module Plugins test_cases.each do |test_case| it "passes test: #{test_case['documentation']}" do - algorithm = test_case['checksumAlgorithm'].upcase - unless ChecksumAlgorithm::CLIENT_ALGORITHMS.include?(algorithm) - skip "Algorithm #{algorithm} not supported" - end - + algorithm = test_case['checksumAlgorithm'] resp = client.http_checksum_operation( "checksum_#{algorithm.downcase}".to_sym => test_case['checksumValue'], body: test_case['requestPayload'] From 75d7731a0b8f23cd9ab7bc8f6b3d0bda703b85cc Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Sun, 3 Nov 2024 17:27:49 -0500 Subject: [PATCH 14/18] Minor changes to provided checksum tests --- .../spec/aws/plugins/checksum_algorithm_spec.rb | 2 +- .../spec/aws/plugins/checksum_provided.json | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb index 93f9b5b59de..a88390b36a4 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb @@ -360,7 +360,7 @@ module Plugins test_case['expectHeaders'].each do |key, value| expect(headers[key]).to eq(value) end - test_case['expectNotPresentHeaders'].each do |key| + test_case['forbidHeaders'].each do |key| expect(headers[key]).to be_nil end end diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json index 59c95ef1463..9a440f62da9 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json @@ -7,19 +7,19 @@ "expectHeaders": { "x-amz-checksum-crc32": "i9aeUg==" }, - "expectNotPresentHeaders": [ + "forbidHeaders": [ "x-amz-request-algorithm" ] }, { "documentation": "CRC32C checksum provided by user.", "requestPayload": "Hello world", - "checksumAlgorithm": "Crc32C", + "checksumAlgorithm": "Crc32c", "checksumValue": "crUfeA==", "expectHeaders": { "x-amz-checksum-crc32c": "crUfeA==" }, - "expectNotPresentHeaders": [ + "forbidHeaders": [ "x-amz-request-algorithm" ] }, @@ -31,7 +31,7 @@ "expectHeaders": { "x-amz-checksum-crc64nvme": "OOJZ0D8xKts=" }, - "expectNotPresentHeaders": [ + "forbidHeaders": [ "x-amz-request-algorithm" ] }, @@ -43,7 +43,7 @@ "expectHeaders": { "x-amz-checksum-sha1": "e1AsOh9IyGCa4hLN+2Od7jlnP14=" }, - "expectNotPresentHeaders": [ + "forbidHeaders": [ "x-amz-request-algorithm" ] }, @@ -55,19 +55,19 @@ "expectHeaders": { "x-amz-checksum-sha256": "ZOyIygCyaOW6GjVnihtTFtIS9PNmskdyMlNKiuyjfzw=" }, - "expectNotPresentHeaders": [ + "forbidHeaders": [ "x-amz-request-algorithm" ] }, { - "documentation": "Unsupported checksum provided by user.", + "documentation": "Forwards compatibility, unmodeled checksum provided by user.", "requestPayload": "Hello world", "checksumAlgorithm": "Foo", "checksumValue": "This-is-not-a-real-checksum", "expectHeaders": { "x-amz-checksum-foo": "This-is-not-a-real-checksum" }, - "expectNotPresentHeaders": [ + "forbidHeaders": [ "x-amz-request-algorithm" ] } From 0e2cc06b1907b3622ab595427936da806562dc60 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Mon, 11 Nov 2024 09:24:58 -0500 Subject: [PATCH 15/18] Minor change --- gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json index 9a440f62da9..86bc5f77ef6 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_provided.json @@ -14,7 +14,7 @@ { "documentation": "CRC32C checksum provided by user.", "requestPayload": "Hello world", - "checksumAlgorithm": "Crc32c", + "checksumAlgorithm": "Crc32C", "checksumValue": "crUfeA==", "expectHeaders": { "x-amz-checksum-crc32c": "crUfeA==" From 426710853396e45b0501118826054ba67af9ca66 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Mon, 18 Nov 2024 17:25:13 -0500 Subject: [PATCH 16/18] PR feedback --- .../plugins/checksum_algorithm.rb | 20 ++++++++++--------- .../aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb | 12 +++-------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index da012a806c1..ff870f25e12 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -20,6 +20,8 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin supported end.freeze + CRT_ALGORITHMS = %w[CRC32C CRC64NVME].freeze + # Priority order of checksum algorithms to validate responses against. # Remove any algorithms not supported by client (ie, depending on CRT availability). # This list was chosen based on average performance. @@ -49,7 +51,7 @@ class ChecksumAlgorithm < Seahorse::Client::Plugin `requestAlgorithmMember` is modeled. * `when_required` - When set, a checksum will only be calculated for request payloads of operations modeled with the `httpChecksum` trait where - `requestChecksumRequired` is `true` or where a requestAlgorithmMember + `requestChecksumRequired` is `true` or where a `requestAlgorithmMember` is modeled and supplied. DOCS resolve_request_checksum_calculation(cfg) @@ -114,14 +116,14 @@ def resolve_request_checksum_calculation(cfg) end def resolve_response_checksum_validation(cfg) - mode = ENV['AWS_response_checksum_validation'] || + mode = ENV['AWS_RESPONSE_CHECKSUM_VALIDATION'] || Aws.shared_config.response_checksum_validation(profile: cfg.profile) || 'when_supported' mode = mode.downcase unless %w[when_supported when_required].include?(mode) raise ArgumentError, 'expected :response_checksum_validation or' \ - " ENV['AWS_response_checksum_validation'] to be " \ + " ENV['AWS_RESPONSE_CHECKSUM_VALIDATION'] to be " \ '`when_supported` or `when_required`.' end mode @@ -212,18 +214,18 @@ def with_metrics(config, algorithm, &block) def add_request_config_metric(config, metrics) case config.request_checksum_calculation when 'when_supported' - metrics << 'FLEXIBLE_CHECKSUMS_REQ_when_supported' + metrics << 'FLEXIBLE_CHECKSUMS_REQ_WHEN_SUPPORTED' when 'when_required' - metrics << 'FLEXIBLE_CHECKSUMS_REQ_when_required' + metrics << 'FLEXIBLE_CHECKSUMS_REQ_WHEN_REQUIRED' end end def add_response_config_metric(config, metrics) case config.response_checksum_validation when 'when_supported' - metrics << 'FLEXIBLE_CHECKSUMS_RES_when_supported' + metrics << 'FLEXIBLE_CHECKSUMS_RES_WHEN_SUPPORTED' when 'when_required' - metrics << 'FLEXIBLE_CHECKSUMS_RES_when_required' + metrics << 'FLEXIBLE_CHECKSUMS_RES_WHEN_REQUIRED' end end @@ -270,7 +272,7 @@ def checksum_required?(context) def checksum_optional?(context) context.operation.http_checksum && - context.config.request_checksum_calculation == 'when_supported' + context.config.request_checksum_calculation != 'when_required' end def checksum_provided_as_header?(headers) @@ -287,7 +289,7 @@ def choose_request_algorithm!(context) algorithm = request_algorithm_selection(context).upcase return algorithm if CLIENT_ALGORITHMS.include?(algorithm) - if %w[CRC32C CRC64NVME].include?(algorithm) + if CRT_ALGORITHMS.include?(algorithm) raise ArgumentError, 'CRC32C and CRC64NVME requires CRT support ' \ '- install the aws-crt gem' diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb index 002a4f6faf5..65428fd4254 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/md5s.rb @@ -13,18 +13,12 @@ class Md5s < Seahorse::Client::Plugin default: true, doc_type: 'Boolean', docstring: <<~DOCS) - This option is deprecated. Please use `:request_checksum_calculation` - instead. When `true`, `request_checksum_calculation` is set to `when_supported`, - and if `false` it is set to `when_required`. + This option is deprecated. Please use `:request_checksum_calculation` instead. + When `false`, `request_checksum_calculation` is overridden to `when_required`. DOCS def after_initialize(client) - client.config.request_checksum_calculation = - if client.config.compute_checksums - 'when_supported' - else - 'when_required' - end + client.config.request_checksum_calculation = 'when_required' unless client.config.compute_checksums end end end From 5e65945ecc0adac02fd1f147d60f72f6d1550745 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Mon, 18 Nov 2024 18:11:59 -0500 Subject: [PATCH 17/18] Default algorithm header test cases --- .../plugins/checksum_algorithm.rb | 39 ++++++++++------ .../aws/plugins/checksum_algorithm_spec.rb | 45 +++++++++---------- .../spec/aws/plugins/checksum_request.json | 8 ++++ .../plugins/checksum_streaming_request.json | 17 +++++++ 4 files changed, 73 insertions(+), 36 deletions(-) diff --git a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb index ff870f25e12..44746a6d466 100644 --- a/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb +++ b/gems/aws-sdk-core/lib/aws-sdk-core/plugins/checksum_algorithm.rb @@ -187,7 +187,8 @@ def call(context) request_algorithm = { algorithm: algorithm, in: checksum_request_in(context), - name: "x-amz-checksum-#{algorithm.downcase}" + name: "x-amz-checksum-#{algorithm.downcase}", + request_algorithm_header: request_algorithm_header(context) } context[:http_checksum][:request_algorithm] = request_algorithm @@ -251,6 +252,12 @@ def request_algorithm_selection(context) context.params[input_member.to_sym] ||= DEFAULT_CHECKSUM if input_member end + def request_algorithm_header(context) + input_member = context.operation.http_checksum['requestAlgorithmMember'] + shape = context.operation.input.shape.member(input_member) + shape.location_name if shape && shape.location == 'header' + end + def request_validation_mode(context) return unless context.operation.http_checksum @@ -309,22 +316,29 @@ def checksum_request_in(context) end def calculate_request_checksum(context, checksum_properties) + headers = context.http_request.headers + if (algorithm_header = checksum_properties[:request_algorithm_header]) + headers[algorithm_header] = checksum_properties[:algorithm] + end case checksum_properties[:in] when 'header' - header_name = checksum_properties[:name] - headers = context.http_request.headers - unless headers[header_name] - body = context.http_request.body_contents - headers[header_name] = calculate_checksum( - checksum_properties[:algorithm], - body - ) - end + apply_request_checksum(context, headers, checksum_properties) when 'trailer' - apply_request_trailer_checksum(context, checksum_properties) + apply_request_trailer_checksum(context, headers, checksum_properties) + else + # nothing end end + def apply_request_checksum(context, headers, checksum_properties) + header_name = checksum_properties[:name] + body = context.http_request.body_contents + headers[header_name] = calculate_checksum( + checksum_properties[:algorithm], + body + ) + end + def calculate_checksum(algorithm, body) digest = ChecksumAlgorithm.digest_for_algorithm(algorithm) if body.respond_to?(:read) @@ -345,11 +359,10 @@ def update_in_chunks(digest, io) io.rewind end - def apply_request_trailer_checksum(context, checksum_properties) + def apply_request_trailer_checksum(context, headers, checksum_properties) location_name = checksum_properties[:name] # set required headers - headers = context.http_request.headers headers['Content-Encoding'] = 'aws-chunked' headers['X-Amz-Content-Sha256'] = 'STREAMING-UNSIGNED-PAYLOAD-TRAILER' headers['X-Amz-Trailer'] = location_name diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb index a88390b36a4..39fda777e87 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_algorithm_spec.rb @@ -175,13 +175,13 @@ module Plugins allow_any_instance_of(Aws::SharedConfig) .to receive(:response_checksum_validation) .and_return('when_supported') - ENV['AWS_response_checksum_validation'] = 'when_required' + ENV['AWS_RESPONSE_CHECKSUM_VALIDATION'] = 'when_required' expect(client.config.response_checksum_validation) .to eq('when_required') end it 'raises when response_checksum_validation is not valid' do - ENV['AWS_response_checksum_validation'] = 'peccy' + ENV['AWS_RESPONSE_CHECKSUM_VALIDATION'] = 'peccy' expect { client }.to raise_error(ArgumentError, /when_supported/) end end @@ -193,26 +193,23 @@ module Plugins end.to raise_error(ArgumentError) end - it 'with requestAlgorithmMember; will use a CRC32 as a default' do - resp = client.http_checksum_operation - header = resp.context.http_request.headers['x-amz-checksum-crc32'] - expect(header).to eq('AAAAAA==') - end - file = File.expand_path('checksum_request.json', __dir__) test_cases = JSON.load_file(file) test_cases.each do |test_case| it "passes test: #{test_case['documentation']}" do - algorithm = test_case['checksumAlgorithm'].upcase - unless ChecksumAlgorithm::CLIENT_ALGORITHMS.include?(algorithm) - skip "Algorithm #{algorithm} not supported" + options = { + body: test_case['requestPayload'] + } + if (algorithm = test_case['checksumAlgorithm']) + algorithm.upcase! + unless ChecksumAlgorithm::CLIENT_ALGORITHMS.include?(algorithm) + skip "Algorithm #{algorithm} not supported" + end + options[:checksum_algorithm] = algorithm end - resp = client.http_checksum_operation( - checksum_algorithm: algorithm, - body: test_case['requestPayload'] - ) + resp = client.http_checksum_operation(**options) headers = resp.context.http_request.headers test_case['expectHeaders'].each do |key, value| expect(headers[key]).to eq(value) @@ -227,12 +224,17 @@ module Plugins test_cases.each do |test_case| it "passes test: #{test_case['documentation']}" do - algorithm = test_case['checksumAlgorithm'].upcase - unless ChecksumAlgorithm::CLIENT_ALGORITHMS.include?(algorithm) - skip "Algorithm #{algorithm} not supported" + options = { + body: test_case['requestPayload'] + } + if (algorithm = test_case['checksumAlgorithm']) + algorithm.upcase! + unless ChecksumAlgorithm::CLIENT_ALGORITHMS.include?(algorithm) + skip "Algorithm #{algorithm} not supported" + end + options[:checksum_algorithm] = algorithm end - body = test_case['requestPayload'] client.stub_responses(:http_checksum_streaming_operation, lambda do |context| headers = context.http_request.headers @@ -256,10 +258,7 @@ module Plugins context end) - client.http_checksum_streaming_operation( - checksum_algorithm: algorithm, - body: body - ) + client.http_checksum_streaming_operation(**options) end end end diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json index 713e7a4b416..9afd88c28ff 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_request.json @@ -1,4 +1,12 @@ [ + { + "documentation": "Defaults to CRC32.", + "requestPayload": "Hello world", + "expectHeaders": { + "x-amz-request-algorithm": "CRC32", + "x-amz-checksum-crc32": "i9aeUg==" + } + }, { "documentation": "CRC32 checksum calculation works.", "requestPayload": "Hello world", diff --git a/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json b/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json index 14750563d62..3c472ad9b27 100644 --- a/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json +++ b/gems/aws-sdk-core/spec/aws/plugins/checksum_streaming_request.json @@ -1,10 +1,23 @@ [ + { + "documentation": "Defaults to CRC32.", + "requestPayload": "Hello world", + "expectHeaders": { + "content-encoding": "aws-chunked", + "x-amz-request-algorithm": "CRC32", + "x-amz-trailer": "x-amz-checksum-crc32" + }, + "expectTrailers": { + "x-amz-checksum-crc32": "i9aeUg==" + } + }, { "documentation": "CRC32 streaming checksum calculation works.", "requestPayload": "Hello world", "checksumAlgorithm": "Crc32", "expectHeaders": { "content-encoding": "aws-chunked", + "x-amz-request-algorithm": "CRC32", "x-amz-trailer": "x-amz-checksum-crc32" }, "expectTrailers": { @@ -17,6 +30,7 @@ "checksumAlgorithm": "Crc32C", "expectHeaders": { "content-encoding": "aws-chunked", + "x-amz-request-algorithm": "CRC32C", "x-amz-trailer": "x-amz-checksum-crc32c" }, "expectTrailers": { @@ -29,6 +43,7 @@ "checksumAlgorithm": "Crc64Nvme", "expectHeaders": { "content-encoding": "aws-chunked", + "x-amz-request-algorithm": "CRC64NVME", "x-amz-trailer": "x-amz-checksum-crc64nvme" }, "expectTrailers": { @@ -41,6 +56,7 @@ "checksumAlgorithm": "Sha1", "expectHeaders": { "content-encoding": "aws-chunked", + "x-amz-request-algorithm": "SHA1", "x-amz-trailer": "x-amz-checksum-sha1" }, "expectTrailers": { @@ -53,6 +69,7 @@ "checksumAlgorithm": "Sha256", "expectHeaders": { "content-encoding": "aws-chunked", + "x-amz-request-algorithm": "SHA256", "x-amz-trailer": "x-amz-checksum-sha256" }, "expectTrailers": { From 4df2d605d6905e6710d656b5cad01de070b8d8b2 Mon Sep 17 00:00:00 2001 From: Matt Muller Date: Tue, 19 Nov 2024 13:08:15 -0500 Subject: [PATCH 18/18] Fix changelog --- gems/aws-sdk-core/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gems/aws-sdk-core/CHANGELOG.md b/gems/aws-sdk-core/CHANGELOG.md index 4a81c3406ca..65f9a8f25e6 100644 --- a/gems/aws-sdk-core/CHANGELOG.md +++ b/gems/aws-sdk-core/CHANGELOG.md @@ -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_validation`, in the shared config file as `response_checksum_validation`, and in the ENV as `ENV['AWS_response_checksum_validation']`. +* 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.213.0 (2024-11-14) ------------------