klT45 opened a new issue, #7853:
URL: https://github.com/apache/shenyu/issues/7853
## Current Behavior
`RewritePlugin` applies a rule handle with `String#replaceAll`:
```java
rewriteUri = rewriteUri.replaceAll(rewriteHandle.getRegex(),
rewriteHandle.getReplace());
```
The second argument is a replacement template, not a literal string: `$`
starts a capturing-group reference and `\` escapes the next character. The rule
handle is passed through as-is, so a `replace` that is not a valid template for
its `regex` throws on **every request that matches the rule**, and the request
fails instead of being forwarded to the upstream service.
Measured against current `master` (`RewritePlugin.java:80`, JDK 21;
`String#replaceAll` behaves identically on 17), rewriting `/shenyu/test`:
| `regex` | `replace` | result on a matching request |
| --- | --- | --- |
| `test` | `/new/$1` | `IndexOutOfBoundsException: No group 1` |
| `(test)` | `/new/$2` | `IndexOutOfBoundsException: No group 2` |
| `test` | `/price/$` | `IllegalArgumentException: Illegal group reference:
group index is missing` |
| `test` | `/v1/$$` | `IllegalArgumentException: Illegal group reference` |
| `test` | `/new/${group}` | `IllegalArgumentException: No group with name
{group}` |
| `(test)` | `/new/$1` | OK → `/shenyu//new/test` |
| `test` | `/price/\$` | OK → `/shenyu//price/$` |
A replacement containing a plain `$` (`/price/$`, `/v1/$$`) or a group
reference that the regex does not declare therefore turns a one-line
configuration mistake into a 100% error rate for that rule's traffic. Literal
`$` has to be escaped as `\$`, which is not documented anywhere in the plugin.
The placeholder branch has a second, narrower failure mode: with
`regex=/shenyu/{id}` and `replace=/target/{id}`, a matched request whose path
is shorter than the placeholder offset (`/a`) throws
`StringIndexOutOfBoundsException` from
`rewriteUri.substring(rewriteHandle.getRegex().indexOf("{"))` at
`RewritePlugin.java:79`. The guard added in #6951 only ensures that both values
contain `{`; it does not make the substring safe.
Nothing validates the handle on the way in either:
`RewritePluginDataHandler#handlerRule` (`:47-48`) parses the JSON into a
`RewriteHandle` and caches it unconditionally.
## Expected Behavior
- A rule handle that the plugin cannot apply should be rejected when it is
published/synced, with a clear error log, instead of being cached and failing
every matching request. This is the same expectation that #6509 raised for the
placeholder mode.
- If an unusable handle is cached anyway, the rewrite step should skip the
rewrite and forward the request with its original URI, so that one
misconfigured rule cannot break all traffic for the selector.
## Reproduction And Validation
1. Enable the rewrite plugin and create a selector + rule.
2. Configure the rule handle as:
```json
{"regex": "test", "replace": "/new/$1"}
```
3. Send `GET /shenyu/test` so the rule conditions match.
4. Every matching request fails with `IndexOutOfBoundsException: No group
1`. The exception is thrown inside the reactive chain in `doExecute`, so the
client receives a 500.
The reproduction runs the real `RewritePlugin#doExecute` with a mock
exchange and a terminal no-op chain; no backend service is needed. The same
handle is accepted by the admin API today.
## Location And Impact
-
[RewritePlugin.java:80](https://github.com/apache/shenyu/blob/8af03fc9bb6bc1cdd2a1077ac5274aedffa91d4b/shenyu-plugin/shenyu-plugin-rewrite/src/main/java/org/apache/shenyu/plugin/rewrite/RewritePlugin.java#L80)
— the replacement template is applied without validation; `$` is interpreted
as a group reference.
-
[RewritePlugin.java:79](https://github.com/apache/shenyu/blob/8af03fc9bb6bc1cdd2a1077ac5274aedffa91d4b/shenyu-plugin/shenyu-plugin-rewrite/src/main/java/org/apache/shenyu/plugin/rewrite/RewritePlugin.java#L79)
— the placeholder substring can exceed the request path.
-
[RewritePluginDataHandler.java:47](https://github.com/apache/shenyu/blob/8af03fc9bb6bc1cdd2a1077ac5274aedffa91d4b/shenyu-plugin/shenyu-plugin-rewrite/src/main/java/org/apache/shenyu/plugin/rewrite/handler/RewritePluginDataHandler.java#L47)
— the handle is parsed and cached without validation.
Impact: 500 for every request that reaches the rule, until the rule is
corrected in the dashboard. A valid rewrite to a literal `$` (prices,
templates) is not expressible without knowing the escaping rules.
## Suggested Fix
- Validate the handle in `RewritePluginDataHandler#handlerRule`: for the
plain-regex mode, compile the regex and dry-run the replacement against it; if
either fails, log an error and do not cache the handle.
- Guard the rewrite step in `RewritePlugin#doExecute` and forward the
original URI on failure as a last resort.
Do not "fix" this by always quoting the replacement: group references such
as `(test)` + `/new/$1` work today and must keep working.
## Environment And Related Issues
Apache ShenYu `2.7.2-SNAPSHOT`, `master` at
`8af03fc9bb6bc1cdd2a1077ac5274aedffa91d4b`, JDK 21 (`String#replaceAll`
semantics are the same on JDK 17).
- #6509 (closed) reported the same class of problem for placeholder mode and
asked for invalid rewrite config to be rejected at save/sync; only the
placeholder-mode condition was tightened.
- #6882 / #6951 (closed) fixed the placeholder off-by-one and the "replace
contains `{` but regex does not" mode; the replacement template and the
short-path substring remain uncovered.
- Checked open issue and PR titles for `RewritePlugin`, `rewrite
replacement`, `group reference`, `replaceAll`; no open issue or PR covers this.
--
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]
