diff --git a/.chronus/changes/python-fix-multipart-form-data-order-2026-5-18.md b/.chronus/changes/python-fix-multipart-form-data-order-2026-5-18.md new file mode 100644 index 00000000000..beec01b3500 --- /dev/null +++ b/.chronus/changes/python-fix-multipart-form-data-order-2026-5-18.md @@ -0,0 +1,23 @@ +--- +changeKind: fix +packages: + - "@typespec/http-client-python" +--- + +Generated `prepare_multipart_form_data` now serializes multipart fields in the +order declared in the TypeSpec model, instead of emitting all file parts +before all data parts. The previous behavior could break streaming server-side +multipart parsers that require small JSON metadata parts to precede large +binary file parts (per RFC 7578 §5.2), and it did not match the order +documented in Spector multipart scenarios. + +This is observable on the wire for services whose TypeSpec model declares data +fields before file fields — that order is now preserved. + +```python +# previously emitted call: +_files = prepare_multipart_form_data(_body, _file_fields, _data_fields) + +# now emitted call (single ordered list of (wire_name, is_file) tuples): +_files = prepare_multipart_form_data(_body, _fields) +``` diff --git a/packages/http-client-python/generator/pygen/codegen/serializers/builder_serializer.py b/packages/http-client-python/generator/pygen/codegen/serializers/builder_serializer.py index e43dcd916eb..a4524da1801 100644 --- a/packages/http-client-python/generator/pygen/codegen/serializers/builder_serializer.py +++ b/packages/http-client-python/generator/pygen/codegen/serializers/builder_serializer.py @@ -681,8 +681,7 @@ def _serialize_body_parameter(self, builder: OperationType) -> list[str]: else body_param.type ), ) - file_fields = [p.wire_name for p in model_type.properties if p.is_multipart_file_input] - data_fields = [p.wire_name for p in model_type.properties if not p.is_multipart_file_input] + fields = [(p.wire_name, p.is_multipart_file_input) for p in model_type.properties] retval.extend( [ "_body = (", @@ -690,9 +689,8 @@ def _serialize_body_parameter(self, builder: OperationType) -> list[str]: f" if isinstance({body_param.client_name}, _Model) else", f" {body_param.client_name}", ")", - f"_file_fields: list[str] = {file_fields}", - f"_data_fields: list[str] = {data_fields}", - "_files = prepare_multipart_form_data(_body, _file_fields, _data_fields)", + f"_fields: list[tuple[str, bool]] = {fields}", + "_files = prepare_multipart_form_data(_body, _fields)", ] ) return retval diff --git a/packages/http-client-python/generator/pygen/codegen/templates/utils.py.jinja2 b/packages/http-client-python/generator/pygen/codegen/templates/utils.py.jinja2 index 140f80638cd..43c1deed6eb 100644 --- a/packages/http-client-python/generator/pygen/codegen/templates/utils.py.jinja2 +++ b/packages/http-client-python/generator/pygen/codegen/templates/utils.py.jinja2 @@ -76,23 +76,27 @@ def serialize_multipart_data_entry(data_entry: Any) -> Any: return json.dumps(data_entry, cls=SdkJSONEncoder, exclude_readonly=True) return data_entry +# ``fields`` is an ordered list of ``(wire_name, is_file)`` pairs taken from +# the body model's properties. Iterating in that order preserves the TypeSpec +# declaration order on the wire, which per RFC 7578 §5.2 is significant: some +# streaming server-side parsers require small JSON metadata parts to precede +# large binary file parts; otherwise they report the metadata part as missing. def prepare_multipart_form_data( - body: Mapping[str, Any], multipart_fields: list[str], data_fields: list[str] + body: Mapping[str, Any], fields: list[tuple[str, bool]] ) -> list[FileType]: files: list[FileType] = [] - for multipart_field in multipart_fields: - multipart_entry = body.get(multipart_field) - if isinstance(multipart_entry, list): - files.extend([(multipart_field, e) for e in multipart_entry ]) - elif multipart_entry: - files.append((multipart_field, multipart_entry)) - - # if files is empty, sdk core library can't handle multipart/form-data correctly, so - # we put data fields into files with filename as None to avoid that scenario. - for data_field in data_fields: - data_entry = body.get(data_field) - if data_entry: - files.append((data_field, str(serialize_multipart_data_entry(data_entry)))) + for wire_name, is_file in fields: + entry = body.get(wire_name) + if is_file: + if isinstance(entry, list): + files.extend([(wire_name, e) for e in entry]) + elif entry: + files.append((wire_name, entry)) + elif entry: + # data fields are placed into files with filename as None so that + # sdk core can handle the multipart/form-data correctly even when + # there are no file parts. + files.append((wire_name, str(serialize_multipart_data_entry(entry)))) return files {% endif %} diff --git a/packages/http-client-python/tests/unit/test_prepare_multipart_form_data.py b/packages/http-client-python/tests/unit/test_prepare_multipart_form_data.py new file mode 100644 index 00000000000..53fd422a60b --- /dev/null +++ b/packages/http-client-python/tests/unit/test_prepare_multipart_form_data.py @@ -0,0 +1,112 @@ +# ------------------------------------------------------------------------- +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. See License.txt in the project root for +# license information. +# -------------------------------------------------------------------------- +"""Unit tests for the generated ``prepare_multipart_form_data`` helper. + +The helper is rendered from ``utils.py.jinja2`` into every SDK that has a +multipart/form-data operation. We import one such rendered copy (from the +``payload.multipart`` test SDK) and assert that: + +1. Fields are serialized in the TypeSpec declaration order — not split into + "all files first, then all data" the way the previous implementation did. + This matters because some streaming server-side parsers require JSON + metadata parts to precede binary file parts (see RFC 7578 §5.2 and the + ``create_agent_version_from_code`` endpoint of the Azure AI Foundry + hosted-agents service). +2. File fields handle list-valued entries (multiple files under the same + wire name) by emitting one part per element. +3. Data fields are serialized through ``serialize_multipart_data_entry`` so + model/dict/list values are encoded as JSON. +""" + +import json + +from payload.multipart._utils.utils import prepare_multipart_form_data + + +def test_fields_preserve_declaration_order(): + """Data fields declared before file fields must appear first on the wire.""" + body = { + "id": "123", + "address": {"city": "X"}, + "profileImage": b"jpg-bytes", + "previousAddresses": [{"city": "Y"}, {"city": "Z"}], + "pictures": [b"png-bytes-1", b"png-bytes-2"], + } + fields = [ + ("id", False), + ("address", False), + ("profileImage", True), + ("previousAddresses", False), + ("pictures", True), + ] + + files = prepare_multipart_form_data(body, fields) + + assert [name for name, _ in files] == [ + "id", + "address", + "profileImage", + "previousAddresses", + "pictures", + "pictures", + ] + + +def test_files_first_when_declared_first(): + """If the TypeSpec model declares files first, that order is preserved.""" + body = {"profileImage": b"jpg-bytes", "id": "123"} + fields = [("profileImage", True), ("id", False)] + + files = prepare_multipart_form_data(body, fields) + + assert [name for name, _ in files] == ["profileImage", "id"] + + +def test_list_valued_file_field_emits_one_part_per_element(): + body = {"pictures": [b"a", b"b", b"c"]} + fields = [("pictures", True)] + + files = prepare_multipart_form_data(body, fields) + + assert files == [("pictures", b"a"), ("pictures", b"b"), ("pictures", b"c")] + + +def test_data_field_dict_is_json_encoded(): + body = {"address": {"city": "X"}} + fields = [("address", False)] + + files = prepare_multipart_form_data(body, fields) + + assert len(files) == 1 + name, value = files[0] + assert name == "address" + assert json.loads(value) == {"city": "X"} + + +def test_data_field_list_is_json_encoded(): + body = {"previousAddresses": [{"city": "Y"}, {"city": "Z"}]} + fields = [("previousAddresses", False)] + + files = prepare_multipart_form_data(body, fields) + + assert len(files) == 1 + name, value = files[0] + assert name == "previousAddresses" + assert json.loads(value) == [{"city": "Y"}, {"city": "Z"}] + + +def test_missing_or_falsy_entries_are_skipped(): + """Matches the pre-fix behavior: ``body.get(field)`` falsy values are dropped.""" + body = {"id": "", "profileImage": None, "address": {"city": "X"}} + fields = [ + ("id", False), + ("address", False), + ("profileImage", True), + ] + + files = prepare_multipart_form_data(body, fields) + + assert [name for name, _ in files] == ["address"]