Aias00 commented on PR #6892:
URL: https://github.com/apache/shenyu/pull/6892#issuecomment-5193352649
Clean fix — the double-increment in `Objects.isNull(protocols[i++]) ?
"dubbo://" : protocols[i++]` was a classic bug (the condition's `i++` always
advances, and the true branch advances again, skipping elements), correctly
resolved by computing `upstreamProtocol` once before the ternary. The AIOOBE
bounds check and the annotation null-guard are both correct.
I did a variant search across the k8s parsers:
`WebSocketParser.java:170-192` already has the correct guard pattern
(null-check + bounds-check + single evaluation), so this PR brings
`DivideIngressParser` and `DubboIngressParser` in line with that reference
implementation. No other parser carries the `[i++]` bug —
`SofaParser`/`GrpcParser` don't use the annotation-based protocol array, so
they're unaffected.
One minor pre-existing edge case (consistent across all parsers incl.
`WebSocketParser`, so not a regression): when the annotation value is an empty
string, `"".split(",")` returns `[""]`, so the first upstream gets `protocol =
""` rather than the `"http://"`/`"dubbo://"` fallback — the bounds check only
covers index-out-of-range, not empty entries. `testEmptyProtocolAnnotation`
only asserts `assertNotNull`, sidestepping this. A
`StringUtils.isBlank(protocols[i])` check would make empty entries fall back to
the default too. Fine to leave, just flagging for completeness.
--
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]
