bblimke / webmock

Library for stubbing and setting expectations on HTTP requests in Ruby.
MIT License
3.98k stars 555 forks source link

Webmock stubs treat HEADER keys slightly differently than real Net::HTTP?? #686

Open lastobelus opened 7 years ago

lastobelus commented 7 years ago

WebMock stubs have dasherized keys when using request[xxx], while Net::HTTP responses have underscored keys:

To illustrate here are a real call and a mocked call compared:

=== Real Request ===
------------------  response[] ---------------
1. response[:http_x_shopify_shop_api_call_limit]: nil
2. response['http_x_shopify_shop_api_call_limit']: "1/40"
3. response['http_x_shopify_shop_api_call_limit'.dasherize]: nil
------------------  response.headers[] ---------------
4. response.headers[:http_x_shopify_shop_api_call_limit]: ["1/40"]
5. response.headers['http_x_shopify_shop_api_call_limit']: nil
6. response.headers['http_x_shopify_shop_api_call_limit'.dasherize]: nil

=== Mocked Request ===
------------------  response[] ---------------
1. response[:http_x_shopify_shop_api_call_limit]: nil
2. response['http_x_shopify_shop_api_call_limit']: nil
3. response['http_x_shopify_shop_api_call_limit'.dasherize]: "1/40"
------------------  response.headers[] ---------------
4. response.headers[:http_x_shopify_shop_api_call_limit]: ["1/40"]
5. response.headers['http_x_shopify_shop_api_call_limit']: nil
6. response.headers['http_x_shopify_shop_api_call_limit'.dasherize]: nil

I am using a gem that uses the second form.

Ruby 2.3.1, but the code in the gem that uses response['underscored_header_name'] form is 4 year old code.

bblimke commented 7 years ago

@lastobelus what version of WebMock are you using with VCR? have cassettes been recorded using the same version of WebMock?

andrewsmith commented 6 years ago

I ran into this issue today, using WebMock 3.4.2 and VCR 4.0.0. I tracked this down to some non-standards-conforming behavior in WebMock::Util::Headers#normalize_headers. Specifically, there's this bit of code that attempts to normalize a header name:

name.to_s.split(/_|-/).map { |segment| segment.capitalize }.join("-")

If a header name contains underscores, this will convert those to dashes. The HTTP/1.1 spec only discusses case-insensitivity and whitespace, but doesn't prescribe any other forms of normalization (https://www.w3.org/Protocols/rfc2616/rfc2616-sec4.html). Therefore, underscores should be treated as distinct from dashes.

Perhaps a better header name normalization might simply be:

name.to_s.downcase

Funny enough, my problem was with the same HTTP header as @lastobelus. Shopify produces this unconventionally-named header server-side and consumes it in their Ruby API. Real interactions were working fine, but VCR'd ones weren't.

I'm happy to put together a pull request for this.

andrewsmith commented 6 years ago

After looking at this briefly, I feel this would be difficult to fix without breaking a lot of existing WebMock usage. There are many cases where symbols of the form :content_type are normalized to Content-Type for the purposes of comparing with a request's headers. This change would probably need a new major version and lots of tests/examples would need to be updated.

For my case, I've fixed my issue by setting up a WebMock.after_request hook that mutates the response headers if this header name is present. This appears to work just fine with VCR. It certainly isn't ideal, but both Shopify's choice in header name and WebMock's behavior are at the fringes of acceptability.

animista01 commented 6 years ago

I'm also having this issue, I needed to stub a response with a header key has underscores and webmock is converting them to dashes.