[
https://issues.apache.org/jira/browse/THRIFT-6169?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer resolved THRIFT-6169.
--------------------------------
Assignee: Jens Geyer
Resolution: Fixed
> 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
> Assignee: Jens Geyer
> Priority: Major
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> 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)
