aws-sdk-ruby icon indicating copy to clipboard operation
aws-sdk-ruby copied to clipboard

SESV2 ListContacts breaks with "Content-Length not given and Transfer-Encoding is not `chunked'"

Open mbland opened this issue 1 year ago • 8 comments

Describe the bug

The ListContacts SESV2 API call uses the GET method, but includes a JSON body. However, Seahorse::Client::Plugins::ContentLength::Handler.call() doesn't set the Content-Length HTTP header for GET requests. This causes the resulting HTTP request to fail with:

net/http/generic_request.rb:195:in `send_request_with_body_stream': Content-Length not given and Transfer-Encoding is not `chunked' (ArgumentError)

Expected Behavior

Calling Aws::SESV2::Client.new.list_contacts should return a list of contacts from a valid SES contact list.

If the method worked as expected, the Reproduction Steps script would instead fail with: List with name: TestList doesn't exist. (Aws::SESV2::Errors::NotFoundException).

Current Behavior

Calling Aws::SESV2::Client.new.list_contacts in the Reproduction Steps script creates a {} request body, producing the stack trace below. An existing SES contact list and a filter spec (which would produce more JSON body content) isn't necessary.

The short version:

~/.rbenv/versions/3.2.1/lib/ruby/3.2.0/net/http/generic_request.rb:195:in `send_request_with_body_stream': Content-Length not given and Transfer-Encoding is not `chunked' (ArgumentError)
  from ~/.rbenv/versions/3.2.1/lib/ruby/3.2.0/net/http/generic_request.rb:126:in `exec'
  [...snip...]
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/request.rb:72:in `send_request'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-sesv2-1.32.0/lib/aws-sdk-sesv2/client.rb:2418:in `list_contacts'
  from ./list-contacts.rb:6:in `<main>'

The full version:

~/.rbenv/versions/3.2.1/lib/ruby/3.2.0/net/http/generic_request.rb:195:in `send_request_with_body_stream': Content-Length not given and Transfer-Encoding is not `chunked' (ArgumentError)
  from ~/.rbenv/versions/3.2.1/lib/ruby/3.2.0/net/http/generic_request.rb:126:in `exec'
  from ~/.rbenv/versions/3.2.1/lib/ruby/3.2.0/net/http.rb:1855:in `block in transport_request'
  from ~/.rbenv/versions/3.2.1/lib/ruby/3.2.0/net/http.rb:1853:in `catch'
  from ~/.rbenv/versions/3.2.1/lib/ruby/3.2.0/net/http.rb:1853:in `transport_request'
  from ~/.rbenv/versions/3.2.1/lib/ruby/3.2.0/net/http.rb:1826:in `request'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/net_http/connection_pool.rb:348:in `request'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/net_http/handler.rb:79:in `block in transmit'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/net_http/handler.rb:133:in `block in session'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/net_http/connection_pool.rb:104:in `session_for'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/net_http/handler.rb:128:in `session'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/net_http/handler.rb:76:in `transmit'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/net_http/handler.rb:50:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/plugins/content_length.rb:24:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/plugins/request_callback.rb:87:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/json/error_handler.rb:10:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/sign.rb:49:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/transfer_encoding.rb:26:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/helpful_socket_errors.rb:12:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/retry_errors.rb:360:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/http_checksum.rb:19:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/endpoint_pattern.rb:30:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/checksum_algorithm.rb:136:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/protocols/rest_json.rb:18:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/rest/handler.rb:10:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/recursion_detection.rb:18:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/user_agent.rb:13:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-sesv2-1.32.0/lib/aws-sdk-sesv2/plugins/endpoints.rb:41:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/endpoint_discovery.rb:84:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/plugins/endpoint.rb:47:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/param_validator.rb:26:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/plugins/raise_response_errors.rb:16:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/checksum_algorithm.rb:111:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/jsonvalue_converter.rb:16:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/idempotency_token.rb:19:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/param_converter.rb:26:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/plugins/request_callback.rb:71:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/aws-sdk-core/plugins/response_paging.rb:12:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/plugins/response_target.rb:24:in `call'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-core-3.170.1/lib/seahorse/client/request.rb:72:in `send_request'
  from ~/.rbenv/versions/3.2.1/lib/ruby/gems/3.2.0/gems/aws-sdk-sesv2-1.32.0/lib/aws-sdk-sesv2/client.rb:2418:in `list_contacts'
  from ./list-contacts.rb:6:in `<main>'

Reproduction Steps

