Jens-G opened a new pull request, #3875: URL: https://github.com/apache/thrift/pull/3875
JIRA: [THRIFT-6283](https://issues.apache.org/jira/browse/THRIFT-6283) Client: erl The third point of THRIFT-6268, split out as THRIFT-6283: a limit on the size of a reply. The first point, a failed request, was #3866. `thrift_http_transport:flush/1` called `httpc:request/4` in its synchronous mode, which reads the whole reply into memory before returning, however long it is. ### Change - **Streaming.** The request is now asynchronous with `{stream, {self, once}}`. The transport counts the body as it arrives and asks httpc for each next part with `httpc:stream_next/1`. Once the body is longer than the limit, it cancels the request, so at most one part is read past the limit. A reply whose `Content-Length` is already longer is given up at `stream_start`, before any of its body is read. The flush returns `{error, {message_size_exceeds_maximum, Max}}`, and the write buffer is cleared as for any other failed request. - **Limit.** `DEFAULT_MAX_MESSAGE_SIZE` in `thrift_constants.hrl` is 100 MB, the default maximum message size of the other bindings. It can be overridden in two ways: - application-wide with `{thrift, [{max_message_size, Bytes}]}`, read at flush time as `max_frame_size` is; - per transport with the new `{max_message_size, Bytes}` option of `new/3`, which must be a positive integer. - **Replies httpc does not stream.** httpc streams only 200 and 206 replies and reads any other reply whole. Where httpc takes the `max_body_size` request option, the transport passes the limit to it as well (see below). httpc's refusals, `body_too_big` and `{body_too_long, _}`, are returned as the same `message_size_exceeds_maximum` error. - **Helper process.** For every asynchronous request, httpc leaves an alias of the calling process active for as long as that process lives. Measured over 100 requests, a process making them itself grows by 9600 bytes on OTP 25 and by 11200 bytes on OTP 28. A synchronous request cleans its alias up, and a long-lived client process would otherwise grow with every call. The request is therefore made from a short-lived process spawned with `spawn_monitor`: - With it, the caller grows by 0 bytes on OTP 25–28. - Parts of a cancelled reply that are still on their way die with that process, instead of piling up in the caller's mailbox. - An exception from the request is raised again in the caller, as before. ### `max_body_size` exists only on recent OTP patch releases The ticket assumed that `max_body_size` exists from OTP 27 on. That is not so: - It came with the OTP patch releases of 2026-09-01 (OTP-20343): inets 9.3.2.7 (OTP 27.3.4.17), 9.6.2.3 (OTP 28.5.0.6) and 9.7.2 (OTP 29.0.6). This was checked at each release tag. - OTP 27.0 to 27.3.4.16, 28.0 to 28.5.0.5, 29.0 to 29.0.5 and all of OTP 26 do not have it. - An httpc without it logs `Invalid option {max_body_size,…} ignored` on every request. The log level is `notice`, so the line shows at the default log level. - So `takes_max_body_size/1` checks the inets version against those three patch lines. Any other version gets no `max_body_size`. Even where httpc takes the option, it applies it only to a reply that declares its length or is chunked. A reply that is not 200, and whose body ends when the connection closes, is still read whole by httpc. The README says so. ### Behaviour changes - A reply longer than the limit is now an error. - A 206 reply is now read like a 200 reply. httpc streams both, and its stream messages do not say which of the two a reply was. #3866 made every non-200 status an error; no release contains that change yet. ### Tests The new tests in `test_thrift_http_transport.erl` use a second local server. It sends the reply body in 250-byte slices, a millisecond apart, and reports how much of the body it sent before the client went away. The limit is set to 1000 bytes for these tests. - **Limit cases, for each framing** (`Content-Length`, connection close, chunked): - A reply exactly at the limit is read. - A reply one byte over the limit is refused, and nothing of it is left to read. - For a 100 KB reply, the server is stopped after less than 20 KB. The caller's mailbox stays empty. - **Default limit:** with no limit set, a reply declared at 100 MB + 1 byte is refused before any of its body is read. - **Replies that are not 200** (with `Content-Length` and chunked): refused where httpc takes `max_body_size`. Otherwise read whole and returned as `http_status`. - **Request timeout:** a timeout still applies while the reply is being received. - **Options:** the transport's own limit takes the place of the application-wide one, in either direction. Invalid limit values are rejected. - **After a refused reply:** the next request carries only its own data, and the transport keeps working. - **No ignored-option log line:** no `Invalid option` line is logged during a request. The test catches the logger events with a temporary handler. - **Memory:** the caller does not grow over 100 requests. - **Version check:** `takes_max_body_size/1` is tested against the version boundaries, one version on each side. Results: - **Against the unmodified library:** 27 of the 37 new tests fail on OTP 25, 26 and 28. Ten pass, because the synchronous request passes them as well: the three at-limit reads, the four invalid-option checks, and the timeout, log and memory tests. - **`rebar3 eunit`:** all 397 tests pass on OTP 25.3.2.21, on 25.3.2.9 with rebar3 3.18.0 (the docker image), and on 26.2.5.21, 27.3.4.17 and 28.5.0.6. The first, third, fourth and fifth are the patch levels CI's setup-beam installs. On the final code, the suite ran 6 times on 25.3.2.21, 4 times on 25.3.2.9, 3 times each on 26 and 28, and 5 times on 27. - **Mutations**, each detected: - Making the request in the caller is caught by the memory test. - Always passing `max_body_size` is caught by the log test on OTP 25/26. - Never passing it is caught on 27/28. - Dropping the check of the declared length is caught on 25. - Dropping the byte count is caught on 25 and 28. - Using `>=` instead of `>` is caught. - Dropping either error mapping, or reporting a wrong limit, is caught on 28. - Dropping the cancellation is caught. - **Dialyzer:** no warnings, on this branch and on master. - **erlfmt:** `rebar3 fmt -c` reports nothing for the changed files. It still flags the untouched `src/thrift_binary_protocol.erl`, as on master. The existing test server from #3866 crashed when `accept/1` returned `einval` for a listening socket the test had already closed. That happened once on OTP 27, and eunit reported 2 cancelled tests. The server now ends quietly on any accept error. ### Threat model Checked against `doc/thrift-threat-model.md`. Q14 treats the client-side wire boundary like the server side, and this change follows that. The document still lists Erlang among the bindings without an enforced message size limit (line 886), which is no longer true for the HTTP transport. The document is not changed here. The change was written with AI assistance (Claude Opus 5). 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
