From b6a4c5b547ac0be2b1e31095052b2ba0b592e1ab Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Fri, 12 Nov 2021 12:47:07 -0600 Subject: [PATCH 1/7] feat: allow passing multipart headers --- httpx/_multipart.py | 29 ++++++++++++++++++++++------- httpx/_types.py | 2 ++ tests/test_multipart.py | 25 +++++++++++++++++++++++++ 3 files changed, 49 insertions(+), 7 deletions(-) diff --git a/httpx/_multipart.py b/httpx/_multipart.py index 4dfb838a68..0c23aa9b28 100644 --- a/httpx/_multipart.py +++ b/httpx/_multipart.py @@ -76,23 +76,38 @@ def __init__(self, name: str, value: FileTypes) -> None: fileobj: FileContent + headers: typing.Dict[str, str] = {} + content_type: typing.Optional[str] = None + if isinstance(value, tuple): try: - filename, fileobj, content_type = value # type: ignore + filename, fileobj, content_type, headers = value # type: ignore + headers = {k.title(): v for k, v in headers.items()} + if "Content-Type" in headers: + raise ValueError( + "Content-Type cannot be included in multipart headers" + ) except ValueError: - filename, fileobj = value # type: ignore - content_type = guess_content_type(filename) + try: + filename, fileobj, content_type = value # type: ignore + except ValueError: + filename, fileobj = value # type: ignore else: filename = Path(str(getattr(value, "name", "upload"))).name fileobj = value + + if content_type is None: content_type = guess_content_type(filename) + if content_type is not None: + headers["Content-Type"] = content_type + 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 + self.headers = headers self._consumed = False def get_length(self) -> int: @@ -120,9 +135,9 @@ def render_headers(self) -> bytes: if self.filename: filename = format_form_param("filename", self.filename) parts.extend([b"; ", filename]) - if self.content_type is not None: - content_type = self.content_type.encode() - parts.extend([b"\r\nContent-Type: ", content_type]) + for header_name, header_value in self.headers.items(): + key, val = f"\r\n{header_name}: ".encode(), header_value.encode() + parts.extend([key, val]) parts.append(b"\r\n\r\n") self._headers = b"".join(parts) diff --git a/httpx/_types.py b/httpx/_types.py index 2b08df75cb..1f6462a2f5 100644 --- a/httpx/_types.py +++ b/httpx/_types.py @@ -89,6 +89,8 @@ Tuple[Optional[str], FileContent], # (filename, file (or bytes), content_type) Tuple[Optional[str], FileContent, Optional[str]], + # (filename, file (or bytes), content_type, headers) + Tuple[Optional[str], FileContent, Optional[str], Mapping[str, str]], ] RequestFiles = Union[Mapping[str, FileTypes], Sequence[Tuple[str, FileTypes]]] diff --git a/tests/test_multipart.py b/tests/test_multipart.py index cd71a246b3..7f9b82e881 100644 --- a/tests/test_multipart.py +++ b/tests/test_multipart.py @@ -94,6 +94,31 @@ def test_multipart_file_tuple(): assert multipart["file"] == [b""] +def test_multipart_file_tuple_headers(): + file_name = "test.txt" + content_type = "text/plain" + headers = {"Expires": "0"} + + files = {"file": (file_name, io.BytesIO(b""), content_type, headers)} + with mock.patch("os.urandom", return_value=os.urandom(16)): + boundary = os.urandom(16).hex() + + headers, stream = encode_request(data={}, files=files) + assert isinstance(stream, typing.Iterable) + + content = ( + f'--{boundary}\r\nContent-Disposition: form-data; name="file"; ' + f'filename="{file_name}"\r\nExpires: 0\r\nContent-Type: ' + f"{content_type}\r\n\r\n\r\n--{boundary}--\r\n" + "".encode("ascii") + ) + assert headers == { + "Content-Type": f"multipart/form-data; boundary={boundary}", + "Content-Length": str(len(content)), + } + assert content == b"".join(stream) + + def test_multipart_encode(tmp_path: typing.Any) -> None: path = str(tmp_path / "name.txt") with open(path, "wb") as f: From 01fc2fc742ed6807e506eaedefbac80a4f180e70 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Fri, 12 Nov 2021 12:54:14 -0600 Subject: [PATCH 2/7] Add test for including content-type in headers --- httpx/_multipart.py | 11 ++++++----- tests/test_multipart.py | 25 ++++++++++++++++++++++--- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/httpx/_multipart.py b/httpx/_multipart.py index 0c23aa9b28..3630d164b0 100644 --- a/httpx/_multipart.py +++ b/httpx/_multipart.py @@ -82,16 +82,17 @@ def __init__(self, name: str, value: FileTypes) -> None: if isinstance(value, tuple): try: filename, fileobj, content_type, headers = value # type: ignore - headers = {k.title(): v for k, v in headers.items()} - if "Content-Type" in headers: - raise ValueError( - "Content-Type cannot be included in multipart headers" - ) except ValueError: try: filename, fileobj, content_type = value # type: ignore except ValueError: filename, fileobj = value # type: ignore + else: + headers = {k.title(): v for k, v in headers.items()} + if "Content-Type" in headers: + raise ValueError( + "Content-Type cannot be included in multipart headers" + ) else: filename = Path(str(getattr(value, "name", "upload"))).name fileobj = value diff --git a/tests/test_multipart.py b/tests/test_multipart.py index 7f9b82e881..3247d8868f 100644 --- a/tests/test_multipart.py +++ b/tests/test_multipart.py @@ -94,9 +94,12 @@ def test_multipart_file_tuple(): assert multipart["file"] == [b""] -def test_multipart_file_tuple_headers(): +@pytest.mark.parametrize( + "content_type", [None, "text/plain"] +) +def test_multipart_file_tuple_headers(content_type: typing.Optional[str]): file_name = "test.txt" - content_type = "text/plain" + expected_content_type = "text/plain" headers = {"Expires": "0"} files = {"file": (file_name, io.BytesIO(b""), content_type, headers)} @@ -109,7 +112,7 @@ def test_multipart_file_tuple_headers(): content = ( f'--{boundary}\r\nContent-Disposition: form-data; name="file"; ' f'filename="{file_name}"\r\nExpires: 0\r\nContent-Type: ' - f"{content_type}\r\n\r\n\r\n--{boundary}--\r\n" + f"{expected_content_type}\r\n\r\n\r\n--{boundary}--\r\n" "".encode("ascii") ) assert headers == { @@ -119,6 +122,22 @@ def test_multipart_file_tuple_headers(): assert content == b"".join(stream) +@pytest.mark.parametrize( + "content_type", [None, "text/plain"] +) +@pytest.mark.parametrize( + "headers", [{"content-type": "text/plain"}, {"Content-Type": "text/plain"}, {"CONTENT-TYPE": "text/plain"}] +) +def test_multipart_headers_include_content_type(content_type: typing.Optional[str], headers: typing.Dict[str, str]) -> None: + """Including contet-type in the multipart headers should not be allowed""" + client = httpx.Client(transport=httpx.MockTransport(echo_request_content)) + + files = {"file": ("test.txt", b"content", content_type, headers)} + pat = "Content-Type cannot be included in multipart headers" + with pytest.raises(ValueError, match=pat): + client.post("http://127.0.0.1:8000/", files=files) + + def test_multipart_encode(tmp_path: typing.Any) -> None: path = str(tmp_path / "name.txt") with open(path, "wb") as f: From 79fcd728b49041c2103ab7275ff92a387997fa2c Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Fri, 12 Nov 2021 12:57:20 -0600 Subject: [PATCH 3/7] lint --- tests/test_multipart.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/tests/test_multipart.py b/tests/test_multipart.py index 3247d8868f..8a27caebcc 100644 --- a/tests/test_multipart.py +++ b/tests/test_multipart.py @@ -94,9 +94,7 @@ def test_multipart_file_tuple(): assert multipart["file"] == [b""] -@pytest.mark.parametrize( - "content_type", [None, "text/plain"] -) +@pytest.mark.parametrize("content_type", [None, "text/plain"]) def test_multipart_file_tuple_headers(content_type: typing.Optional[str]): file_name = "test.txt" expected_content_type = "text/plain" @@ -122,13 +120,18 @@ def test_multipart_file_tuple_headers(content_type: typing.Optional[str]): assert content == b"".join(stream) +@pytest.mark.parametrize("content_type", [None, "text/plain"]) @pytest.mark.parametrize( - "content_type", [None, "text/plain"] -) -@pytest.mark.parametrize( - "headers", [{"content-type": "text/plain"}, {"Content-Type": "text/plain"}, {"CONTENT-TYPE": "text/plain"}] + "headers", + [ + {"content-type": "text/plain"}, + {"Content-Type": "text/plain"}, + {"CONTENT-TYPE": "text/plain"}, + ], ) -def test_multipart_headers_include_content_type(content_type: typing.Optional[str], headers: typing.Dict[str, str]) -> None: +def test_multipart_headers_include_content_type( + content_type: typing.Optional[str], headers: typing.Dict[str, str] +) -> None: """Including contet-type in the multipart headers should not be allowed""" client = httpx.Client(transport=httpx.MockTransport(echo_request_content)) From ec474fd14ebf5153981cc1c1ab9ff7ab1605880e Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Mon, 10 Jan 2022 08:34:24 -0800 Subject: [PATCH 4/7] override content_type with headers --- httpx/_multipart.py | 15 ++++++++++----- tests/test_multipart.py | 41 +++++++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/httpx/_multipart.py b/httpx/_multipart.py index a40e28570b..76dbb7f5cf 100644 --- a/httpx/_multipart.py +++ b/httpx/_multipart.py @@ -81,20 +81,22 @@ def __init__(self, name: str, value: FileTypes) -> None: headers: typing.Dict[str, str] = {} content_type: typing.Optional[str] = None + # This large tuple based API largely mirror's requests' API + # It would be good to think of better APIs for this that we could include in httpx 2.0 + # since variable length tuples (especially of 4 elements) are quite unwieldly if isinstance(value, tuple): try: filename, fileobj, content_type, headers = value # type: ignore except ValueError: + # 4th parameter (headers) not included try: filename, fileobj, content_type = value # type: ignore except ValueError: + # neither the 3rd parameter (content_type) nor the 4th (headers) was included filename, fileobj = value # type: ignore else: + # corresponds to (filename, fileobj, content_type, headers) headers = {k.title(): v for k, v in headers.items()} - if "Content-Type" in headers: - raise ValueError( - "Content-Type cannot be included in multipart headers" - ) else: filename = Path(str(getattr(value, "name", "upload"))).name fileobj = value @@ -102,7 +104,10 @@ def __init__(self, name: str, value: FileTypes) -> None: if content_type is None: content_type = guess_content_type(filename) - if content_type is not None: + if content_type is not None and "Content-Type" not in headers: + # note that unlike requests, we ignore the content_type + # provided in the 3rd tuple element if it is also included in the headers + # requests does the opposite headers["Content-Type"] = content_type if isinstance(fileobj, (str, io.StringIO)): diff --git a/tests/test_multipart.py b/tests/test_multipart.py index 8a27caebcc..9980cb5b4e 100644 --- a/tests/test_multipart.py +++ b/tests/test_multipart.py @@ -120,25 +120,30 @@ def test_multipart_file_tuple_headers(content_type: typing.Optional[str]): assert content == b"".join(stream) -@pytest.mark.parametrize("content_type", [None, "text/plain"]) -@pytest.mark.parametrize( - "headers", - [ - {"content-type": "text/plain"}, - {"Content-Type": "text/plain"}, - {"CONTENT-TYPE": "text/plain"}, - ], -) -def test_multipart_headers_include_content_type( - content_type: typing.Optional[str], headers: typing.Dict[str, str] -) -> None: - """Including contet-type in the multipart headers should not be allowed""" - client = httpx.Client(transport=httpx.MockTransport(echo_request_content)) +def test_multipart_headers_include_content_type() -> None: + """Content-Type from 4th tuple parameter (headers) should override the 3rd parameter (content_type)""" + file_name = "test.txt" + expected_content_type = "image/png" + headers = {"Content-Type": "image/png"} + + files = {"file": (file_name, io.BytesIO(b""), "text_plain", headers)} + with mock.patch("os.urandom", return_value=os.urandom(16)): + boundary = os.urandom(16).hex() - files = {"file": ("test.txt", b"content", content_type, headers)} - pat = "Content-Type cannot be included in multipart headers" - with pytest.raises(ValueError, match=pat): - client.post("http://127.0.0.1:8000/", files=files) + headers, stream = encode_request(data={}, files=files) + assert isinstance(stream, typing.Iterable) + + content = ( + f'--{boundary}\r\nContent-Disposition: form-data; name="file"; ' + f'filename="{file_name}"\r\nContent-Type: ' + f"{expected_content_type}\r\n\r\n\r\n--{boundary}--\r\n" + "".encode("ascii") + ) + assert headers == { + "Content-Type": f"multipart/form-data; boundary={boundary}", + "Content-Length": str(len(content)), + } + assert content == b"".join(stream) def test_multipart_encode(tmp_path: typing.Any) -> None: From 79d1521f319ad597b54eb42b9b13f3b14ad95617 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Tue, 11 Jan 2022 10:17:00 -0800 Subject: [PATCH 5/7] compare tuples based on length --- httpx/_multipart.py | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/httpx/_multipart.py b/httpx/_multipart.py index 76dbb7f5cf..2751f46fc6 100644 --- a/httpx/_multipart.py +++ b/httpx/_multipart.py @@ -85,17 +85,14 @@ def __init__(self, name: str, value: FileTypes) -> None: # It would be good to think of better APIs for this that we could include in httpx 2.0 # since variable length tuples (especially of 4 elements) are quite unwieldly if isinstance(value, tuple): - try: - filename, fileobj, content_type, headers = value # type: ignore - except ValueError: - # 4th parameter (headers) not included - try: - filename, fileobj, content_type = value # type: ignore - except ValueError: - # neither the 3rd parameter (content_type) nor the 4th (headers) was included - filename, fileobj = value # type: ignore + if len(value) == 2: + # neither the 3rd parameter (content_type) nor the 4th (headers) was included + filename, fileobj = value # type: ignore + elif len(value) == 3: + filename, fileobj, content_type = value # type: ignore else: - # corresponds to (filename, fileobj, content_type, headers) + # all 4 parameters included + filename, fileobj, content_type, headers = value # type: ignore headers = {k.title(): v for k, v in headers.items()} else: filename = Path(str(getattr(value, "name", "upload"))).name @@ -107,7 +104,7 @@ def __init__(self, name: str, value: FileTypes) -> None: if content_type is not None and "Content-Type" not in headers: # note that unlike requests, we ignore the content_type # provided in the 3rd tuple element if it is also included in the headers - # requests does the opposite + # requests does the opposite (it overwrites the header with the 3rd tuple element) headers["Content-Type"] = content_type if isinstance(fileobj, (str, io.StringIO)): From 195c661aff204688df2f68d51a787da4c929d663 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 12 Jan 2022 08:43:19 -0800 Subject: [PATCH 6/7] incorporate suggestion --- httpx/_multipart.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/httpx/_multipart.py b/httpx/_multipart.py index 2751f46fc6..6e26100e2f 100644 --- a/httpx/_multipart.py +++ b/httpx/_multipart.py @@ -97,11 +97,12 @@ def __init__(self, name: str, value: FileTypes) -> None: else: filename = Path(str(getattr(value, "name", "upload"))).name fileobj = value - + if content_type is None: content_type = guess_content_type(filename) - if content_type is not None and "Content-Type" not in headers: + has_content_type_header = any("content-type" in key.lower() for key in headers) + if content_type is not None and not has_content_type_header: # note that unlike requests, we ignore the content_type # provided in the 3rd tuple element if it is also included in the headers # requests does the opposite (it overwrites the header with the 3rd tuple element) From 1c9dc26dfaaa94aa97f7ad6037533f44583149b4 Mon Sep 17 00:00:00 2001 From: Adrian Garcia Badaracco <1755071+adriangb@users.noreply.github.com> Date: Wed, 12 Jan 2022 08:44:31 -0800 Subject: [PATCH 7/7] remove .title() on headers --- httpx/_multipart.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/httpx/_multipart.py b/httpx/_multipart.py index 6e26100e2f..34ee631557 100644 --- a/httpx/_multipart.py +++ b/httpx/_multipart.py @@ -93,11 +93,10 @@ def __init__(self, name: str, value: FileTypes) -> None: else: # all 4 parameters included filename, fileobj, content_type, headers = value # type: ignore - headers = {k.title(): v for k, v in headers.items()} else: filename = Path(str(getattr(value, "name", "upload"))).name fileobj = value - + if content_type is None: content_type = guess_content_type(filename)