You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "RainYuY (via GitHub)" <gi...@apache.org> on 2026/07/31 07:37:54 UTC
[PR] fix: reject oversized telnet payload before header copy [dubbo]
RainYuY opened a new pull request, #16404:
URL: https://github.com/apache/dubbo/pull/16404
When the server-side `ExchangeCodec` receives non-Dubbo input, it currently allocates and copies the entire readable buffer before the Telnet payload limit is checked. An oversized input can therefore cause an unnecessary large byte-array allocation and copy.
This change validates the readable payload size in `ExchangeCodec.decode()` before allocating the header buffer for non-magic server-side input.
### What has changed?
- Added an early `checkPayload(channel, readable)` for non-magic server-side input.
- The validation is performed before `new byte[]` and `buffer.readBytes(...)`.
- Inputs with the Dubbo magic header continue through the existing Dubbo header/body decoding path.
- Client-side decoding and Telnet command parsing behavior remain unchanged.
- No cross-packet scan state or decoder structure was introduced.
### Tests
Added a regression test to verify that:
- An oversized non-magic input throws `ExceedPayloadLimitException`.
- The input buffer is not consumed when validation fails.
Executed:
```bash
mvn -pl dubbo-remoting/dubbo-remoting-api \
-DskipITs \
-Dcheckstyle.skip \
-Drat.skip=true \
-Dtest=ExchangeCodecTest \
test
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: reject oversized telnet payload before header copy [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on code in PR #16404:
URL: https://github.com/apache/dubbo/pull/16404#discussion_r3702344126
##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/codec/ExchangeCodec.java:
##########
@@ -87,6 +87,13 @@ public void encode(Channel channel, ChannelBuffer buffer, Object msg) throws IOE
@Override
public Object decode(Channel channel, ChannelBuffer buffer) throws IOException {
int readable = buffer.readableBytes();
+ if (readable > 0
+ && !isClientSide(channel)
Review Comment:
why not use ```isServerSide``` directly?
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: reject oversized telnet payload before header copy [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on code in PR #16404:
URL: https://github.com/apache/dubbo/pull/16404#discussion_r3702344126
##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/codec/ExchangeCodec.java:
##########
@@ -87,6 +87,13 @@ public void encode(Channel channel, ChannelBuffer buffer, Object msg) throws IOE
@Override
public Object decode(Channel channel, ChannelBuffer buffer) throws IOException {
int readable = buffer.readableBytes();
+ if (readable > 0
+ && !isClientSide(channel)
Review Comment:
why not use ```isServerSide``` directly?
```
if (isServerSide(channel)
&& (readable == 1
|| buffer.getByte(buffer.readerIndex()) != MAGIC_HIGH
|| buffer.getByte(buffer.readerIndex() + 1) != MAGIC_LOW)) {
checkPayload(channel, readable);
}
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: reject oversized telnet payload before header copy [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on PR #16404:
URL: https://github.com/apache/dubbo/pull/16404#issuecomment-5173996344
> ```
> if (isServerSide(channel)
> && readable > 1
> && (buffer.getByte(buffer.readerIndex()) != MAGIC_HIGH
> || buffer.getByte(buffer.readerIndex() + 1) != MAGIC_LOW)) {
> checkPayload(channel, readable);
> }
> ```
>
> It may be sufficient to only handle cases greater than 1
I don't think this change provides any meaningful behavioral benefit. When readable == 1, checkPayload is effectively a no-op because the smallest positive payload limit is 1, and the check only rejects size > payload. The current condition is correct and keeps the zero-readable case explicit, so I'd prefer to leave it as is.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: reject oversized telnet payload before header copy [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on PR #16404:
URL: https://github.com/apache/dubbo/pull/16404#issuecomment-5173391866
``
if (isServerSide(channel)
&& readable > 1
&& (buffer.getByte(buffer.readerIndex()) != MAGIC_HIGH
|| buffer.getByte(buffer.readerIndex() + 1) != MAGIC_LOW)) {
checkPayload(channel, readable);
}
```
It may be sufficient to only handle cases greater than 1
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: reject oversized telnet payload before header copy [dubbo]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #16404:
URL: https://github.com/apache/dubbo/pull/16404#issuecomment-5140731469
## [Codecov](https://app.codecov.io/gh/apache/dubbo/pull/16404?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `20.00000%` with `4 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 35.18%. Comparing base ([`3a30432`](https://app.codecov.io/gh/apache/dubbo/commit/3a3043227f5571d25eb2889de5bca22f2914843b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`04ecf16`](https://app.codecov.io/gh/apache/dubbo/commit/04ecf1669103c9639b1e0b9cec395cad1500d933?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo/pull/16404?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...e/dubbo/remoting/exchange/codec/ExchangeCodec.java](https://app.codecov.io/gh/apache/dubbo/pull/16404?src=pr&el=tree&filepath=dubbo-remoting%2Fdubbo-remoting-api%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fdubbo%2Fremoting%2Fexchange%2Fcodec%2FExchangeCodec.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9leGNoYW5nZS9jb2RlYy9FeGNoYW5nZUNvZGVjLmphdmE=) | 20.00% | [1 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/dubbo/pull/16404?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
> :exclamation: There is a different number of reports uploaded between BASE (3a30432) and HEAD (04ecf16). Click for more details.
>
> <details><summary>HEAD has 7 uploads less than BASE</summary>
>
>| Flag | BASE (3a30432) | HEAD (04ecf16) |
>|------|------|------|
>|integration-tests-java8|1|0|
>|samples-tests-java21|1|0|
>|unit-tests-java25|1|0|
>|unit-tests-java8|1|0|
>|unit-tests-java17|1|0|
>|unit-tests-java21|1|0|
>|unit-tests-java11|1|0|
></details>
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## 3.3 #16404 +/- ##
=============================================
- Coverage 60.87% 35.18% -25.70%
+ Complexity 11766 11167 -599
=============================================
Files 1953 1952 -1
Lines 89273 89256 -17
Branches 13473 13393 -80
=============================================
- Hits 54346 31401 -22945
- Misses 29333 53317 +23984
+ Partials 5594 4538 -1056
```
| [Flag](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [integration-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `32.08% <20.00%> (-0.06%)` | :arrow_down: |
| [integration-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [samples-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [samples-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `29.84% <20.00%> (+0.04%)` | :arrow_up: |
| [unit-tests-java11](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java17](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java21](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java25](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
| [unit-tests-java8](https://app.codecov.io/gh/apache/dubbo/pull/16404/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `?` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo/pull/16404?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: reject oversized telnet payload before header copy [dubbo]
Posted by "zrlw (via GitHub)" <gi...@apache.org>.
zrlw commented on code in PR #16404:
URL: https://github.com/apache/dubbo/pull/16404#discussion_r3702344126
##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/codec/ExchangeCodec.java:
##########
@@ -87,6 +87,13 @@ public void encode(Channel channel, ChannelBuffer buffer, Object msg) throws IOE
@Override
public Object decode(Channel channel, ChannelBuffer buffer) throws IOException {
int readable = buffer.readableBytes();
+ if (readable > 0
+ && !isClientSide(channel)
Review Comment:
why not use ```isServerSide``` directly?
```
if (isServerSide(channel)
&& (readable == 1
|| readable == 2
|| buffer.getByte(buffer.readerIndex()) != MAGIC_HIGH
|| buffer.getByte(buffer.readerIndex() + 1) != MAGIC_LOW)) {
checkPayload(channel, readable);
}
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix: reject oversized telnet payload before header copy [dubbo]
Posted by "RainYuY (via GitHub)" <gi...@apache.org>.
RainYuY commented on code in PR #16404:
URL: https://github.com/apache/dubbo/pull/16404#discussion_r3703987406
##########
dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/codec/ExchangeCodec.java:
##########
@@ -87,6 +87,13 @@ public void encode(Channel channel, ChannelBuffer buffer, Object msg) throws IOE
@Override
public Object decode(Channel channel, ChannelBuffer buffer) throws IOException {
int readable = buffer.readableBytes();
+ if (readable > 0
+ && !isClientSide(channel)
Review Comment:
I missed that. My mistake.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org