From 6dd55a460366fa87c70e33badeaa35afbad0917e Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 22 Jul 2026 09:44:53 +0100 Subject: [PATCH] fix: api tool encoding bug --- .../agents/tools/api_body_serializer.py | 47 +++++++------ tests/agents/test_api_body_serializer.py | 1 - .../agents/tools/test_api_body_serializer.py | 69 +++++++++++++++++++ 3 files changed, 92 insertions(+), 25 deletions(-) diff --git a/application/agents/tools/api_body_serializer.py b/application/agents/tools/api_body_serializer.py index d23d1fcf..92a950af 100644 --- a/application/agents/tools/api_body_serializer.py +++ b/application/agents/tools/api_body_serializer.py @@ -115,12 +115,12 @@ class RequestBodySerializer: ) if isinstance(serialized_value, list): - for sv in serialized_value: - params.append((key, sv)) + params.extend(serialized_value) else: params.append((key, serialized_value)) - # Use standard urlencode (replaces space with +) + # Values are raw here; urlencode does the single percent-encoding + # pass (space becomes +), matching what servers decode once. serialized = urlencode(params, safe="") headers = {"Content-Type": ContentType.FORM_URLENCODED.value} return serialized, headers @@ -129,7 +129,11 @@ class RequestBodySerializer: def _serialize_form_value( value: Any, style: str, explode: bool, content_type: str, key: str ) -> Union[str, list]: - """Serialize individual form value with encoding rules.""" + """Serialize individual form value with encoding rules. + + Returns a raw (unencoded) string, or a list of raw (name, value) + pairs for exploded dicts/lists; the caller percent-encodes once. + """ if isinstance(value, dict): if content_type == "application/json": return json.dumps(value, separators=(",", ":")) @@ -137,31 +141,20 @@ class RequestBodySerializer: return RequestBodySerializer._dict_to_xml(value) else: if style == "deepObject" and explode: - return [ - f"{RequestBodySerializer._percent_encode(str(v))}" - for v in value.values() - ] + return [(f"{key}[{k}]", str(v)) for k, v in value.items()] elif explode: - return [ - f"{RequestBodySerializer._percent_encode(str(v))}" - for v in value.values() - ] + return [(str(k), str(v)) for k, v in value.items()] else: - pairs = [f"{k},{v}" for k, v in value.items()] - return RequestBodySerializer._percent_encode(",".join(pairs)) + return ",".join(f"{k},{v}" for k, v in value.items()) elif isinstance(value, (list, tuple)): if explode: - return [ - RequestBodySerializer._percent_encode(str(item)) for item in value - ] + return [(key, str(item)) for item in value] else: - return RequestBodySerializer._percent_encode( - ",".join(str(v) for v in value) - ) + return ",".join(str(v) for v in value) else: - return RequestBodySerializer._percent_encode(str(value)) + return str(value) @staticmethod def _serialize_multipart_form_data( @@ -206,9 +199,15 @@ class RequestBodySerializer: name: str, value: Any, content_type: str, headers_rule: Dict[str, Any] ) -> str: """Create a single multipart/form-data part.""" - headers = [ - f'Content-Disposition: form-data; name="{RequestBodySerializer._percent_encode(name)}"' - ] + # RFC 7578: field names go in a quoted string, not percent-encoded; + # only CR/LF (header injection) and the quoting chars need escaping. + safe_name = ( + name.replace("\\", "\\\\") + .replace('"', '\\"') + .replace("\r", "%0D") + .replace("\n", "%0A") + ) + headers = [f'Content-Disposition: form-data; name="{safe_name}"'] if isinstance(value, bytes): if content_type == "application/octet-stream": diff --git a/tests/agents/test_api_body_serializer.py b/tests/agents/test_api_body_serializer.py index b10d0429..81d156e0 100644 --- a/tests/agents/test_api_body_serializer.py +++ b/tests/agents/test_api_body_serializer.py @@ -99,7 +99,6 @@ class TestSerializeFormUrlencoded: ContentType.FORM_URLENCODED, encoding_rules={"tags": {"style": "form", "explode": False}}, ) - # Value is percent-encoded by _serialize_form_value then urlencoded again assert "tags=" in body assert "a" in body and "b" in body diff --git a/tests/agents/tools/test_api_body_serializer.py b/tests/agents/tools/test_api_body_serializer.py index 597f5f08..f79a78cf 100644 --- a/tests/agents/tools/test_api_body_serializer.py +++ b/tests/agents/tools/test_api_body_serializer.py @@ -6,6 +6,7 @@ helper methods (_percent_encode, _escape_xml, _dict_to_xml). """ import json +from urllib.parse import parse_qs import pytest @@ -158,6 +159,46 @@ class TestSerializeFormUrlencoded: ) assert "data" in body + def test_special_chars_encoded_once(self): + """Values must survive exactly one server-side decode (no %25 double-encoding).""" + original = "hello world & stuff=yes 100%" + body, _ = RequestBodySerializer.serialize( + {"msg": original, "email": "a+b@example.com"}, + ContentType.FORM_URLENCODED, + ) + assert "%2520" not in body and "%2525" not in body + decoded = parse_qs(body) + assert decoded["msg"] == [original] + assert decoded["email"] == ["a+b@example.com"] + + def test_list_values_with_special_chars_round_trip(self): + body, _ = RequestBodySerializer.serialize( + {"tags": ["a b", "c&d"]}, + ContentType.FORM_URLENCODED, + encoding_rules={"tags": {"style": "form", "explode": True}}, + ) + assert parse_qs(body)["tags"] == ["a b", "c&d"] + + def test_deep_object_keeps_subkeys(self): + body, _ = RequestBodySerializer.serialize( + {"filter": {"color": "red", "size": "L"}}, + ContentType.FORM_URLENCODED, + encoding_rules={"filter": {"style": "deepObject", "explode": True}}, + ) + decoded = parse_qs(body) + assert decoded["filter[color]"] == ["red"] + assert decoded["filter[size]"] == ["L"] + + def test_exploded_dict_uses_subkeys_as_param_names(self): + body, _ = RequestBodySerializer.serialize( + {"obj": {"color": "red", "size": "L"}}, + ContentType.FORM_URLENCODED, + encoding_rules={"obj": {"style": "form", "explode": True}}, + ) + decoded = parse_qs(body) + assert decoded["color"] == ["red"] + assert decoded["size"] == ["L"] + def test_dict_value_deep_object_explode(self): body, headers = RequestBodySerializer.serialize( {"filter": {"status": "active", "type": "doc"}}, @@ -518,6 +559,34 @@ class TestSerializeFormValueGaps: @pytest.mark.unit class TestApiBodySerializerMultipartParts: + def test_multipart_field_name_not_percent_encoded(self): + """RFC 7578: names are a quoted string, so 'file name' stays literal.""" + from application.agents.tools.api_body_serializer import ( + RequestBodySerializer, + ) + + result = RequestBodySerializer._create_multipart_part( + name="file name", + value="contents", + content_type="text/plain", + headers_rule={}, + ) + assert 'name="file name"' in result + assert "file%20name" not in result + + def test_multipart_field_name_escapes_quotes_and_newlines(self): + from application.agents.tools.api_body_serializer import ( + RequestBodySerializer, + ) + + result = RequestBodySerializer._create_multipart_part( + name='a"b\r\nc', + value="v", + content_type="text/plain", + headers_rule={}, + ) + assert 'name="a\\"b%0D%0Ac"' in result + def test_multipart_dict_unknown_content_type(self): """Cover line 226: dict with unknown content type uses str().""" from application.agents.tools.api_body_serializer import (