Repository: urllib3/urllib3

Issue:
Should we prevent `read(decode_content=True)` followed by `read(decode_content=False)`?
`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

:dollar: **You can get paid to complete this issue! [Please read the docs for more information](https://urllib3.readthedocs.io/en/latest/contributing.html#getting-paid-for-your-contributions).**

- [ ] 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.

Discussion:

Patch:
diff --git a/changelog/2800.bugfix.rst b/changelog/2800.bugfix.rst
new file mode 100644
index 00000000..3632316a
--- /dev/null
+++ b/changelog/2800.bugfix.rst
@@ -0,0 +1,1 @@
+Changed ``HTTPResponse.read()`` to raise an error when calling with ``decode_content=False`` after using ``decode_content=True`` to prevent data loss.
diff --git a/src/urllib3/response.py b/src/urllib3/response.py
index 1b5f8822..b35f7db8 100644
--- a/src/urllib3/response.py
+++ b/src/urllib3/response.py
@@ -317,6 +317,7 @@ class BaseHTTPResponse(io.IOBase):
         self.version = version
         self.reason = reason
         self.decode_content = decode_content
+        self._has_decoded_content = False
         self._request_url: Optional[str] = request_url
         self.retries = retries
 
@@ -436,11 +437,17 @@ class BaseHTTPResponse(io.IOBase):
         Decode the data passed in and potentially flush the decoder.
         """
         if not decode_content:
+            if self._has_decoded_content:
+                raise RuntimeError(
+                    "Calling read(decode_content=False) is not supported after "
+                    "read(decode_content=True) was called."
+                )
             return data
 
         try:
             if self._decoder:
                 data = self._decoder.decompress(data)
+                self._has_decoded_content = True
         except self.DECODER_ERROR_CLASSES as e:
             content_encoding = self.headers.get("content-encoding", "").lower()
             raise DecodeError(
@@ -891,6 +898,11 @@ class HTTPResponse(BaseHTTPResponse):
         else:
             # do not waste memory on buffer when not decoding
             if not decode_content:
+                if self._has_decoded_content:
+                    raise RuntimeError(
+                        "Calling read(decode_content=False) is not supported after "
+                        "read(decode_content=True) was called."
+                    )
                 return data
 
             decoded_data = self._decode(data, decode_content, flush_decoder)


Test patch:
diff --git a/test/test_response.py b/test/test_response.py
index fba63c3a..025f8641 100644
--- a/test/test_response.py
+++ b/test/test_response.py
@@ -565,6 +565,46 @@ class TestResponse:
         with pytest.raises(ValueError, match="I/O operation on closed file.?"):
             next(reader)
 
+    def test_read_with_illegal_mix_decode_toggle(self) -> None:
+        data = zlib.compress(b"foo")
+
+        fp = BytesIO(data)
+
+        resp = HTTPResponse(
+            fp, headers={"content-encoding": "deflate"}, preload_content=False
+        )
+
+        assert resp.read(1) == b"f"
+
+        with pytest.raises(
+            RuntimeError,
+            match=(
+                r"Calling read\(decode_content=False\) is not supported after "
+                r"read\(decode_content=True\) was called"
+            ),
+        ):
+            resp.read(1, decode_content=False)
+
+        with pytest.raises(
+            RuntimeError,
+            match=(
+                r"Calling read\(decode_content=False\) is not supported after "
+                r"read\(decode_content=True\) was called"
+            ),
+        ):
+            resp.read(decode_content=False)
+
+    def test_read_with_mix_decode_toggle(self) -> None:
+        data = zlib.compress(b"foo")
+
+        fp = BytesIO(data)
+
+        resp = HTTPResponse(
+            fp, headers={"content-encoding": "deflate"}, preload_content=False
+        )
+        assert resp.read(2, decode_content=False) is not None
+        assert resp.read(1, decode_content=True) == b"f"
+
     def test_streaming(self) -> None:
         fp = BytesIO(b"foo")
         resp = HTTPResponse(fp, preload_content=False)