Sean-Walker0 opened a new pull request, #7374:
URL: https://github.com/apache/shenyu/pull/7374
<!-- Describe your PR here; e.g. Fixes #issueNo -->
Found by code audit (no existing issue — happy to file one if maintainers
prefer).
`BasicAuthPluginDataHandler#handlerRule` wraps its parse in
`Optional.ofNullable(ruleData.getHandle()).ifPresent(ruleHandle -> ...)` and
then calls `StringUtils.defaultString(ruleHandle,
basicAuthConfig.getDefaultHandleJson())`. `defaultString` only substitutes when
the value is **null** — but inside `ifPresent` the handle is provably non-null,
so the configured `defaultHandleJson` is unreachable dead code (the whole
purpose of `handlerPlugin` storing `Constants.DEFAULT_HANDLE_JSON` into
`BasicAuthConfig` is to be used here). A rule saved with an **empty** handle —
the exact case the fallback exists for — reaches
`BasicAuthRuleHandle.newInstance("")`, whose Gson parse of the empty string
returns null, and the following
`basicAuthRuleHandle.setBasicAuthAuthenticationStrategy(...)` throws
`NullPointerException` inside the data-sync callback, failing the whole rule
refresh.
<!--
Thank you for proposing a pull request. This template will guide you through
the essential steps necessary for a pull request.
-->
Make sure that:
- [x] You have read the [contribution
guidelines](https://shenyu.apache.org/community/contributor-guide).
- [x] You submit test cases (unit or integration tests) that back your
changes.
- [x] Your local test passed `./mvnw test -pl
shenyu-plugin/shenyu-plugin-security/shenyu-plugin-basic-auth -am and ./mvnw
checkstyle:check -pl
shenyu-plugin/shenyu-plugin-security/shenyu-plugin-basic-auth` (module-scoped;
full build left to CI).
### Modifications
- `StringUtils.defaultString(...)` → `StringUtils.defaultIfBlank(...)`:
blank rule handles now parse the plugin-level default handle JSON, restoring
the intended fallback semantics (one-word change).
### Verifying this change
- New `testHandlerRuleFallsBackToDefaultHandleJsonWhenRuleHandleIsBlank`
saves a rule with `handle = ""` after configuring a default handle, and asserts
the cached handle carries the default's authorization. It fails on current
master with `NullPointerException: Cannot invoke
"BasicAuthRuleHandle.setBasicAuthAuthenticationStrategy(...)" because
"basicAuthRuleHandle" is null` and passes with this change.
- Full module suite green (182 test classes across the reactor); checkstyle
green.
### Notes
- Behavior change: rules with empty handles now resolve to the configured
default handle instead of crashing the rule refresh with an NPE.
- Orthogonal to open PRs: no open PR touches `BasicAuthPluginDataHandler`
(checked against the file lists of all 200 open PRs).
--
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]
