Jens Geyer created THRIFT-6294:
----------------------------------
Summary: Delphi: add a container-size limit to the Thrift
configuration
Key: THRIFT-6294
URL: https://issues.apache.org/jira/browse/THRIFT-6294
Project: Thrift
Issue Type: Sub-task
Components: Delphi - Library
Reporter: Jens Geyer
Sub-task of THRIFT-6291. {{Thrift.Configuration.pas}} exposes
{{RecursionLimit}}, {{MaxFrameSize}} and {{MaxMessageSize}}, but nothing that
bounds the declared element count of a list, set or map. See THRIFT-6291 for
why the existing {{CheckReadBytesAvailable}} call does not cover that case.
h2. Suggested
* Add {{MaxContainerSize}} to {{IThriftConfiguration}} and
{{TThriftConfigurationImpl}} in {{Thrift.Configuration.pas}}, with the off
convention THRIFT-6291 settles.
* Enforce it in the three shared helpers in {{Thrift.Protocol.pas}}
({{CheckReadBytesAvailable}} overloads for {{TThriftList}}, {{TThriftSet}} and
{{TThriftMap}}, declared at lines 284-286, implemented from line 700). All nine
call sites — binary, compact and JSON, each for list, set and map — already go
through them, so no protocol needs its own check.
* {{TProtocolImpl.Create}} already caches
{{aTransport.Configuration.RecursionLimit}} into a field; cache the new setting
the same way.
* Raise {{TProtocolExceptionSizeLimit}} when the count exceeds the limit,
before any element is read.
* Off by default. No message accepted today may be rejected afterwards.
h2. Note on the interface
{{IThriftConfiguration}} is a public interface with a fixed GUID, so adding
accessors to it changes the vtable for anyone who implements the interface
themselves rather than deriving from {{TThriftConfigurationImpl}}. That is
expected to be rare, but it is a compatibility point worth calling out in the
pull request rather than discovering downstream. If it is judged too intrusive,
the alternative is to put the setting on {{TThriftConfigurationImpl}} only and
read it through an {{is}}/{{Supports}} test — uglier, and THRIFT-6291's
preference is the interface.
h2. Test
The test has to distinguish the new check from the existing byte-budget check,
which the unfixed code already has. Write a *complete and well-formed*
container of {{limit + 1}} single-byte elements, so every declared element
really is on the wire and {{CheckReadBytesAvailable}} is satisfied. The test
has to show two states — unfixed accepts the message, fixed raises a size-limit
exception — and the unfixed half must be *observed, not assumed*. Another guard
may fire first (the recursion limit, the frame size, or the remaining-message
budget as the test transport seeds it), which would quietly turn this into a
test that passes either way.
Cover binary, compact and JSON, each for list, set and map, and at both
{{limit}} and {{limit + 1}}.
_Drafted with AI assistance (Claude Opus 5); reviewed and filed by Jens Geyer._
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