#!/usr/bin/env ruby

require 'aws-sdk-sesv2'


Aws::SESV2::Client.new.list_contacts(contact_list_name: "TestList").each do |r|
  r.contacts.each {|c| puts c.to_s}
end

Possible Solution

It would seem worth updating Seahorse::Client::Plugins::ContentLength::Handler.call() to always set Content-Length when body.respond_to?(:size) && body.size != 0 is true.

Depending on the complexity of the proper solution, I'm happy to file a PR myself. Either way, I'm in no rush, as I have a hacky but effective workaround for my personal project.


Then again, as noted in Additional Information/Context, having a body in GET request is an ambiguous use case. It would seem "more correct" to update the API to use URL params (which would require some massaging) or the POST method instead.

However, I'm sure there are many clients of the published API, which would render updating the API spec impractical (before the next major version, anyway). Also, I tried searching several times for this error before diving in, and I seem to be the only person in the world calling ListContacts via the Ruby SDK right now.

Additional Information/Context

The ListContacts API call documentation shows the GET method with a JSON body:

GET /v2/email/contact-lists/ContactListName/contacts?NextToken=NextToken&PageSize=PageSize HTTP/1.1
Content-type: application/json

{
   "Filter": { 
      "FilteredStatus": "string",
      "TopicFilter": { 
         "TopicName": "string",
         "UseDefaultIfPreferenceUnavailable": boolean
      }
   }
}

Here's the current implementation of Seahorse::Client::Plugins::ContentLength::Handler.call() from aws-sdk-core-3.170.1/lib/seahorse/client/plugins/content_length.rb, which doesn't set the Content-Length HTTP header for GET requests:

https://github.com/aws/aws-sdk-ruby/blob/cfe7ba3e032b26329225d4eb409ff56457b20cfd/gems/aws-sdk-core/lib/seahorse/client/plugins/content_length.rb#L9-L26

I was able to instrument this method to see what was happening, and hack it to send a successful ListContacts request (by commenting out the !METHODS_WITHOUT_BODY expression):

  def call(context)
    body = context.http_request.body
    method = context.http_request.http_method
    puts "CALLING CONTENT LENGTH - METHOD: #{method} - BODY: #{body.string}"
    puts "PARAMS: #{context.params}"
    # We use Net::HTTP with body_stream which doesn't do this by default
    if body.respond_to?(:size) # && !METHODS_WITHOUT_BODY.include?(method)
      context.http_request.headers['Content-Length'] = body.size
    end 
    @handler.call(context)
  end 

I would see this before the successful call in my actual code, which included a Filter:

CALLING CONTENT LENGTH - METHOD: GET - BODY: {"Filter":{"FilteredStatus":"OPT_IN","TopicFilter":{"TopicName":"BlogPosts","UseDefaultIfPreferenceUnavailable":true}}}
PARAMS: {:contact_list_name=>"MikeBlandsBlog", :filter=>{:filtered_status=>"OPT_IN", :topic_filter=>{:topic_name=>"BlogPosts", :use_default_if_preference_unavailable=>true}}}

FWIW, including a body in a GET request is apparently an ambiguous use case, per https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods/GET:

Note: Sending body/payload in a GET request may cause some existing implementations to reject the request — while not prohibited by the specification, the semantics are undefined. It is better to just avoid sending payloads in GET requests.

Further discussion:

  • https://stackoverflow.com/a/983458
  • https://stackoverflow.com/a/63190922
  • https://stackoverflow.com/a/67890542

The current API spec in this repo: https://github.com/aws/aws-sdk-ruby/blob/049b16eedb441597ebac394965515a6642501b49/apis/sesv2/2019-09-27/api-2.json#L643-L656

Gem name ('aws-sdk', 'aws-sdk-resources' or service gems like 'aws-sdk-s3') and its version

aws-sdk-sesv2 (1.32.0), aws-sdk-core (3.170.1)

Environment details (Version of Ruby, OS environment)

ruby 3.2.1 (2023-02-08 revision 31819e82c8) [arm64-darwin22], macOS 13.2.1

mbland avatar Mar 21 '23 00:03 mbland

Thanks for the detailed report. I agree it's broken - and you are possibly the first one to find it (many customers still use SES v1). We deliberately blocked Content-Length for GET methods because it is ambiguous / un-defined / not intended and is actually not recommended in our AWS API standards. But alas, here we are, and there are some teams/services where this slipped in, and even did so prior to that guidance.

