Repository: encode/httpx (python)

Issue:
Enforce that upload files must be binary, not text.
### Based on this discussion... https://github.com/encode/httpx/discussions/1704

<div type='discussions-op-text'>

<sup>Originally posted by **plotski** June 21, 2021</sup>
I just spent several hours debugging `LocalProtocolError("Too much data for declared Content-Length")`. I've figured out what's going on, but I don't know what the best fix would be or if this is entirely my own fault.

I was passing a `StringIO` object to a POST request, e.g.:

```python
>>> files = {'upload-file': ('foo', StringIO('cöntént of föö'))}
>>> r = httpx.post("https://httpbin.org/post", files=files)
```

The `StringIO` object ends up in `FileField.get_length()`, which calls `peek_filelike_length()`, which correctly reports the the length of the string. However, we must know the length of the string as `bytes` because that's what we need for the POST request.

The fix is very simple: Use `BytesIO` instead of `StringIO`. But the error message was extremely unhelpful.

Maybe `FileField` shouldn't accept a `StringIO` object? Or maybe it should convert to `bytes` first? Or maybe this is really obvious for anyone with more experience and there's no need to fix it?</div>

I'm fairly sure that the most sensible thing to do here is to raise an error on:

* Files opened in text mode.
* `StringIO` instances, used instead of `BytesIO` instances.

Pull request:
enforce-upload-files-binary-type
Fixes #1706

Forces `FileContent` to be `bytes` or `BytesIO` instances. For `str` and `StringIO` instances a `TypeError` is raised.

Patch:
diff --git a/httpx/_multipart.py b/httpx/_multipart.py
index 36bae66..683e6f1 100644
--- a/httpx/_multipart.py
+++ b/httpx/_multipart.py
@@ -1,4 +1,5 @@
 import binascii
+import io
 import os
 import typing
 from pathlib import Path
@@ -81,6 +82,9 @@ class FileField:
             fileobj = value
             content_type = guess_content_type(filename)
 
+        if isinstance(fileobj, str) or isinstance(fileobj, io.StringIO):
+            raise TypeError(f"Expected bytes or bytes-like object got: {type(fileobj)}")
+
         self.filename = filename
         self.file = fileobj
         self.content_type = content_type
diff --git a/httpx/_types.py b/httpx/_types.py
index 75bb900..2381996 100644
--- a/httpx/_types.py
+++ b/httpx/_types.py
@@ -79,7 +79,7 @@ ResponseContent = Union[str, bytes, Iterable[bytes], AsyncIterable[bytes]]
 
 RequestData = dict
 
-FileContent = Union[IO[str], IO[bytes], str, bytes]
+FileContent = Union[IO[bytes], bytes]
 FileTypes = Union[
     # file (or text)
     FileContent,


Test patch:
diff --git a/tests/test_multipart.py b/tests/test_multipart.py
index 3824fb6..cd71a24 100644
--- a/tests/test_multipart.py
+++ b/tests/test_multipart.py
@@ -139,7 +139,7 @@ def test_multipart_encode(tmp_path: typing.Any) -> None:
 
 
 def test_multipart_encode_unicode_file_contents() -> None:
-    files = {"file": ("name.txt", "<únicode string>")}
+    files = {"file": ("name.txt", b"<bytes content>")}
 
     with mock.patch("os.urandom", return_value=os.urandom(16)):
         boundary = os.urandom(16).hex()
@@ -150,7 +150,7 @@ def test_multipart_encode_unicode_file_contents() -> None:
         content = (
             '--{0}\r\nContent-Disposition: form-data; name="file";'
             ' filename="name.txt"\r\n'
-            "Content-Type: text/plain\r\n\r\n<únicode string>\r\n"
+            "Content-Type: text/plain\r\n\r\n<bytes content>\r\n"
             "--{0}--\r\n"
             "".format(boundary).encode("utf-8")
         )
@@ -212,14 +212,8 @@ def test_multipart_encode_files_guesses_correct_content_type(
         assert content == b"".join(stream)
 
 
-@pytest.mark.parametrize(
-    "value, output",
-    ((b"<bytes content>", "<bytes content>"), ("<string content>", "<string content>")),
-)
-def test_multipart_encode_files_allows_bytes_or_str_content(
-    value: typing.Union[str, bytes], output: str
-) -> None:
-    files = {"file": ("test.txt", value, "text/plain")}
+def test_multipart_encode_files_allows_bytes_content() -> None:
+    files = {"file": ("test.txt", b"<bytes content>", "text/plain")}
     with mock.patch("os.urandom", return_value=os.urandom(16)):
         boundary = os.urandom(16).hex()
 
@@ -229,9 +223,9 @@ def test_multipart_encode_files_allows_bytes_or_str_content(
         content = (
             '--{0}\r\nContent-Disposition: form-data; name="file"; '
             'filename="test.txt"\r\n'
-            "Content-Type: text/plain\r\n\r\n{1}\r\n"
+            "Content-Type: text/plain\r\n\r\n<bytes content>\r\n"
             "--{0}--\r\n"
-            "".format(boundary, output).encode("ascii")
+            "".format(boundary).encode("ascii")
         )
         assert headers == {
             "Content-Type": f"multipart/form-data; boundary={boundary}",
@@ -240,6 +234,22 @@ def test_multipart_encode_files_allows_bytes_or_str_content(
         assert content == b"".join(stream)
 
 
+def test_multipart_encode_files_raises_exception_with_str_content() -> None:
+    files = {"file": ("test.txt", "<bytes content>", "text/plain")}
+    with mock.patch("os.urandom", return_value=os.urandom(16)):
+
+        with pytest.raises(TypeError):
+            encode_request(data={}, files=files)  # type: ignore
+
+
+def test_multipart_encode_files_raises_exception_with_StringIO_content() -> None:
+    files = {"file": ("test.txt", io.StringIO("content"), "text/plain")}
+    with mock.patch("os.urandom", return_value=os.urandom(16)):
+
+        with pytest.raises(TypeError):
+            encode_request(data={}, files=files)  # type: ignore
+
+
 def test_multipart_encode_non_seekable_filelike() -> None:
     """
     Test that special readable but non-seekable filelike objects are supported,