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]

Reply via email to