Sean-Walker0 opened a new pull request, #7281:
URL: https://github.com/apache/shenyu/pull/7281
<!-- Describe your PR here; e.g. Fixes #issueNo -->
Two public setters are no-ops because of a parameter/field name mismatch —
the right-hand side of each assignment resolves to the **field itself**, so the
field is assigned to itself and the caller's value is silently dropped:
```java
// shenyu-common, SofaUpstream
public void setRegister(final String registry) {
this.register = register; // reads the field, not the parameter
}
// shenyu-plugin-logging-common, CommonLoggingRuleHandle
public void setMaskType(final String desensitizeType) {
this.maskType = maskType; // reads the field, not the parameter
}
```
Runtime proof on current master: `setRegister("zookeeper://...")` followed
by `getRegister()` returns `null`.
Neither setter has any caller today, and both rule-handle DTOs are populated
in production via Gson reflection (which writes fields directly instead of
calling setters) — which is why the defect went unnoticed. Any caller using the
setters (tests, SDK users, refactors) gets `null` back and fails downstream:
e.g. `DigestUtils.md5Hex(sofaUpstream.getRegister())` NPEs while building the
sofa reference cache key (hit this while writing tests for #7217).
<!--
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-common,shenyu-plugin/shenyu-plugin-logging/shenyu-plugin-logging-common
-am` and `./mvnw checkstyle:check` on both modules (module-scoped; full build
left to CI).
### Modifications
- `SofaUpstream#setRegister`: assign the `registry` parameter.
- `CommonLoggingRuleHandle#setMaskType`: assign the `desensitizeType`
parameter.
- Add `SofaUpstreamTest` / `CommonLoggingRuleHandleTest` setter round-trip
tests (the existing suites populate DTOs via Gson and never exercised the
setters).
### Verifying this change
- `shenyu-common`: full module suite passed, new `SofaUpstreamTest` 1/1.
- `shenyu-plugin-logging-common`: 49/49 passed, new
`CommonLoggingRuleHandleTest` 1/1 (set then get, plus null reset).
- Checkstyle passed on both modules.
### Notes
- Found with a repo-wide scan for setter assignments whose right-hand side
is neither a parameter nor a literal; the only other hit,
`ShenyuMcpServerTool#setRequestConfig`, is a false positive (null-clearing
guard branch that uses the parameter on the next line).
- Related observation (not changed here): `SofaPluginDataHandler` reads
`ApplicationConfigCache#getUpstream(selectorData.getId())` while
`UPSTREAM_CACHE_MAP` is keyed by the full reference cache key, so that lookup
never hits — worth a separate issue.
--
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]
