Jens-G opened a new pull request, #3879: URL: https://github.com/apache/thrift/pull/3879
[THRIFT-6286](https://issues.apache.org/jira/browse/THRIFT-6286) Go's `TConfiguration` gains `MaxContainerSize`. This is the Go counterpart of C++ `container_limit_`, Java `containerLengthLimit_`, D `containerSizeLimit` and Rust `max_container_size`. ### What changes - **`TConfiguration.MaxContainerSize int32`** and the nil-safe getter **`GetMaxContainerSize()`**. - A value `<= 0` means no limit of its own; the getter then returns `math.MaxInt32`. - This follows the other `Get*` methods, and it is the same "off" rule as D and c_glib. C++ uses `0`, Java `-1` and Python `None`. - **`checkContainerSizeForProtocol`** rejects a count above the limit with a `SIZE_LIMIT` `TProtocolException`. - The check sits between the int32 range check and the minimum-size and remaining-bytes checks. - All protocols already route list, set and map headers through this function: binary, compact, JSON and simple JSON. `THeaderProtocol` passes its configuration on to the inner protocol, so the limit applies there as well. - **`THeaderTransport`** also routes the transform count of a frame header through this function, so the limit covers that count too. It has done so since 6351cd5a0, and the code comment there treats the count like a list, set or map count. - The field comment and the README say so. - In practice a frame carries zero or one transform, so any positive limit leaves it alone. - **`lib/go/README.md`**: new section "A note about container sizes". The default does not change: without `MaxContainerSize` a count is bounded as before, by `MaxMessageSize` and, where the transport knows it, by the bytes left to read. THRIFT-6287 asks the same default question for Rust. Only the read side is checked, as in the other bindings. ### Tests `lib/go/thrift/max_container_size_test.go` (new): - **`TestMaxContainerSize`**: limit 10, with counts 10 and 11, for list, set and map. It covers binary, compact, JSON, simple JSON and THeader. - Each container is written in full, so the bytes after the count are no limit of their own. - A count at the limit is read; one above it fails with `SIZE_LIMIT`. - **`TestMaxContainerSizeUnset`**: a nil configuration, `{}` and `MaxContainerSize: -1` all read a count of 1000. - **`TestGetMaxContainerSize`** and **`TestCheckContainerSizeForProtocolLimit`**: the getter and the check function on their own. - **`TestTHeaderTransportTransformCountMaxContainerSize`**: a hand-built frame with two transforms is read under limit 2 and rejected under limit 1. Verification: - **Test-first:** before the change, the test file fails to compile (the field and the getter did not exist). - **Mutation:** 17 mutants, all caught. Each one fails at least one of the new tests: - drop the check; - `>=` instead of `>`; - getter: `< 0` instead of `<= 0`, no nil check, `0` instead of `MaxInt32` when unset, and off by one; - pass `nil` instead of the configuration at each call site: 2 JSON, 2 simple JSON, 3 binary, 2 compact, and the THeader transform count; - drop the configuration propagation in the `THeaderProtocol` constructor. - **Toolchains:** the whole `thrift` package passes on Go 1.26 and 1.27, each also with `-race` and under `GOARCH=386`. - **Linting:** `gofmt -s -l` is clean; `go vet -stdmethods=false` (as in `lib/go/Makefile.am`) is clean. - **Merges:** the branch merges cleanly with the open Go PRs #3873 and #3812. ### Threat model Checked against `doc/thrift-threat-model.md`: - **§12 item 3 does not apply.** That item is about a §5a knob changing from unlimited to a finite default, which would reclassify §11a entries. This knob stays off by default. - **§10 item 9** asks operators to set a finite container limit. Go now has one, and §6 ("Cross-binding parity assumption") counts Go among the reference bindings. - **Q50** says a binding that exposes the knob must enforce it. The tests above check that for every Go protocol that reads a container header. --- The change and tests were written with AI assistance (Claude Opus 5), then reviewed and tested as described above. 🤖 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]
