Should we prevent `read(decode_content=True)` followed by `read(decode_content=False)`?

[pquentin]

BaseHTTPResponse.read() has a decode_content parameter to let the user choose between... decoding the content or not. (I'm not sure why the word is "decode" because what we're really doing is decompressing. Anyway.)

Until #2712 and #2798, issuing read() calls with differing values of decode_content would be really weird but you would not loose any data. Now that we have a buffer, issuing decode_content=False after having issued decode_content=True will cause bugs:

• if you have enough decoded data in the buffer for a read(amt, decode_content=False) you will get decoded data
• if you don't have enough data, the existing data in the buffer will be silently ignored and lost

I don't think there's an use case here to support, but should we actively prevent this footgun?

Minimum requirements

💵 You can get paid to complete this issue! Please read the docs for more information.

• Calling HTTPResponse.read(decode_content=True) or HTTPResponse.read(decode_content=False) should continue working as before.
• Calling HTTPResponse.read(decode_content=True) followed by HTTPResponse.read(decode_content=False) should raise a RuntimeError. The other way is fine.
• The requests and botocore integration tests should continue to pass.

[sethmlarson]

I agree this isn't a use-case we should support. Too many chances for data to get stuck in a weird half-decoded half-not-decoded state and us not having a good solution to someone who wants both for likely bad reasons. Users who really want to do something like this can handle decompression on their own. Let's make calling back-and-forth like this a RuntimeError?

[pquentin]

I tried it naively in f894e83 (#2798), but that broke the requests integration tests: https://github.com/urllib3/urllib3/actions/runs/3452169720/jobs/5761894025. @sethmlarson then had the following comment:

@pquentin I wonder if we need to change the call to read() with decode_content=False inside of the "drain" method on HTTPResponse? I know Requests allows for raw data access.