[Bug] Negative frame data length causes IllegalArgumentException in ExchangeCodec
- Dominant language
- Java
- Stars
- 41.6k
- Forks
- 26.4k
- Avg merge
- 15h 13m
- Merged PRs (30d)
- 4
Description
### Pre-check
- [x] I am sure that all the content I provide is in English.
### Search before asking
- [x] I had searched in the [issues](https://github.com/apache/dubbo/issues?q=is%3Aissue) and found no similar issues.
### Apache Dubbo Component
Java SDK (apache/dubbo)
### Dubbo Version
Dubbo Java 3.3.6 (tag dubbo-3.3.6); the same code is unchanged on the 3.3 branch (checked today).
OpenJDK 21, Linux.
Protocol: dubbo.
### Steps to reproduce this issue
While testing how the dubbo protocol codec handles invalid frames locally, we found that a frame whose header declares a negative data length makes decode throw IllegalArgumentException before decodeBody is reached.
A complete minimal frame is 16 bytes:
```
da bb c2 00 ff ff ff ff ff ff ff ff ff ff ff ff
```
magic dabb, flag c2 (request, twoway, hessian2), requestId all ff, and the data length field at offset 12 set to ffffffff, which reads back as -1.
Passing these bytes to DubboCodec.decode, or writing them to a provider port over TCP, throws:
```
java.lang.IllegalArgumentException: length: -1
at org.apache.dubbo.remoting.buffer.ChannelBufferInputStream.(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)
```
On the netty4 transport the exception leaves the codec and is handled as a connection-level exception (logged, connection closed).
### What you expected to happen
A header declaring a negative data length is not a valid frame, and no additional input can make it one. decode should reject such a frame as invalid at the point the length is read, instead of letting a stream constructor throw an uncaught IllegalArgumentException from inside the codec. A sign check on len right after Bytes.bytes2int would cover it; returning NEED_MORE_INPUT would not fit here, since it would leave the channel waiting for input that can never make a negative length valid.
### Anything else
Root cause, in ExchangeCodec (dubbo-remoting-api, tag dubbo-3.3.6):
- line 119: the data length is read as a signed int (Bytes.bytes2int(header, 12))
- line 123: finishRespWhenOverPayload only compares against the payload upper bound, and only for responses
- line 129: the completeness check (readable < len + 16) passes for every negative len
- line 134: ChannelBufferInputStream rejects negative lengths with IllegalArgumentException, which escapes decode
We have the 16-byte reproducer and a direct codec call available as a minimal demo, and are willing to submit a PR with the check and a test.
### Do you have a (mini) reproduction demo?
- [x] Yes, I have a minimal reproduction demo to help resolve this issue more effectively!
### Are you willing to submit a pull request to fix on your own?
- [x] Yes I am willing to submit a pull request on my own!
### Code of Conduct
- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
Contributor guide
Research direction
Start in dubbo-remoting-api's ExchangeCodec.decode, focusing on the signed length read with Bytes.bytes2int at offset 12 and the subsequent completeness check. Use the provided 16-byte negative-length frame or direct codec reproduction, then add a regression test showing the invalid frame is rejected without an IllegalArgumentException escaping from the stream constructor.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- java
- Domain
- distributed-systems, networking
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100