[
https://issues.apache.org/jira/browse/THRIFT-6182?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Jens Geyer reassigned THRIFT-6182:
----------------------------------
Assignee: Jens Geyer
> Wrapping a transport must not raise the configured maximum frame size (Java)
> ----------------------------------------------------------------------------
>
> Key: THRIFT-6182
> URL: https://issues.apache.org/jira/browse/THRIFT-6182
> Project: Thrift
> Issue Type: Bug
> Components: Java - Library
> Reporter: Jens Geyer
> Assignee: Jens Geyer
> Priority: Major
> Fix For: 0.25.0
>
> Time Spent: 20m
> Remaining Estimate: 0h
>
> Wrapping a transport in TFramedTransport or TFastFramedTransport overwrites
> the maximum frame size the transport was configured with, even when no
> maximum was asked for.
> lib/java/src/main/java/org/apache/thrift/transport/layered/TFramedTransport.java:
> {code}
> public TFramedTransport(TTransport transport, int maxLength) throws
> TTransportException {
> super(transport);
> TConfiguration _configuration =
> Objects.isNull(transport.getConfiguration())
> ? new TConfiguration()
> : transport.getConfiguration();
> _configuration.setMaxFrameSize(maxLength);
> ...
> }
> public TFramedTransport(TTransport transport) throws TTransportException {
> this(transport, TConfiguration.DEFAULT_MAX_FRAME_SIZE);
> }
> {code}
> So
> {code}
> socket.getConfiguration().setMaxFrameSize(1024);
> TTransport framed = new TFramedTransport(socket);
> {code}
> leaves the socket configured for 16384000. Two things go wrong at once. The
> maximum an operator set is silently replaced by the library default, and the
> object it is written into belongs to the transport underneath, so anything
> else sharing that TConfiguration is changed too --
> TLayeredTransport.getConfiguration() delegates to the inner transport by
> design, so a layered transport has no configuration of its own to write into.
> The same holds for TFastFramedTransport(underlying) and
> TFastFramedTransport(underlying, initialBufferCapacity), and for
> TFramedTransport.Factory() and TFastFramedTransport.Factory() /
> Factory(initialCapacity), which all pass
> TConfiguration.DEFAULT_MAX_FRAME_SIZE to the constructor that applies it. A
> server configured with new TFramedTransport.Factory() therefore raises the
> limit on every connection it accepts.
> The fix is that a constructor or factory which was not given a maximum passes
> back the one the transport already carries, so wrapping changes nothing.
> Where a maximum is given explicitly the behaviour is unchanged: it is
> applied, as the argument's javadoc promises.
> Go solved this for its own deprecated constructors and the reason is written
> into TConfiguration: the noPropagation flag exists "to avoid overriding
> underlying TTransport/TProtocol's cfg by accidental propagations"
> (lib/go/thrift/configuration.go). C++, netstd, Delphi and Haxe all read the
> configured maximum and never write it. Java is the only binding where
> wrapping writes it.
> Tests are added to TestTFramedTransport and inherited by
> TestTFastFramedTransport, so both classes are covered: wrapping keeps a
> lowered maximum, the factory keeps it, an explicit maximum is still applied,
> and -- the point of the other three -- a frame declaring 2048 bytes is still
> refused by a transport configured for 1024 after being wrapped. Three of the
> four fail on each class before the change; the explicit-maximum one is a
> regression guard and passes either way.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
