Skip to content

Commit 4d6baa7

Browse files
authored
fix: Let httpx2 choose the multipart/form-data boundary (#1511)
Fixes #1509 Since #1005 every generated `multipart/form-data` request sets `Content-Type: multipart/form-data; boundary=+++`. httpx2 builds the body with that boundary, so a file that contains `\r\n--+++` is split there and the server gets a truncated file plus a stray part. This stops setting `Content-Type` for `multipart/form-data` bodies. httpx2 then adds the header with its own random boundary, the same way single-body endpoints worked before 0.29.0. For endpoints with several body types, the multipart branch no longer sets the header either, so #947 stays fixed: httpx2 still writes a boundary because the header isn't set. The other content types still set their header explicitly. Changes: - `endpoint_module.py.jinja`: skip the `Content-Type` line for multipart bodies. - Golden records and the integration-test client regenerated (only the `boundary=+++` line goes away). - New functional test `test_multipart_bodies.py`: generates a single-body and a multi-body endpoint, sends a file containing `\r\n--+++` through a `MockTransport` and parses the multipart body back. - Patch changeset. Testing: - `pytest end_to_end_tests/functional_tests/generated_code_execution/test_multipart_bodies.py` - before the template change: 2 failed (`{'file': b'first line'} != {'file': b'first line\r\n--+++\r\nstill the same file\r\n'}`), 1 passed (JSON control) - after: 3 passed - Full `pdm test` equivalent with coverage: 471 passed, 100% coverage, 5 snapshots passed - `ruff format --check .`, `ruff check .`, `mypy openapi_python_client fuzz` pass I didn't run the `integration` job locally because it needs the openapi-test-server container.
1 parent a92a25d commit 4d6baa7

7 files changed

Lines changed: 126 additions & 14 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
default: patch
3+
---
4+
5+
# Let httpx2 choose the boundary for `multipart/form-data` requests
6+
7+
Since 0.29.0, generated endpoints sent every `multipart/form-data` body with the fixed boundary `+++`, so an uploaded file containing `\r\n--+++` was cut short at that point (#1509). These bodies no longer set `Content-Type`, so httpx2 sets it with a random boundary for each request, as single-body endpoints did before 0.29.0. Endpoints with several body types still get a boundary (#947).
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
import email.parser
2+
import email.policy
3+
import io
4+
5+
import httpx2
6+
7+
from end_to_end_tests.functional_tests.helpers import (
8+
with_generated_client_fixture,
9+
with_generated_code_imports,
10+
)
11+
12+
# Content that would end the part early if the request used a fixed "+++" boundary
13+
FILE_CONTENT = b"first line\r\n--+++\r\nstill the same file\r\n"
14+
15+
16+
def _send_and_capture(Client, sync_detailed, body) -> httpx2.Request:
17+
sent: list[httpx2.Request] = []
18+
19+
def handler(request: httpx2.Request) -> httpx2.Response:
20+
request.read()
21+
sent.append(request)
22+
return httpx2.Response(200)
23+
24+
client = Client(base_url="http://test", httpx_args={"transport": httpx2.MockTransport(handler)})
25+
sync_detailed(client=client, body=body)
26+
assert len(sent) == 1
27+
return sent[0]
28+
29+
30+
def _parse_parts(request: httpx2.Request) -> dict[str, bytes]:
31+
content_type = request.headers["content-type"]
32+
assert content_type.startswith("multipart/form-data; boundary=")
33+
message = email.parser.BytesParser(policy=email.policy.HTTP).parsebytes(
34+
b"Content-Type: " + content_type.encode() + b"\r\n\r\n" + request.content
35+
)
36+
return {
37+
part.get_param("name", header="content-disposition"): part.get_payload(decode=True)
38+
for part in message.iter_parts()
39+
}
40+
41+
42+
@with_generated_client_fixture(
43+
"""
44+
paths:
45+
/upload:
46+
post:
47+
operationId: upload
48+
requestBody:
49+
required: true
50+
content:
51+
multipart/form-data:
52+
schema:
53+
type: object
54+
properties:
55+
file: {type: string, format: binary}
56+
responses:
57+
"200": {description: OK}
58+
"""
59+
)
60+
@with_generated_code_imports(
61+
".api.default.upload.sync_detailed",
62+
".client.Client",
63+
".models.UploadBody",
64+
".types.File",
65+
)
66+
class TestSingleMultipartBody:
67+
def test_file_content_is_not_split_by_boundary(self, sync_detailed, Client, UploadBody, File):
68+
body = UploadBody(file=File(payload=io.BytesIO(FILE_CONTENT), file_name="a.txt"))
69+
70+
request = _send_and_capture(Client, sync_detailed, body)
71+
72+
assert _parse_parts(request) == {"file": FILE_CONTENT}
73+
74+
75+
@with_generated_client_fixture(
76+
"""
77+
paths:
78+
/upload:
79+
post:
80+
operationId: upload
81+
requestBody:
82+
content:
83+
application/json:
84+
schema:
85+
type: object
86+
properties:
87+
name: {type: string}
88+
multipart/form-data:
89+
schema:
90+
type: object
91+
properties:
92+
file: {type: string, format: binary}
93+
responses:
94+
"200": {description: OK}
95+
"""
96+
)
97+
@with_generated_code_imports(
98+
".api.default.upload.sync_detailed",
99+
".client.Client",
100+
".models.UploadJsonBody",
101+
".models.UploadFilesBody",
102+
".types.File",
103+
)
104+
class TestMultipartBodyAmongSeveralBodies:
105+
def test_file_content_is_not_split_by_boundary(self, sync_detailed, Client, UploadJsonBody, UploadFilesBody, File):
106+
body = UploadFilesBody(file=File(payload=io.BytesIO(FILE_CONTENT), file_name="a.txt"))
107+
108+
request = _send_and_capture(Client, sync_detailed, body)
109+
110+
assert _parse_parts(request) == {"file": FILE_CONTENT}
111+
112+
def test_json_body_still_sets_its_content_type(self, sync_detailed, Client, UploadJsonBody, UploadFilesBody, File):
113+
request = _send_and_capture(Client, sync_detailed, UploadJsonBody(name="a"))
114+
115+
assert request.headers["content-type"] == "application/json"
116+
assert request.content == b'{"name":"a"}'

‎end_to_end_tests/golden-records/my-enum-api-client/my_enum_api_client/api/tests/post_user_list.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,6 @@ def _get_kwargs(
2424
if not isinstance(body, Unset):
2525
_kwargs["files"] = body.to_multipart()
2626

27-
headers["Content-Type"] = "multipart/form-data; boundary=+++"
28-
2927
_kwargs["headers"] = headers
3028
return _kwargs
3129

‎end_to_end_tests/golden-records/my-test-api-client/my_test_api_client/api/bodies/post_bodies_multiple.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,6 @@ def _get_kwargs(
3939
if not isinstance(body, Unset):
4040
_kwargs["files"] = body.to_multipart()
4141

42-
headers["Content-Type"] = "multipart/form-data; boundary=+++"
43-
4442
_kwargs["headers"] = headers
4543
return _kwargs
4644

‎end_to_end_tests/golden-records/my-test-api-client/my_test_api_client/api/tests/upload_file_tests_upload_post.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ def _get_kwargs(
2323

2424
_kwargs["files"] = body.to_multipart()
2525

26-
headers["Content-Type"] = "multipart/form-data; boundary=+++"
27-
2826
_kwargs["headers"] = headers
2927
return _kwargs
3028

‎integration-tests/integration_tests/api/body/post_body_multipart.py‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,6 @@ def _get_kwargs(
2525
if not isinstance(body, Unset):
2626
_kwargs["files"] = body.to_multipart()
2727

28-
headers["Content-Type"] = "multipart/form-data; boundary=+++"
29-
3028
_kwargs["headers"] = headers
3129
return _kwargs
3230

‎openapi_python_client/templates/endpoint_module.py.jinja‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -50,18 +50,15 @@ def _get_kwargs(
5050
{% for body in endpoint.bodies %}
5151
if isinstance(body, {{body.prop.get_type_string(no_optional=True) | as_unembedded_code }}):
5252
{{ body_to_kwarg(body) | indent(8) }}
53-
{%- if body.content_type == "multipart/form-data" %}
54-
headers["Content-Type"] = "multipart/form-data; boundary=+++"
55-
{% else %}
53+
{#- httpx2 sets multipart/form-data with a random boundary itself, a fixed one could appear in the content #}
54+
{%- if body.content_type != "multipart/form-data" %}
5655
headers["Content-Type"] = "{{ body.content_type | in_double_quote_literal }}"
5756
{% endif %}
5857
{% endfor %}
5958
{% elif endpoint.bodies | length == 1 %}
6059
{% set body = endpoint.bodies[0] %}
6160
{{ body_to_kwarg(body) | indent(4) }}
62-
{%- if body.content_type == "multipart/form-data" %}
63-
headers["Content-Type"] = "multipart/form-data; boundary=+++"
64-
{% else %}
61+
{%- if body.content_type != "multipart/form-data" %}
6562
headers["Content-Type"] = "{{ body.content_type | in_double_quote_literal }}"
6663
{% endif %}
6764
{% endif %}

0 commit comments

Comments
 (0)