Jens Geyer created THRIFT-6295:
----------------------------------
Summary: Container-size limit: paths where an existing limit
cannot be reached
Key: THRIFT-6295
URL: https://issues.apache.org/jira/browse/THRIFT-6295
Project: Thrift
Issue Type: Improvement
Components: C++ - Library, Java - Library, C glib - Library, Python -
Library, Node.js - Library
Reporter: Jens Geyer
Eight bindings expose a limit on the declared element count of a list, set or
map: C++ {{container_limit_}}, Java {{containerLengthLimit_}}, Python
{{container_length_limit}}, D {{containerSizeLimit}}, Rust
{{max_container_size}}, c_glib {{container_limit}}, the Node.js compact
protocol {{container_limit_}}, and Go once THRIFT-6286 lands.
THRIFT-6291 records in {{doc/specs/thrift-tconfiguration.md}} that such a knob
serves its purpose only where it is reachable on the path that sizes the
collection. The places below are where that condition is not met today: the
limit exists in the binding, but a caller cannot get it to the protocol that
reads the container, or the protocol never consults it.
Fixing any of them would not change a default. Each one is off unless the
caller sets it, and today the caller cannot.
* *Java:* {{TTupleProtocol}} extends {{TCompactProtocol}}, but its only
constructor takes a transport and its {{Factory}} takes no arguments, so no
limit can be passed to it. Its own typed {{readMapBegin(byte, byte)}} and
{{readListBegin(byte)}} read the count and call {{checkReadBytesAvailable}}
only; they never reach {{checkContainerReadLength}}, which is where
{{containerLengthLimit_}} is consulted. {{readSetBegin(byte)}} delegates to
{{readListBegin(byte)}} and inherits the same gap.
* *c_glib:* {{thrift_binary_protocol_factory_get_protocol}} constructs the
protocol with {{"transport"}} alone, and {{ThriftBinaryProtocolFactory}} has no
{{container_limit}} or {{string_limit}} field to pass on. Both properties are
{{G_PARAM_CONSTRUCT_ONLY}} on the protocol, so they cannot be set afterwards
either: a binary protocol obtained from this factory can never have a limit.
{{ThriftCompactProtocolFactory}} already carries both.
* *C++:* {{THeaderProtocol::resetProtocol}} builds its inner protocol with the
single-argument constructor, {{TBinaryProtocolT<THeaderTransport>(trans_)}} or
{{TCompactProtocolT<THeaderTransport>(trans_)}}, which sets both limits to 0.
{{THeaderProtocol}} exposes no way to pass values in, and the inner protocol is
rebuilt on every protocol switch.
* *Python:* {{THeaderProtocol}} builds its inner protocol as
{{protocol_cls(self.trans)}}, so the container limit takes its default and
there is no parameter to pass another. Separately,
{{TJSONProtocol.container_length_limit}} is a property that always returns
{{None}}, on the protocol and on its factory; the comment above both says a
length limit is not implemented for JSON container reads.
* *Node.js:* {{TCompactProtocol}} assigns {{this.container_limit_}} and
{{this.string_limit_}} in its constructor and never reads either again.
h2. Suggested
Handle each item on its own, or close it as not wanted. They are collected here
so that the decision is made in one place.
_Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
