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

Net::HTTP adapter doesn't automatically decompress like the real Net::HTTP does #1002

Open
bdewater opened this issue Aug 25, 2022 · 2 comments

Comments

@bdewater
Copy link

bdewater commented Aug 25, 2022

In Ruby 1.9+ Net::HTTP automatically handles gzip and deflate decompression. In the Faraday ecosystem FaradayMiddleware::Gzip is/was a popular way to handle for HTTP clients that don't have built-in support. While working on a gem to remove this middleware from the dependencies (and unblock the Faraday 1.0 -> 2.0 upgrade) I noticed the WebMock adapter behaves differently than the actual Net::HTTP implementation.

The 'real' Net::HTTP#readbody calls #read_body_0. This calls #inflater which wraps the socket to transparently decompress if need be (if not the socket is passed through as-is), the socket is read from and written to the destination buffer. Since WebMock adapter extends the created Net::HTTPResponse object and overrides #readbody, none of this happens inside the test suite for a stubbed response.

I poked at creating a PR but I'm getting a bit lost in what the best place would be (so the next part is rubber ducking myself 😄) to put a fix in:

  • The adapter instantiates Net::HTTPResponse directly but doesn't set the @socket instance variable (normally done via .reading_body), so WebMockHTTPResponse can't just delegate to #inflate.
  • It looks like StubSocket is supposed to stand in for Net::BufferedIO as a socket (which wraps OpenSSL::SSL::SSLSocket, which wraps TCPSocket), but it's missing stuff (eg #read_all) for it to work when used by the deflater.
  • WebMockHTTPResponse could get a slimmed down version of the original #inflate but that feels like a suboptimal way of going about things?
@bdewater
Copy link
Author

@rzane it seems your work in #992 would get us to a place where @socket is actually set 🙌

Then I'd be running into the same "StubSocket isn't a very convincing stub of a socket" problem you mentioned in #999 (comment) - I wonder if using a (subclass of) StringIO instead would make sense? It looks like we could remove directly setting the body ivar and the rest - like decompression - should just work?

@rzane
Copy link
Contributor

rzane commented Aug 28, 2022

Unfortunately #992 was reverted. There were a few problems with that PR. But yes, it sounds like that might work.

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

No branches or pull requests

2 participants