Jens-G opened a new pull request, #3785:
URL: https://github.com/apache/thrift/pull/3785
Wrapping a transport in `TFramedTransport` or `TFastFramedTransport`
overwrote the maximum frame size it was configured with, even when no maximum
had been asked for:
```java
public TFramedTransport(TTransport transport) throws TTransportException {
this(transport, TConfiguration.DEFAULT_MAX_FRAME_SIZE);
}
public TFramedTransport(TTransport transport, int maxLength) throws
TTransportException {
TConfiguration _configuration = ... transport.getConfiguration();
_configuration.setMaxFrameSize(maxLength);
```
So
```java
socket.getConfiguration().setMaxFrameSize(1024);
TTransport framed = new TFramedTransport(socket);
```
left the socket configured for `16384000`.
Two things went wrong at once. A maximum the operator set was replaced by
the library default; and the object written into belongs to the transport
underneath, so anything else sharing that `TConfiguration` changed with it. A
layered transport has no configuration of its own to write into —
`TLayeredTransport.getConfiguration()` delegates to the inner transport by
design.
`TFastFramedTransport(underlying)` and `TFastFramedTransport(underlying,
initialBufferCapacity)` did the same, as did `TFramedTransport.Factory()` and
`TFastFramedTransport.Factory()` / `Factory(initialCapacity)` — so a server
configured with `new TFramedTransport.Factory()` raised the limit on **every
connection it accepted**.
### The change
A constructor or factory that was not given a maximum now passes back the
one the transport already carries, so wrapping changes nothing. Given one
explicitly, the behaviour is unchanged: it is applied, as the argument's
javadoc promises.
### The rest of the tree
Go solved this for its own deprecated constructors, and the reason is
written into its `TConfiguration`:
> `noPropagation` — *"Used internally by deprecated constructors, to avoid
overriding underlying TTransport/TProtocol's cfg by accidental propagations."*
C++ (`TFramedTransport` copies `configuration_->getMaxFrameSize()` into its
own `maxFrameSize_`), netstd, Delphi and Haxe all read the configured maximum
and never write it. Java was the only binding where wrapping wrote it.
### Tests
Four in `TestTFramedTransport`, 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 beforehand (`expected: <1024> but was:
<16384000>`, and *"Expected TTransportException to be thrown, but nothing was
thrown"*). The explicit-maximum one is a regression guard and passes either way.
Full `gradle test spotlessCheck`: BUILD SUCCESSFUL.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
