From 39b0c69bacd093bea1e5ad8030963520dbf44ffa Mon Sep 17 00:00:00 2001 From: Muhammad Abdullah Rasheed Date: Mon, 27 Jul 2026 01:48:03 +0100 Subject: [PATCH] Keep every URL value of a multipart list field _build_multipart walks the values of a file field and splits them: real uploads are appended to the files list, while values the file manager returns as plain strings, such as HTTP URLs, went into data[name]. The string branch assigned rather than accumulated, so each value overwrote the previous one and a list field carrying several URLs arrived at the service with only the last. Real uploads were unaffected because that branch already appended. Collect the string values in a list instead. The aiohttp client also needs its FormData loop to add one field per value, since it feeds data entries to add_field one at a time. Both clients were affected and are fixed together. --- src/_bentoml_impl/client/http.py | 5 +- src/_bentoml_impl/client/proxy2.py | 11 ++- .../client/test_multipart_list_fields.py | 82 +++++++++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) create mode 100644 tests/unit/_internal/client/test_multipart_list_fields.py diff --git a/src/_bentoml_impl/client/http.py b/src/_bentoml_impl/client/http.py index cfe64b4d779..a04d5428eee 100644 --- a/src/_bentoml_impl/client/http.py +++ b/src/_bentoml_impl/client/http.py @@ -336,7 +336,10 @@ def is_file_field(k: str) -> bool: for v in value: file = self._file_manager.get_file(v) if isinstance(file, str): - data[name] = file + # A list field can resolve to several plain values, e.g. URLs that + # are forwarded as-is. Collect them so each one is sent as its own + # part, instead of overwriting the previous value. + data.setdefault(name, []).append(file) else: files.append((name, file)) headers.pop("content-type", None) diff --git a/src/_bentoml_impl/client/proxy2.py b/src/_bentoml_impl/client/proxy2.py index 602d10b9344..741aa52920d 100644 --- a/src/_bentoml_impl/client/proxy2.py +++ b/src/_bentoml_impl/client/proxy2.py @@ -493,13 +493,20 @@ def is_file_field(k: str) -> bool: for v in value: file = self._file_manager.get_file(v) if isinstance(file, str): - data[name] = file + # A list field can resolve to several plain values, e.g. URLs that + # are forwarded as-is. Collect them so each one is sent as its own + # part, instead of overwriting the previous value. + data.setdefault(name, []).append(file) else: files.append((name, file)) headers.pop("content-type", None) payload = aiohttp.FormData() for key, val in data.items(): - payload.add_field(key, val) + if isinstance(val, list): + for item in val: + payload.add_field(key, item) + else: + payload.add_field(key, val) for key, (filename, fileobj, content_type) in files: payload.add_field( key, diff --git a/tests/unit/_internal/client/test_multipart_list_fields.py b/tests/unit/_internal/client/test_multipart_list_fields.py new file mode 100644 index 00000000000..37147c34f8d --- /dev/null +++ b/tests/unit/_internal/client/test_multipart_list_fields.py @@ -0,0 +1,82 @@ +from __future__ import annotations + +import io +import types +import typing as t + +import httpx +import pytest + +from _bentoml_impl.client.http import HTTPClient +from _bentoml_impl.client.proxy2 import AsyncClient + + +class _PassthroughFileManager: + """Stands in for the real file manager. + + `ClientFileManager.get_file` returns an HTTP URL unchanged, as a plain string, + and returns a `(filename, fileobj, content_type)` tuple for an actual upload. + """ + + def get_file(self, value: t.Any) -> t.Any: + return value + + +def _endpoint(is_array: bool = True) -> types.SimpleNamespace: + field = ( + {"type": "array", "items": {"type": "file"}} if is_array else {"type": "file"} + ) + return types.SimpleNamespace(route="/echo", input={"properties": {"files": field}}) + + +def _httpx_body(model: dict[str, t.Any], is_array: bool = True) -> str: + client = types.SimpleNamespace( + _file_manager=_PassthroughFileManager(), client=httpx.Client() + ) + request = HTTPClient._build_multipart( + client, _endpoint(is_array), model, httpx.Headers() + ) + return request.read().decode("utf-8", "replace") + + +def _aiohttp_values(model: dict[str, t.Any]) -> list[t.Any]: + client = types.SimpleNamespace(_file_manager=_PassthroughFileManager()) + payload = AsyncClient._build_multipart(client, _endpoint(), model, {}) + return [field[2] for field in payload._fields] + + +URL_A = "https://files.example.invalid/a.txt" +URL_B = "https://files.example.invalid/b.txt" + + +def test_httpx_client_keeps_every_url_in_a_list_field() -> None: + # Each value of a list field resolves to a plain string, and every one of them + # has to be sent. Assigning instead of accumulating dropped all but the last. + body = _httpx_body({"files": [URL_A, URL_B]}) + + assert "a.txt" in body + assert "b.txt" in body + + +def test_aiohttp_client_keeps_every_url_in_a_list_field() -> None: + assert _aiohttp_values({"files": [URL_A, URL_B]}) == [URL_A, URL_B] + + +@pytest.mark.parametrize("is_array", [True, False]) +def test_single_url_is_unchanged(is_array: bool) -> None: + # A field carrying one value must still be sent exactly once. + model = {"files": [URL_A] if is_array else URL_A} + assert _httpx_body(model, is_array).count("a.txt") == 1 + + +def test_urls_and_uploads_can_be_mixed() -> None: + # A URL travels as a form value while a real upload travels as a file part; + # collecting the URLs must not disturb the upload. + body = _httpx_body({"files": [URL_A, ("real.txt", io.BytesIO(b"X"), "text/plain")]}) + + assert "a.txt" in body + assert 'filename="real.txt"' in body + + +def test_aiohttp_single_url_is_unchanged() -> None: + assert _aiohttp_values({"files": [URL_A]}) == [URL_A]