Let me follow up with the team to see if we should write in a hack or if we can get the SES team to fix their modeling. We are generating a default body of {} by inspecting the model - it checks for any shape/member that does not have a location (no location = body). The other members in this operation are bound to the URI or a query string, but this one is not, so it creates the body, but then the corresponding Content-Length is not provided.

In the mean time, you can certainly monkey patch METHODS_WITHOUT_BODY to remove GET.

mullermp avatar Mar 21 '23 01:03 mullermp

@mullermp Thanks for following up so quickly!

BTW, I'm not yet following "We are generating a default body of {} by inspecting the model..." explanation exactly yet, and I have a question about "The other members in this operation are bound to the URI or a query string..."

In my original code, I was also passing a Filter value. Should Filter be bound to a query string? Because in my experiment below, all the other request options are bound to the URI or the query string, but Filter always shows up as a JSON body.

If I update the script as follows (looking more like the list_contacts API documentation)...

#!/usr/bin/env ruby

require 'aws-sdk-sesv2'

options = {
  contact_list_name: "TestList",
  filter: {
    filtered_status: "OPT_IN",
    topic_filter: {
      topic_name: "TestTopic",
      use_default_if_preference_unavailable: true,
    },
  },
  page_size: 1,
  next_token: "NextToken",
}

Aws::SESV2::Client.new.list_contacts(options).each do |r|
  r.contacts.each {|c| puts c.to_s}
end

...and hack this into net/http/generic_request.rb:

def send_request_with_body_stream(sock, ver, path, f)                         
  puts "PATH: #{path}\nBODY:\n#{f.string}"

...the script produces the following output before the stack trace:

PATH: /v2/email/contact-lists/TestList/contacts?PageSize=1&NextToken=NextToken                                                                                   
BODY:                                                                                                                                                            
{"Filter":{"FilteredStatus":"OPT_IN","TopicFilter":{"TopicName":"TestTopic","UseDefaultIfPreferenceUnavailable":true}}}

mbland avatar Mar 21 '23 02:03 mbland

The Ruby SDK is placing Filter in the body correctly because that's what the model says to do. But it is a GET operation with a modeled body, which is incorrect. The Ruby SDK is correctly inspecting the model and seeing at least one member (Filter) and generating a default body {} when Filter is not provided (logic is here), because AWS rest-json protocol requires at least an empty object. When Filter is provided, it's in the body, as you have seen. Whether Filter should be a query string, header, etc, is up to the service/modeling - it definitely should NOT be on the body unless this method is a POST/PUT.

I talked with other teams on this and apparently this is a known issue. The operation is just broken for Java SDK, .NET, and others. After talking with our leadership here, I am going to re-raise this to the SES team with high severity. The service model change is not trivial. To unblock you, I recommend monkey patching METHODS_WITHOUT_BODY (re-define it) without GET. Ruby will allow GET with body, but Java does not for example.

mullermp avatar Mar 21 '23 15:03 mullermp

That makes perfect sense now, thanks! And thanks for taking the issue up the chain.

mbland avatar Mar 21 '23 16:03 mbland

Has there been any progress on this? This is quite a big deal if anyone is thinking about using the SDK for synching a contact list (ie remove the deleted contacts from a list would require fetching the contacts within a list first).

Are there any other ways for synching a contact list? Otherwise, I guess the only option is to write this method outside of the SDK for now using HTTParty or Faraday directly.

khash avatar Jul 13 '23 14:07 khash

Checking the internal ticket I created, it looks like estimated fix is this month some time. I'll check in and see the progress. In the mean time, you can use the monkey patch method above.

mullermp avatar Jul 13 '23 17:07 mullermp

@mullermp thanks for the explanation, I just ran into this. Any news?

chuckmitchell avatar Feb 06 '24 16:02 chuckmitchell

None yet, annoyingly. I am going to make noise on this one, because it's been lingering again for too long.

mullermp avatar Feb 06 '24 18:02 mullermp

This looks to be fixed as of today. Please give it a try and let me know if there are any issues. https://github.com/aws/aws-sdk-ruby/commit/d64f04b0d4a48051ea6e19ea062c3f25b51417bd#diff-5c4ea219c21cf488f5de98b490f875ad38db14867dcad78c54539a7328b54b2cR7

mullermp avatar May 01 '24 21:05 mullermp

This issue is now closed. Comments on closed issues are hard for our team to see. If you need more assistance, please open a new issue that references this one.

github-actions[bot] avatar May 01 '24 21:05 github-actions[bot]