L1nq0 opened a new pull request, #16448:
URL: https://github.com/apache/dubbo/pull/16448
## What is the purpose of the change?
Closes #16447
A frame header that declares a negative data length is not a valid frame,
and no additional input can make it one. On the 3.3 branch such a header passes
every check in ExchangeCodec.decode: finishRespWhenOverPayload only compares
against the payload upper bound, and the completeness check readable < len + 16
holds for every negative len. The negative value then reaches the
ChannelBufferInputStream constructor, which rejects it with an
IllegalArgumentException that escapes decode.
Stack trace from a 16-byte frame with data length ffffffff:
```
java.lang.IllegalArgumentException: length: -1
at
org.apache.dubbo.remoting.buffer.ChannelBufferInputStream.<init>(ChannelBufferInputStream.java:37)
at
org.apache.dubbo.remoting.exchange.codec.ExchangeCodec.decode(ExchangeCodec.java:134)
at
org.apache.dubbo.remoting.exchange.codec.ExchangeCodec.decode(ExchangeCodec.java:92)
```
What this PR does
decode now checks the sign of len right after Bytes.bytes2int(header, 12)
and rejects a negative value with an IOException, so an invalid header is
handled as a decode-level protocol error at the point the length is read
instead of surfacing as an unexpected exception from a stream constructor. On
the netty4 transport the connection-level behavior is unchanged: the exception
is caught by InternalDecoder, logged, and the connection is closed.
NEED_MORE_INPUT is deliberately not used here. It is only correct when more
input can complete the frame, and a negative length can never be completed.
The same guard is added to DeprecatedExchangeCodec in test sources:
CodecAdapterTest re-runs the ExchangeCodecTest suite through the old Codec
interface, so the copy needs the same behavior to keep the two paths aligned.
Testing
A new test, test_Decode_Negative_Data_Length, feeds the exact 16-byte
reproducer from the issue (magic dabb, flag c2, requestId all ff, data length
ffffffff) and asserts that decode rejects it with an IOException. Without the
fix this test fails with the stack above.
The full dubbo-remoting-api module suite (385 tests) and the
dubbo-remoting-netty4 module pass locally.
## Checklist
- [x] Make sure there is a
[GitHub_issue](https://github.com/apache/dubbo/issues) field for the change.
- [x] Write a pull request description that is detailed enough to understand
what the pull request does, how, and why.
- [x] Write necessary unit-test to verify your logic correction. If the new
feature or significant change is committed, please remember to add sample in
[dubbo samples](https://github.com/apache/dubbo-samples) project.
- [x] Make sure gitHub actions can pass. [Why the workflow is failing and
how to fix it?](../CONTRIBUTING.md)
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
