From 8a667ab87b34e693118d8076719c97fe0eba34de Mon Sep 17 00:00:00 2001 From: Matt Muller <53055821+mullermp@users.noreply.github.com> Date: Mon, 11 Nov 2024 09:27:22 -0500 Subject: [PATCH] Tighten regex used for S3 200 errors (#3130) --- gems/aws-sdk-s3/CHANGELOG.md | 2 + .../lib/aws-sdk-s3/plugins/http_200_errors.rb | 6 +- gems/aws-sdk-s3/spec/client_spec.rb | 303 ++++++++++-------- 3 files changed, 169 insertions(+), 142 deletions(-) diff --git a/gems/aws-sdk-s3/CHANGELOG.md b/gems/aws-sdk-s3/CHANGELOG.md index 538df708ad5..a6bfa6eae3f 100644 --- a/gems/aws-sdk-s3/CHANGELOG.md +++ b/gems/aws-sdk-s3/CHANGELOG.md @@ -1,6 +1,8 @@ Unreleased Changes ------------------ +* Issue - Tighten regex used to check for S3 200 errors. + 1.170.0 (2024-11-06) ------------------ diff --git a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/http_200_errors.rb b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/http_200_errors.rb index d193fe3356e..d8538b84e9b 100644 --- a/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/http_200_errors.rb +++ b/gems/aws-sdk-s3/lib/aws-sdk-s3/plugins/http_200_errors.rb @@ -71,9 +71,9 @@ def incomplete_xml_body?(xml, output) def check_for_error(context) xml = context.http_response.body_contents - if xml.match(/\?>\s*/) - error_code = xml.match(/(.+?)<\/Code>/)[1] - error_message = xml.match(/(.+?)<\/Message>/)[1] + if xml.match(/<\?xml\s[^>]*\?>\s*/) + error_code = xml.match(%r{(.+?)})[1] + error_message = xml.match(%r{(.+?)})[1] S3::Errors.error_class(error_code).new(context, error_message) elsif incomplete_xml_body?(xml, context.operation.output) Seahorse::Client::NetworkingError.new( diff --git a/gems/aws-sdk-s3/spec/client_spec.rb b/gems/aws-sdk-s3/spec/client_spec.rb index 7e97440128b..1fabee78a0f 100644 --- a/gems/aws-sdk-s3/spec/client_spec.rb +++ b/gems/aws-sdk-s3/spec/client_spec.rb @@ -44,16 +44,16 @@ module S3 'rw0nS41rawnLDzkf+PKXmmt/uEi4bzvNMr72o=', 'x-amz-request-id' => 'BE9C18E622969B17' }, - body: <<-XML) - - - - - aws-sdk-ruby - - - -XML + body: <<~XML) + + + + + aws-sdk-ruby + + + + XML Seahorse::Client::Response.new(context: context) end resp = client.list_buckets @@ -119,15 +119,15 @@ module S3 context.http_response.signal_done( status_code: 301, headers: { 'x-amz-bucket-region' => 'us-peccy-1' }, - body: <<-BODY) - - - PermanentRedirect - Error message. - http://foo.com - bucket - -BODY + body: <<~BODY) + + + PermanentRedirect + Error message. + http://foo.com + bucket + + BODY Seahorse::Client::Response.new(context: context) end expect do @@ -373,10 +373,10 @@ module S3 end resp = client.create_bucket(bucket: 'aws-sdk') expect(resp.context.http_request.body_contents.strip) - .to eq(<<-XML.gsub(/(^\s+|\n)/, '')) - - us-west-2 - + .to eq(<<~XML.gsub(/(^\s+|\n)/, '')) + + us-west-2 + XML end @@ -393,10 +393,10 @@ module S3 } ) expect(resp.context.http_request.body_contents.strip) - .to eq(<<-XML.gsub(/(^\s+|\n)/, '')) - - EU - + .to eq(<<~XML.gsub(/(^\s+|\n)/, '')) + + EU + XML end @@ -413,10 +413,10 @@ module S3 } ) expect(resp.context.http_request.body_contents.strip) - .to eq(<<-XML.gsub(/(^\s+|\n)/, '')) - - AvailabilityZoneus-west-1a - + .to eq(<<~XML.gsub(/(^\s+|\n)/, '')) + + AvailabilityZoneus-west-1a + XML end end @@ -428,15 +428,15 @@ module S3 context.http_response.signal_done( status_code: 409, headers: {}, - body: <<-XML.strip - - - BucketNotEmpty - The bucket you tried to delete is not empty - aws-sdk - 740BE6AB575EACED - MQVg1RMI+d93Hps1E8qpIuDb9Gd2TzkDhm0wE40981DjxSHP1UfLBB7qOAlwAqJB - + body: <<~XML.strip + + + BucketNotEmpty + The bucket you tried to delete is not empty + aws-sdk + 740BE6AB575EACED + MQVg1RMI+d93Hps1E8qpIuDb9Gd2TzkDhm0wE40981DjxSHP1UfLBB7qOAlwAqJB + XML ) Seahorse::Client::Response.new(context: context) @@ -457,9 +457,9 @@ module S3 context.http_response.signal_done( status_code: 200, headers: {}, - body: <<-XML.strip - -EU + body: <<~XML.strip + + EU XML ) Seahorse::Client::Response.new(context: context) @@ -474,9 +474,9 @@ module S3 context.http_response.signal_done( status_code: 200, headers: {}, - body: <<-XML.strip - - + body: <<~XML.strip + + XML ) Seahorse::Client::Response.new(context: context) @@ -509,20 +509,20 @@ module S3 context.http_response.signal_done( status_code: 200, headers: {}, - body: <<-XML.strip) - - - a%26 - b%26 - c%26 - d%26 - - e%26 - - - f%26 - - + body: <<~XML.strip) + + + a%26 + b%26 + c%26 + d%26 + + e%26 + + + f%26 + + XML Seahorse::Client::Response.new(context: context) end @@ -546,13 +546,13 @@ module S3 context.http_response.signal_done( status_code: 200, headers: {}, - body: <<-XML.strip) - - - - a%26 - - + body: <<~XML.strip) + + + + a%26 + + XML Seahorse::Client::Response.new(context: context) end @@ -567,23 +567,23 @@ module S3 context.http_response.signal_done( status_code: 200, headers: {}, - body: <<-XML.strip) - - - a%26 - b%26 - c%26 - d%26 - - e%26 - - - f%26 - - - g%26 - - + body: <<~XML.strip) + + + a%26 + b%26 + c%26 + d%26 + + e%26 + + + f%26 + + + g%26 + + XML Seahorse::Client::Response.new(context: context) end @@ -607,20 +607,20 @@ module S3 context.http_response.signal_done( status_code: 200, headers: {}, - body: <<-XML.strip) - - - a%26 - b%26 - c%26 - d%26 - - e%26 - - - f%26 - - + body: <<~XML.strip) + + + a%26 + b%26 + c%26 + d%26 + + e%26 + + + f%26 + + XML Seahorse::Client::Response.new(context: context) end @@ -655,17 +655,17 @@ module S3 ] } ) - expect(resp.context.http_request.body_contents).to eq(<<-XML.gsub(/(^\s+|\n)/, '')) - - - - - name - - READ - - - + expect(resp.context.http_request.body_contents).to eq(<<~XML.gsub(/(^\s+|\n)/, '')) + + + + + name + + READ + + + XML end end @@ -691,18 +691,18 @@ module S3 context.http_response.signal_done( status_code: 200, headers: {}, - body: <<-XML) - - - prefix+suffix - - - prefix%2Bsuffix - - - prefix%20suffix - - + body: <<~XML) + + + prefix+suffix + + + prefix%2Bsuffix + + + prefix%20suffix + + XML Seahorse::Client::Response.new(context: context) end @@ -770,30 +770,30 @@ module S3 end end - { - complete_multipart_upload: { upload_id: 'upload-id' }, - copy_object: { copy_source: 'bucket/key' }, - upload_part_copy: { - upload_id: 'upload-id', - copy_source: 'bucket/key', - part_number: 1 - } - }.each do |operation_name, params| - describe "#{operation_name} response handling" do + describe "200 errors response handling" do + { + complete_multipart_upload: { upload_id: 'upload-id' }, + copy_object: { copy_source: 'bucket/key' }, + upload_part_copy: { + upload_id: 'upload-id', + copy_source: 'bucket/key', + part_number: 1 + } + }.each do |operation_name, params| it 'handles 200 http response errors', rbs_test: :skip do client.handlers.remove( Seahorse::Client::Plugins::RaiseResponseErrors::Handler ) client.handle(step: :send) do |context| context.http_response.signal_headers(200, {}) - context.http_response.signal_data(<<-XML.strip) - - - InternalError - We encountered an internal error. Please try again. - 656c76696e6727732072657175657374 - Uuag1LuByRx9e6j5Onimru9pO4ZVKnJ2Qz7/C1NPcfTWAtRPfTaOFg== - + context.http_response.signal_data(<<~XML.strip) + + + InternalError + We encountered an internal error. Please try again. + 656c76696e6727732072657175657374 + Uuag1LuByRx9e6j5Onimru9pO4ZVKnJ2Qz7/C1NPcfTWAtRPfTaOFg== + XML context.http_response.signal_done Seahorse::Client::Response.new(context: context) @@ -826,6 +826,31 @@ module S3 expect(resp.data).to be(nil) end end + + it 'ignores processing instructions' do + client.handlers.remove( + Seahorse::Client::Plugins::RaiseResponseErrors::Handler + ) + client.handle(step: :send) do |context| + context.http_response.signal_headers(200, {}) + context.http_response.signal_data(<<~XML.strip) + + + + + Key + Code + Message + + + XML + context.http_response.signal_done + Seahorse::Client::Response.new(context: context) + end + resp = client.delete_objects(bucket: 'bucket', delete: { objects: [{key: 'key'}] }) + expect(resp.error).to_not be_kind_of(S3::Errors::InternalError) + expect(resp.data).to_not be nil + end end context 'metadata stubbing' do