123123213weqw opened a new pull request, #4458:
URL: https://github.com/apache/rocketmq-dashboard/pull/4458
### Which Issue(s) This PR Fixes
- Fixes #4289
### Brief Description
Every Studio search box hands its value to MyBatis-Plus, which wraps it as
`%value%` and appends no `ESCAPE` clause, so the caller's own `%`, `_` and `\`
keep their pattern meaning:
- `search=%` matches every audit row (`operator`, `resource_name` or
`detail` `LIKE '%%'`).
- `search=_` matches every row whose column is a non-empty string.
- `search=100%_done` matches any row that merely contains `100` followed by
four arbitrary characters.
The result set widens instead of narrowing, which is the opposite of what an
operator typing those characters is asking for (SQL text, `a_b` style ids,
wildcard-ish names). On the audit page a full-table match also pushes the CSV
export over its 10 000-record cap and fails it with a 400 instead of returning
the filtered subset.
`common/util/SqlLikeUtils` escapes the three characters and builds the
predicate with an explicit `ESCAPE` clause, and it is applied once per value at
the boundary where a request parameter first becomes a query condition: auth
users, ACL users and rules, alert rules, audit logs, instances, cloud
credentials, data sources, the metadata provider topic and group searches, and
the message/trace query history that already carried a private copy of the
escaping rule. That private copy is now gone, so the repository keeps one
implementation.
This is one functional domain — "a search term must match literally" — so it
is one change rather than one patch per endpoint, per `CONTRIBUTING.md`.
`QueryHistoryService` is the only place that already escaped, and it is the
precedent for the shape of the helper.
**The escape character is spelled `CHAR(92)`, not a quoted backslash.** No
single quoted spelling works on both engines, and I measured rather than
assumed. H2 2.4.240 (the version `mvn test` resolves, in both the plain and the
`MODE=MySQL` mode the dev profile uses) and MySQL 8.0.46 through a JDBC
prepared statement:
```
H2 2.4.240 MySQL 8.0.46
ESCAPE '\' accepted syntax error near
''\'' at line 1
ESCAPE '\\' Error in LIKE ESCAPE accepted
ESCAPE CHAR(92) accepted accepted
```
That matters because the suite has no MySQL, so a clause that only H2
accepts passes CI and breaks production. `CHAR(92)` also stays correct when
MySQL runs with `NO_BACKSLASH_ESCAPES`, where the backslash is not the assumed
`LIKE` escape character either. The three hard-coded `notLikeRight`
system-topic exclusions in `RocketMQMetadataProvider` are deliberately left
alone; a test pins their wildcards so a later escape sweep cannot take them
over.
### How Did You Test This Change?
```
cd server && mvn -B -ntp test
[INFO] Tests run: 2434, Failures: 0, Errors: 0, Skipped: 0
```
2434 tests, 0 failures. The 17 errors reported alongside are the
`@SpringBootTest` classes that need a reachable MySQL 8
(`StudioApplicationTest`, `HealthProbeIntegrationTest`,
`AuthService*IntegrationTest`, `RmqAlertStateMapperIntegrationTest`); they fail
identically on the untouched baseline, which I checked by running
`StudioApplicationTest` on `master` before starting. `checkstyle` runs as part
of the build (it is what caught the first draft's unused imports).
Test coverage added:
- `SqlLikeUtilsTest` — the escaping of each wildcard, the escape character
itself and their combinations; blank and `null` handling, including that
`contains` answers `null` so a blank term cannot become a match-everything `%%`.
- `SqlLikeEscapingAgainstDatabaseTest` — runs the real predicate through H2
in **both** modes and asserts the literal match next to the over-matching
pattern (`%` matches 1 row escaped vs 5 unescaped; `100_done` matches 1 vs 3),
plus a case asserting that H2 rejects the `'\\'` spelling, which is the
executable half of the `CHAR(92)` choice.
- Wiring tests in six classes (`Auth` is covered by the ACL/audit set,
`RocketMQMetadataProviderTest`, `MybatisPlusSettingsRepositoryTest`,
`MybatisPlusAuditRepositoryTest`, `MybatisPlusInstanceRepositoryTest`,
`MybatisPlusAclRepositoryTest`, `MybatisPlusCloudCredentialRepositoryTest`)
asserting both the escaped bound value and that the emitted SQL carries `ESCAPE
CHAR(92)`.
-
`RocketMQMetadataProviderTest.paginatedTopicSearchShouldKeepTheHardCodedNotLikePatternsTest`
— the three system-topic exclusions keep their own wildcards.
Red/green, with the call sites reverted to `master` and the tests kept:
```
RED Tests run: 116, Failures: 7
Expecting actual:
"(name LIKE #{ew.paramNameValuePairs.MPGENVAL1}) ORDER BY
gmt_modified DESC,id DESC"
to contain:
["LIKE", "ESCAPE CHAR(92)"]
but could not find:
["ESCAPE CHAR(92)"]
GREEN Tests run: 141, Failures: 0, Errors: 0, Skipped: 0
```
### Overlap with other open pull requests
- **#4393** covers the same defect on `MybatisPlusSettingsRepository` and
`RocketMQMetadataProvider` and uses `ESCAPE '\'`, which is the spelling that
fails on MySQL in the table above; it also leaves auth, ACL, alert, audit,
instance and credential searches unescaped. This change is a superset of its
two files. I would rather converge on one of the two spellings than leave the
domain half-escaped or ship a clause that only H2 accepts, so say the word and
I will switch to your preferred form.
- **#4392** covers the audit path alone with the escaping but no `ESCAPE`
clause. I have not touched it; it is left for you to place.
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`fix:`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [x] New UI text has both Chinese and English entries under `web/src/i18n/`
— not applicable, no UI text added
- [x] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header
- [x] Documentation touched where behaviour changed — no document describes
the previous wildcard behaviour, so none needed updating
--
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]