Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions httpx/_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,6 +363,20 @@ def build_request(

[0]: /advanced/clients/#request-instances
"""
# Validate data parameter for better error messages
if data is not None and isinstance(data, list):
# Check if this looks like invalid JSON array (list of dicts/strings)
# but allow valid multipart form data (list of 2-item tuples)
if data and all(
isinstance(item, (dict, str, int, float, bool)) for item in data
):
Comment on lines +370 to +372
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems quite inefficient. Also, logically it should be the inverse as you want to check for 2-item collections: For form data, use a dictionary or a list of 2-item. So whitelist instead of blacklist. For example, list[None] is not raising a TypeError here, and much more.

message = (
"Invalid value for 'data'. To send a JSON array, use the 'json' "
"parameter. For form data, use a dictionary or a list of 2-item "
"tuples."
)
raise TypeError(message)

url = self._merge_url(url)
headers = self._merge_headers(headers)
cookies = self._merge_cookies(cookies)
Expand Down
50 changes: 36 additions & 14 deletions httpx/_content.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,21 @@ def encode_urlencoded_data(
data: RequestData,
) -> tuple[dict[str, str], ByteStream]:
plain_data = []
for key, value in data.items():
if isinstance(value, (list, tuple)):
plain_data.extend([(key, primitive_value_to_str(item)) for item in value])
else:

if isinstance(data, list):
# Handle list of tuples case
for key, value in data:
plain_data.append((key, primitive_value_to_str(value)))
else:
# Handle dictionary case
for key, value in data.items():
if isinstance(value, (list, tuple)):
plain_data.extend(
[(key, primitive_value_to_str(item)) for item in value]
)
else:
plain_data.append((key, primitive_value_to_str(value)))

body = urlencode(plain_data, doseq=True).encode("utf-8")
content_length = str(len(body))
content_type = "application/x-www-form-urlencoded"
Expand Down Expand Up @@ -195,16 +205,28 @@ def encode_request(
returning a two-tuple of (<headers>, <stream>).
"""
if data is not None and not isinstance(data, Mapping):
# We prefer to separate `content=<bytes|str|byte iterator|bytes aiterator>`
# for raw request content, and `data=<form data>` for url encoded or
# multipart form content.
#
# However for compat with requests, we *do* still support
# `data=<bytes...>` usages. We deal with that case here, treating it
# as if `content=<...>` had been supplied instead.
message = "Use 'content=<...>' to upload raw bytes/text content."
warnings.warn(message, DeprecationWarning, stacklevel=2)
return encode_content(data)
# Check if this is a list of tuples (valid form data)
if isinstance(data, list) and (
not data or all(isinstance(item, tuple) and len(item) == 2 for item in data)
):
# This is valid form data as a list of tuples (including empty list)
pass
else:
# We prefer to separate `content=<bytes|str|byte iterator|bytes aiterator>`
# for raw request content, and `data=<form data>` for url encoded or
# multipart form content.
#
# However for compat with requests, we *do* still support
# `data=<bytes...>` usages. We deal with that case here, treating it
# as if `content=<...>` had been supplied instead.
message = "Use 'content=<...>' to upload raw bytes/text content."
warnings.warn(message, DeprecationWarning, stacklevel=2)
# At this point, data is not a list of tuples, so it's safe to pass to
# encode_content
from typing import cast

content_data = cast("RequestContent", data)
return encode_content(content_data)

if content is not None:
return encode_content(content)
Expand Down
14 changes: 9 additions & 5 deletions httpx/_multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,12 +244,16 @@ def __init__(
def _iter_fields(
self, data: RequestData, files: RequestFiles
) -> typing.Iterator[FileField | DataField]:
for name, value in data.items():
if isinstance(value, (tuple, list)):
for item in value:
yield DataField(name=name, value=item)
else:
if isinstance(data, list):
for name, value in data:
yield DataField(name=name, value=value)
else:
for name, value in data.items():
if isinstance(value, (tuple, list)):
for item in value:
yield DataField(name=name, value=item)
else:
yield DataField(name=name, value=value)

file_items = files.items() if isinstance(files, typing.Mapping) else files
for name, value in file_items:
Expand Down
2 changes: 1 addition & 1 deletion httpx/_types.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
ResponseContent = Union[str, bytes, Iterable[bytes], AsyncIterable[bytes]]
ResponseExtensions = Mapping[str, Any]

RequestData = Mapping[str, Any]
RequestData = Union[Mapping[str, Any], List[Tuple[str, Any]]]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR says: Raise helpful TypeError for invalid list 'data', but you seem to add a brand new set of logic for handling new request data type.


FileContent = Union[IO[bytes], bytes, str]
FileTypes = Union[
Expand Down
40 changes: 40 additions & 0 deletions tests/client/test_async_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -373,3 +373,43 @@ async def test_server_extensions(server):
response = await client.get(url)
assert response.status_code == 200
assert response.extensions["http_version"] == b"HTTP/1.1"


INVALID_DATA_FORMATS_ASYNC = [
pytest.param([{"a": "b"}], id="list-of-dicts"),
pytest.param(["a", "b", "c"], id="list-of-strings"),
pytest.param([1, 2, 3], id="list-of-integers"),
]


@pytest.mark.anyio
@pytest.mark.parametrize("invalid_data", INVALID_DATA_FORMATS_ASYNC)
async def test_async_build_request_with_invalid_data_list(invalid_data):
"""
Verify that AsyncClient.build_request raises a helpful TypeError for invalid list
formats.
"""
async with httpx.AsyncClient() as client:
expected_message = (
"Invalid value for 'data'. To send a JSON array, use the 'json' parameter. "
"For form data, use a dictionary or a list of 2-item tuples."
)
with pytest.raises(TypeError, match=expected_message):
client.build_request("POST", "https://example.com", data=invalid_data)


@pytest.mark.anyio
async def test_async_build_request_with_valid_data_formats():
"""
Verify that AsyncClient.build_request accepts valid data formats without raising
our custom TypeError.
"""
async with httpx.AsyncClient() as client:
# Test with a dictionary
request = client.build_request("POST", "https://example.com", data={"a": "b"})
assert isinstance(request, httpx.Request)

# Test with a list of 2-item tuples (for multipart)
# This is a valid use case and should not raise our TypeError.
request = client.build_request("POST", "https://example.com", data=[("a", "b")])
assert isinstance(request, httpx.Request)
39 changes: 39 additions & 0 deletions tests/client/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -460,3 +460,42 @@ def cp1252_but_no_content_type(request):
assert response.reason_phrase == "OK"
assert response.encoding == "ISO-8859-1"
assert response.text == text


INVALID_DATA_FORMATS_SYNC = [
pytest.param([{"a": "b"}], id="list-of-dicts"),
pytest.param(["a", "b", "c"], id="list-of-strings"),
pytest.param([1, 2, 3], id="list-of-integers"),
]


@pytest.mark.parametrize("invalid_data", INVALID_DATA_FORMATS_SYNC)
def test_sync_build_request_with_invalid_data_list(invalid_data):
"""
Verify that Client.build_request raises a helpful TypeError for invalid list
formats.
"""
client = httpx.Client()
expected_message = (
"Invalid value for 'data'. To send a JSON array, use the 'json' parameter. "
"For form data, use a dictionary or a list of 2-item tuples."
)
with pytest.raises(TypeError, match=expected_message):
client.build_request("POST", "https://example.com", data=invalid_data)


def test_sync_build_request_with_valid_data_formats():
"""
Verify that Client.build_request accepts valid data formats without raising our
custom TypeError.
"""
client = httpx.Client()

# Test with a dictionary
request = client.build_request("POST", "https://example.com", data={"a": "b"})
assert isinstance(request, httpx.Request)

# Test with a list of 2-item tuples (for multipart)
# This is a valid use case and should not raise our TypeError.
request = client.build_request("POST", "https://example.com", data=[("a", "b")])
assert isinstance(request, httpx.Request)
49 changes: 49 additions & 0 deletions tests/test_multipart.py
Original file line number Diff line number Diff line change
Expand Up @@ -467,3 +467,52 @@ def test_unicode_with_control_character(self):
files = {"upload": (filename, b"<file content>")}
request = httpx.Request("GET", "https://www.example.com", files=files)
assert expected in request.read()


def test_multipart_with_empty_list_data():
client = httpx.Client(transport=httpx.MockTransport(echo_request_content))
data: list[tuple[str, str]] = []
files = {"file": io.BytesIO(b"<file content>")}
response = client.post("http://127.0.0.1:8000/", data=data, files=files)
boundary = response.request.headers["Content-Type"].split("boundary=")[-1]
boundary_bytes = boundary.encode("ascii")

assert response.status_code == 200
assert response.content == b"".join(
[
b"--" + boundary_bytes + b"\r\n",
b'Content-Disposition: form-data; name="file"; filename="upload"\r\n',
b"Content-Type: application/octet-stream\r\n",
b"\r\n",
b"<file content>\r\n",
b"--" + boundary_bytes + b"--\r\n",
]
)


def test_multipart_with_invalid_list_data():
client = httpx.Client(transport=httpx.MockTransport(echo_request_content))
data: list[str] = ["not-a-tuple"] # Invalid: not a 2-tuple
files = {"file": io.BytesIO(b"<file content>")}
with pytest.raises(TypeError):
client.post("http://127.0.0.1:8000/", data=data, files=files) # type: ignore[arg-type]


def test_multipart_with_tuple_list_data():
client = httpx.Client(transport=httpx.MockTransport(echo_request_content))
data: list[tuple[str, str]] = [("foo", "bar")]
files: dict[str, io.BytesIO] = {}
response = client.post("http://127.0.0.1:8000/", data=data, files=files)
assert response.status_code == 200
assert b"foo" in response.content
assert b"bar" in response.content


def test_multipart_iter_fields_with_tuple_list():
client = httpx.Client(transport=httpx.MockTransport(echo_request_content))
data: list[tuple[str, str]] = [("key", "value")]
files: dict[str, io.BytesIO] = {"dummy": io.BytesIO(b"")}
response = client.post("http://127.0.0.1:8000/", data=data, files=files)
assert response.status_code == 200
assert b"key" in response.content
assert b"value" in response.content