Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix transfer-encoding header for requests using AsyncHttpClientAdapter raise exception #1044

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tmertens
Copy link

@tmertens tmertens commented Dec 5, 2023

When mocking requests using webmock where the request has a transfer-encoding header with a value of chunked, the request will raise an exception:

Protocol::HTTP1::BadRequest:
       Message contains both transfer encoding and content length!

This happens because the data is not actually chunked, so the underlying Protocol::HTTP1 library attempts to send a fixed length body in the response, which injects the content-length header after the transfer-encoding header was already sent.

This PR simply copies what was done to address a similar problem in WebMock::HttpLibAdapters::EmHttpRequestAdapter - simply remove the transfer-encoding header when present in the mocked response.

@tmertens tmertens changed the title Strip transfer-encoding header for requests using async_http_client_a… Fix transfer-encoding header for requests using AsyncHttpClientAdapter raise exception Dec 5, 2023
@bblimke
Copy link
Owner

bblimke commented Feb 4, 2024

@tmertens thank you for investigating and explaining the issue and for the PR.

I appreciate the proposed fix, but I'm uncertain if it's the right approach. As far as I can see, encountering this error requires manually declaring the stubbed response with a 'Transfer-Encoding' => 'chunked' header. If a developer took the effort to include this header, there might be a specific reason behind it. Consequently, silently removing a header that was explicitly declared in the code feels a bit concerning to me.

Instead, could we consider raising an error with a clear message when encountering stubbed responses with 'Transfer-Encoding' => 'chunked' header, explaining that such headers are not supported and need to be removed from the stubbed response declaration? This way, we provide a more explicit indication to the user about the limitation and guide them on how to resolve it.

Looking forward to your thoughts on this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants