diff --git a/CHANGELOG.md b/CHANGELOG.md index fa98e76..5db65bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,16 @@ # Changelog +## 1.1.1 (2026-09-01) + +- Encode `multipart/form-data` request bodies directly when every field holds in-memory data + (`bytes`, `bytearray` or `str`), instead of letting `requests` build the body up in a `BytesIO`. + Joining the parts sizes the body once and copies each field a single time, which noticeably + reduces both time and peak memory when uploading large fields. Requests carrying file objects, + an explicit `Content-Type`, or a separate `data` payload keep using `requests` as before. +- Request headers set by the client are now normalized to lower case. HTTP header names are + case-insensitive, so servers see the same request, but callers that passed a header in a + different case previously got it sent twice alongside the client default. + ## 1.1.0 (2026-08-12) - `CreateLogEntries` now accepts a payload that is already encoded as JSON `bytes` and sends those bytes diff --git a/python/mujintestwebstackclient/test_webstackclient.py b/python/mujintestwebstackclient/test_webstackclient.py index 95f876a..64d1d27 100644 --- a/python/mujintestwebstackclient/test_webstackclient.py +++ b/python/mujintestwebstackclient/test_webstackclient.py @@ -1,15 +1,19 @@ # -*- coding: utf-8 -*- +import io import msgspec import pytest import requests_mock import random +import requests.models import sys import copy import graphql +import urllib3.filepost from unittest import mock +from mujinwebstackclient import controllerwebclientraw from mujinwebstackclient.webstackclient import WebstackClient from mujinwebstackclient.webstackclientutils import QueryIterator, GetMaximumQueryLimit from mujinwebstackclient.webstackgraphclientutils import GraphQueryIterator @@ -507,7 +511,7 @@ def test_CreateLogEntriesAcceptsPreEncodedPayloads(): logEntry = { 'occurredAt': '2026-08-12T00:00:00Z', 'version': 1, - 'soukoExecutionTask': {'taskId': 'aeon_cycleCount:C6E3KZP5K1LQMQL5', 'taskType': 'aeon_cycleCount'}, + 'soukoExecutionTask': {'taskId': 'cycleCount:C6E3KZP5K1LQMQL5', 'taskType': 'cycleCount'}, } webstackclient.CreateLogEntries(logEntries=[('SoukoExecutionTask', logEntry, {})]) @@ -521,3 +525,44 @@ def test_CreateLogEntriesAcceptsPreEncodedPayloads(): assert filesFromBytes == filesFromDict # The encoded payload went out as given rather than being run through the encoder again. webstackclient._webclient.EncodeJSON.assert_not_called() + + +def test_MultipartBodyMatchesTheEncodingRequestsWouldHaveProduced(monkeypatch): + """The multipart body is built directly rather than through requests, so it must be byte-identical. + + requests appends every part to a growing BytesIO and copies the result again, which for a batch of + already-encoded log entries costs several times the size of the body. Sidestepping that is only safe + if what reaches the server is unchanged, so compare against requests' own encoder part for part. + """ + # Both encoders pick a random boundary, so pin it to compare the rest of the body + fixedBoundary = 'ff00ff00ff00ff00ff00ff00ff00ff00' + monkeypatch.setattr(controllerwebclientraw, 'choose_boundary', lambda: fixedBoundary) + monkeypatch.setattr(urllib3.filepost, 'choose_boundary', lambda: fixedBoundary) + + files = [ + ('logEntry/soukoExecutionTask', ('', b'{"taskId":"cycleCount:C6E3KZP5K1LQMQL5"}', 'application/json')), + ('logEntry/soukoExecutionTaskStateUpdate', ('', b'{"version":2}', 'application/json')), + ('logEntry/unicodePayload', ('', '{"note":"\u65e5\u672c\u8a9e"}', 'application/json')), + ('attachment', ('response.json', b'{"ok":true}')), + ('attachment', ('empty.json', b'')), + ] + expectedBody, expectedContentType = requests.models.RequestEncodingMixin._encode_files(files, None) + + body, contentType = controllerwebclientraw.ControllerWebClientRaw._EncodeMultipartFormData(files) + assert body == expectedBody + assert contentType == expectedContentType + + +@pytest.mark.parametrize( + 'files', + [ + [('files', ('a.txt', io.BytesIO(b'contents')))], # a real file object has to be read by requests + {'file': b'contents'}, # requests guesses a filename for the mapping form + [('field', b'contents')], # and for a bare value, which would change the body + [('attachment', ('a.json', None))], # a None payload is dropped by requests, not encoded + [], # requests raises its own error for no fields + ], +) +def test_MultipartEncodingDeclinesFieldsItCannotEncodeItself(files): + """Anything requests would treat differently must fall back to requests instead of being guessed at.""" + assert controllerwebclientraw.ControllerWebClientRaw._EncodeMultipartFormData(files) is None diff --git a/python/mujinwebstackclient/controllerwebclientraw.py b/python/mujinwebstackclient/controllerwebclientraw.py index 0cc24b6..f3f64ab 100644 --- a/python/mujinwebstackclient/controllerwebclientraw.py +++ b/python/mujinwebstackclient/controllerwebclientraw.py @@ -26,8 +26,10 @@ import websockets from requests import auth as requests_auth from requests import adapters as requests_adapters -from typing import Optional, Callable, Dict, Any, Union, List +from typing import Optional, Callable, Dict, Any, Union, List, Tuple from urllib.parse import urlparse +from urllib3.fields import RequestField +from urllib3.filepost import choose_boundary import websockets.asyncio import websockets.asyncio.client @@ -47,6 +49,10 @@ logging.getLogger('websockets').setLevel(logging.WARNING) log = logging.getLogger(__name__) +# Field data types that the requests library treats as already being in-memory. +# Anything else needs to fall back to the slow path +_IN_MEMORY_FIELD_DATA_TYPES = (bytes, bytearray, str) + class JSONWebTokenAuth(requests_auth.AuthBase): """Attaches JWT Bearer Authentication to a given Request object. Use basic authentication if token is not available.""" @@ -252,6 +258,60 @@ def Destroy(self): def SetDestroy(self): self._isok = False + @staticmethod + def _EncodeMultipartFormData(files: Any) -> Optional[Tuple[bytes, str]]: + """Encodes in-memory multipart/form-data fields into a request body in a single allocation. + + The requests library hands multipart fields to urllib3, which appends each part to a BytesIO. + This reallocates and copies on each resize, and the final getvalue() copies again. + Joining a list of binary parts instead computes the final size up front, and copies only once. + + This optimization only works for fields that are actually in-memory types. + Anything with more custom handling inside of requests (file descriptors, etc) needs to fall back. + + :param files: multipart fields as a sequence of (fieldName, (filename, data[, contentType[, headers]])) pairs + :return: the (body, contentType) pair, or None if the fields are not all in-memory. + """ + if not files or not isinstance(files, (list, tuple)): + return None + + fields: List[RequestField] = [] + for entry in files: + # Only the (fieldName, valueTuple) form is handled. + # requests applies extra filename guessing to the bare-value form, which would change the body. + if not isinstance(entry, (list, tuple)) or len(entry) != 2: + return None + fieldName, value = entry + if not isinstance(value, (list, tuple)) or not 2 <= len(value) <= 4: + return None + data = value[1] + if not isinstance(data, _IN_MEMORY_FIELD_DATA_TYPES): + return None + field = RequestField( + name=fieldName, + data=data, + filename=value[0], + headers=value[3] if len(value) == 4 else None, + ) + + # A two-element value carries no content type, and requests leaves it unset rather than guessing. + # Replicate this behaviour for consistency. + field.make_multipart(content_type=value[2] if len(value) >= 3 else None) + fields.append(field) + + # Accumulate our list of encoded fields + boundary = choose_boundary() + parts: List[bytes] = [] + for field in fields: + parts.append(('--%s\r\n' % boundary).encode('latin-1')) + parts.append(field.render_headers().encode('utf-8')) + parts.append(field.data.encode('utf-8') if isinstance(field.data, str) else field.data) + parts.append(b'\r\n') + parts.append(('--%s--\r\n' % boundary).encode('latin-1')) + + # A binary join performs a single allocation + copy of all the input data + return b''.join(parts), 'multipart/form-data; boundary=%s' % boundary + @staticmethod def _JSONEncodeHook(obj: Any) -> Any: # Convert numpy values to the native python objects that msgspec then re-encodes directly. @@ -383,6 +443,17 @@ def APICall( if headers is None: headers = {} + # Sanitize header keys as lower case so that further presence checks can use hash lookups + headers = {key.lower(): value for key, value in headers.items()} + + # If the files consist of only in-memory data, encode the body ourselves. + # Requests uses BytesIO, which performs unnecessary copies/doubling operations that we can avoid. + if files is not None and data is None and 'content-type' not in headers: + encodedMultipart = self._EncodeMultipartFormData(files) + if encodedMultipart is not None: + data, headers['content-type'] = encodedMultipart + files = None + # GET/HEAD must not carry a request body # Some forwarding proxies (e.g. privoxy) hang when a GET arrives with a body, # since python sends the body in a separate TCP segment that the proxy does not expect on GET @@ -390,12 +461,12 @@ def APICall( data = {} # Default to json content type if not using multipart/form-data - if 'Content-Type' not in headers and files is None and data is not None: - headers['Content-Type'] = 'application/json' + if 'content-type' not in headers and files is None and data is not None: + headers['content-type'] = 'application/json' data = self.EncodeJSON(data) - if 'Accept' not in headers: - headers['Accept'] = 'application/json' + if 'accept' not in headers: + headers['accept'] = 'application/json' response = self.Request(method, path, params=params, data=data, files=files, headers=headers, timeout=timeout) diff --git a/python/mujinwebstackclient/version.py b/python/mujinwebstackclient/version.py index 0308f96..44829ba 100644 --- a/python/mujinwebstackclient/version.py +++ b/python/mujinwebstackclient/version.py @@ -1,3 +1,3 @@ -__version__ = '1.1.0' +__version__ = '1.1.1' # Do not forget to update CHANGELOG.md