Jens Geyer created THRIFT-6169:
----------------------------------
Summary: Size containers from the payload rather than the declared
count in the Python C extension
Key: THRIFT-6169
URL: https://issues.apache.org/jira/browse/THRIFT-6169
Project: Thrift
Issue Type: Bug
Components: Python - Library
Reporter: Jens Geyer
Fix For: 0.25.0
The C extension sizes a list, tuple or set from the element count the peer
declares, before any of the elements have been read
(lib/py/src/ext/protocol.tcc, the T_LIST/T_SET case of decodeValue):
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_ defaults to INT32_MAX (protocol.h, and again in module.cpp as
the fallback when the protocol's container_length_limit attribute is None,
which is its documented default). Measured with tracemalloc on CPython: a nine
byte body declaring 2,000,000 elements reserves 16 MB, and 20,000,000 reserves
152 MB.
This is the same defect as THRIFT-3175, which was fixed in 0.9.3 by a hard cap
of 10,000 elements. That cap turned out to reject legitimate large containers
-- see jrf's comment on THRIFT-3175 -- and THRIFT-3532 lifted it in 0.10.0 in
favour of the configurable container_length_limit, whose default is unlimited.
So the count has been taken at face value again since 0.10.0 for anyone who
does not set that limit.
The fix implements the heuristic dvirsky proposed on THRIFT-3175 in 2015: size
the container from what the message can actually supply. Every element occupies
at least one byte on the wire whatever the protocol, so the bytes still in the
decode buffer bound how many can arrive. A message that is already buffered
whole -- a memory buffer, or a frame -- always has at least as many bytes left
as elements declared, so it is still allocated at full size in one go and
nothing about the common path changes. Anything else grows as the elements
materialise. Neither of the two earlier failure modes returns: large containers
are not refused, and the declared count is not trusted.
The pure-Python decoder is not affected; it appends in a loop and never
preallocates. Of the other native accelerated implementations in the tree,
Ruby's already caps this (new_container_array in lib/rb/ext/struct.c) and PHP's
uses array_init with no size hint.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
