Jens-G opened a new pull request, #3767: URL: https://github.com/apache/thrift/pull/3767
The C extension allocates a list, tuple or set from the element count the peer declares, before a single element has been read (`lib/py/src/ext/protocol.tcc`, the `T_LIST`/`T_SET` case of `decodeValue`): ```cpp bool use_tuple = type == T_LIST && parsedargs.immutable; ScopedPyObject ret(use_tuple ? PyTuple_New(len) : PyList_New(len)); ``` The only check on `len` is `checkLengthLimit(len, containerLimit())`, and `containerLimit_` is `INT32_MAX` by default — in `protocol.h`, and again in `module.cpp` as the fallback for a `container_length_limit` attribute of `None`, which is what the Python protocols document and ship. Measured with `tracemalloc`: a nine-byte body declaring 2,000,000 elements reserves 16 MB; 20,000,000 reserves 152 MB. ### This is the second time round for THRIFT-3175 THRIFT-3175 reported it in 2015 and was fixed in 0.9.3 with a hard cap of 10,000 elements. The cap turned out to refuse containers that were perfectly legitimate — [users pinned 0.9.2 over it](https://issues.apache.org/jira/browse/THRIFT-3175) — and THRIFT-3532 lifted it again in 0.10.0 in favour of the configurable `container_length_limit`, whose default is unlimited. The count has been taken at face value ever since for anyone who does not set that limit. So this takes the heuristic dvirsky proposed on THRIFT-3175 in 2015 instead of a cap: > I would suggest a simpler heuristic - whether an allocated a list of the requested size be applicable to the message length. i.e. it's clear to see that there's no use in allocating 2G elements for a message that's 1K in length... Whatever the protocol, an element occupies at least one byte on the wire, so the bytes still sitting in the decode buffer bound how many can turn up. ### The common path does not change That is the point of measuring rather than capping. A message that is already buffered whole — a memory buffer, a frame — always has at least as many bytes left as it declares elements, so the container is still allocated at its full size in one go and every element still goes in by index. Timed over a million `i64`s from a memory buffer, before and after are the same to within noise, 10.5 ms either way, for both the list and the tuple path. Where the buffer holds only part of the message the allocation starts at what is in hand and the rest is appended as it arrives; a tuple, which cannot be appended to, is collected in a list and handed over once complete. Neither of the two earlier failure modes comes back: large containers are not refused, and the declared count is not believed. A refill can always bring more bytes, which is why this sizes the first allocation only and never rejects anything. `checkLengthLimit` and the `container_length_limit` that reaches it are untouched. ### Tests Seven, none of which need generated code — the `thrift_spec` is written out by hand, which is also the only way to reach the immutable tuple path without a `python.immutable` annotation in the IDL. Four hold the allocation for a declared count against a body that cannot supply it, over list and set, mutable and immutable, binary and compact: eight subtests, **all eight failing against the unmodified extension**. The other three are regression guards that pass either way — a truncated payload still reports `EOF`, small containers still decode, and four cases decode 200,000 elements over a buffered transport, whose four-kilobyte window is what exercises the growth path at all, checking list, tuple, set and frozenset come back whole and in order. Full `lib/py` suite green, `flake8` clean. ### Nothing else in the tree needs it `lib/py`'s own `readContainerList` builds through `islice` and never preallocates. Of the other native protocol implementations, Ruby's already caps this — `new_container_array` in `lib/rb/ext/struct.c` allocates `min(size, 1024)` — and PHP's calls `array_init` with no size hint for map, list and set alike. 🤖 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]
