klT45 opened a new pull request, #7854:
URL: https://github.com/apache/shenyu/pull/7854
Fixes #7853
## Changes
- Validate the rule handle in `RewritePluginDataHandler#handlerRule`. For
the plain-regex mode the regex is compiled and the replacement is dry-run
against it; when either fails the handle is rejected with an error log and the
cached rule is removed, instead of being cached and failing every matching
request. Placeholder-mode handles (`replace` and `regex` both contain `{`) are
accepted because the plugin substitutes them literally and never compiles the
regex; incomplete expressions are accepted because the plugin already skips
them at runtime.
- Guard the rewrite step in `RewritePlugin#doExecute`. A cached handle that
cannot be applied (e.g. a request path shorter than the placeholder offset) now
logs the failure and forwards the original URI instead of propagating the
exception out of the reactive chain.
- Include `percentage` in `RewriteHandle#equals/hashCode`; `toString`
already reported it but equality did not.
Handles that work today keep their behavior: `(test)` + `/new/$1` still
expands the captured group, `/shenyu/{id}` + `/target/{id}` still substitutes
literally, and blank or incomplete values still skip the rewrite.
## Tests
New regressions in `shenyu-plugin-rewrite` (all fail before the change):
- `RewritePluginTest#shouldNotThrowWhenReplacementReferencesMissingGroup` —
`test` + `/new/$1` throws `IndexOutOfBoundsException: No group 1` before the
change.
- `RewritePluginTest#shouldNotThrowWhenReplacementContainsBareDollar` —
`test` + `/v1/$$` throws `IllegalArgumentException` before the change.
-
`RewritePluginTest#shouldNotThrowWhenPathVariableRequestIsShorterThanPattern` —
`/shenyu/{id}` + `/target/{id}` for `/a` throws
`StringIndexOutOfBoundsException` before the change.
- `RewritePluginDataHandlerTest#testInvalidRegexIsNotCached`,
`#testInvalidReplacementIsNotCached`,
`#testInvalidHandleEvictsPreviouslyCachedRule` — fail before the change because
the invalid handle was cached.
- `RewritePluginDataHandlerTest#testPlaceholderHandleIsCached` — regression
guard: placeholder-mode handles must not be rejected.
- `RewriteHandleTest#testEqualsIncludesPercentage`.
## Verifying
- `./mvnw -pl shenyu-common,shenyu-plugin/shenyu-plugin-rewrite -am clean
install -Dmaven.javadoc.skip=true -Djacoco.skip=true` — full build of the
affected modules plus their dependencies, all tests pass (scoped equivalent of
the template's `clean install`).
- `./mvnw -pl shenyu-plugin/shenyu-plugin-rewrite test` — 23 tests, all pass.
- `./mvnw -pl shenyu-common test -Dtest=RewriteHandleTest` — 3 tests, all
pass.
- `./mvnw -pl shenyu-common,shenyu-plugin/shenyu-plugin-rewrite
apache-rat:check` — 0 unapproved licenses; checkstyle reports 0 violations for
both modules.
## Notes
- The validation dry-run appends an empty alternative (`regex + "|"`) so the
empty input matches and the JDK replacement parser actually inspects the
template; the group numbering of the configured regex is unchanged.
- Admin-side validation (rejecting the config when the rule is saved) is
deliberately left out so that the other configuration channels, such as direct
DB writes or the Kubernetes controller, are covered as well; it can be added
separately in `RuleServiceImpl#validateRuleHandle`.
- The second commit (`fix(common): include percentage in RewriteHandle
equals and hashCode`) is independent and can be split into its own PR if
preferred.
<!--
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 -pl
shenyu-common,shenyu-plugin/shenyu-plugin-rewrite -am clean install
-Dmaven.javadoc.skip=true`.
--
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]
