You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Aias00 (via GitHub)" <gi...@apache.org> on 2026/07/29 09:40:57 UTC
[PR] [ISSUE #10692] Tolerate malformed HAProxy port values [rocketmq]
Aias00 opened a new pull request, #10693:
URL: https://github.com/apache/rocketmq/pull/10693
### Which Issue(s) This PR Fixes
Fixes #10692
### Brief Description
`HAProxyMessageForwarder.buildHAProxyMessage` previously parsed source and destination ports with `Integer.parseInt` directly, both from proxy protocol attributes and from channel remote/local addresses. A malformed port could throw `NumberFormatException` and fail HAProxy message forwarding with a runtime parsing exception.
This PR adds safe port parsing. Invalid port values now make `buildHAProxyMessage` return `null`, matching the existing behavior for unavailable HAProxy message data, while valid port values keep the existing behavior.
The test also replaces Java `assert` statements with JUnit assertions in `HAProxyMessageForwarderTest`.
### How Did You Test This Change?
```bash
mvn -pl proxy -Dtest=HAProxyMessageForwarderTest -DfailIfNoTests=false test
```
Result: `BUILD SUCCESS`, `Tests run: 3, Failures: 0, Errors: 0, Skipped: 0`.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10692] Tolerate malformed HAProxy port values [rocketmq]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #10693:
URL: https://github.com/apache/rocketmq/pull/10693#issuecomment-5144305603
Addressed the malformed PROXY metadata rejection coverage in `b0d3ee90a`.
Changes:
- Added `channelRead` regression coverage for out-of-range source/destination ports.
- Added `channelRead` regression coverage for missing and empty PROXY protocol attributes.
- These tests assert malformed metadata does not continue through `fireChannelRead`, does not forward outbound traffic, and closes the inbound channel.
Local verification passed:
`mvn -pl proxy -Dtest=HAProxyMessageForwarderTest -DfailIfNoTests=false 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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10692] Tolerate malformed HAProxy port values [rocketmq]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #10693:
URL: https://github.com/apache/rocketmq/pull/10693#issuecomment-5117067156
## [Codecov](https://app.codecov.io/gh/apache/rocketmq/pull/10693?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 `50.00000%` with `10 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 48.23%. Comparing base ([`00e45b8`](https://app.codecov.io/gh/apache/rocketmq/commit/00e45b8a6db23efbe756d0306f10716156cfd4dd?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`3f496a5`](https://app.codecov.io/gh/apache/rocketmq/commit/3f496a56ab74555401ae5e44bfa3757887467620?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/rocketmq/pull/10693?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...g/protocol/http2proxy/HAProxyMessageForwarder.java](https://app.codecov.io/gh/apache/rocketmq/pull/10693?src=pr&el=tree&filepath=proxy%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Frocketmq%2Fproxy%2Fremoting%2Fprotocol%2Fhttp2proxy%2FHAProxyMessageForwarder.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJveHkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3Byb3h5L3JlbW90aW5nL3Byb3RvY29sL2h0dHAycHJveHkvSEFQcm94eU1lc3NhZ2VGb3J3YXJkZXIuamF2YQ==) | 50.00% | [9 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/rocketmq/pull/10693?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #10693 +/- ##
=============================================
- Coverage 48.31% 48.23% -0.08%
+ Complexity 13511 13483 -28
=============================================
Files 1380 1380
Lines 101091 101107 +16
Branches 13101 13105 +4
=============================================
- Hits 48844 48773 -71
- Misses 46285 46344 +59
- Partials 5962 5990 +28
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/rocketmq/pull/10693?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.
</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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10692] Tolerate malformed HAProxy port values [rocketmq]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #10693:
URL: https://github.com/apache/rocketmq/pull/10693#issuecomment-5118629033
Addressed the Codecov patch coverage gap in the latest commit.\n\nChanges added coverage for:\n- local channel destination port parse failure\n- PROXY_PROTOCOL attribute path with valid ports\n- PROXY_PROTOCOL source and destination malformed port values\n- port range boundaries and out-of-range values\n\nI also tightened `parsePort` to reject values outside 0..65535, matching HAProxy port semantics.\n\nLocal verification passed:\n`mvn -pl proxy -Dtest=HAProxyMessageForwarderTest -DfailIfNoTests=false 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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10692] Tolerate malformed HAProxy port values [rocketmq]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #10693:
URL: https://github.com/apache/rocketmq/pull/10693#issuecomment-5138075286
Addressed the P1 fail-open concern in the latest commit `075b9cd49`.
Changes:
- Separate absent PROXY metadata from malformed PROXY metadata.
- Treat any present `proxy_protocol_*` metadata as requiring a complete valid source/destination address and port set.
- Reject malformed/incomplete PROXY metadata by closing the inbound channel and not calling `ctx.fireChannelRead(msg)`.
- Close the inbound channel when forwarding the HAProxy header fails, including `writeAndFlush(...).sync()` failure, cancellation, and interruption paths.
Regression coverage added for:
- malformed source and destination PROXY ports
- missing and empty individual PROXY attributes
- IPv6 PROXY attributes
- port boundaries `0` and `65535`
- `writeAndFlush(...).sync()` failure / cancellation / interruption
- asserting malformed metadata does not forward application traffic without the PROXY header
Local verification with JDK 8:
`JAVA_HOME=$(/usr/libexec/java_home -v 1.8) mvn -pl proxy -Dtest=HAProxyMessageForwarderTest -DfailIfNoTests=false test`
Result: `Tests run: 15, Failures: 0, Errors: 0, Skipped: 0`; checkstyle and spotbugs passed in the same run.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [ISSUE #10692] Tolerate malformed HAProxy port values [rocketmq]
Posted by "fuyou001 (via GitHub)" <gi...@apache.org>.
fuyou001 commented on PR #10693:
URL: https://github.com/apache/rocketmq/pull/10693#issuecomment-5128558180
[P1] Please avoid failing open when a malformed HAProxy port is present.
`buildHAProxyMessage` now returns `null` both when PROXY metadata is unavailable and when an advertised source/destination port is invalid. `forwardHAProxyMessage` treats `null` as "nothing to forward" and continues with `ctx.fireChannelRead(msg)`. Previously the parsing exception aborted this path; after this change, a connection carrying malformed PROXY metadata continues to the backend without a PROXY header.
That can replace the real client address with the proxy address downstream and affect source-IP ACLs, rate limiting, tenant attribution, and audit logs. Please distinguish "metadata absent" from "metadata present but invalid" and reject/close the latter connection (or propagate an explicit failure) rather than silently continuing.
Please add regression tests asserting that malformed source and destination ports do not call `ctx.fireChannelRead`, do not forward application traffic without the PROXY header, and close or reject the connection. Tests should also cover missing individual attributes, empty values, IPv6, port boundaries, and failures/cancellation/interruption from `writeAndFlush(...).sync()`.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org