123123213weqw opened a new pull request, #4467:
URL: https://github.com/apache/rocketmq-dashboard/pull/4467

   ### Which Issue(s) This PR Fixes
   
   - Fixes #4287
   - Fixes #4167
   
   ### Brief Description
   
   `MessagePropertyDisplay` caps a property value at `MAX_PROPERTY_VALUE_CHARS` 
and marks the cut with `...`. The cap counted UTF-16 `char`s and the cut was a 
plain `substring`, so for a value holding a supplementary character at the 
boundary the result ended with an isolated high surrogate: the panel showed a 
replacement glyph at the end of an already abbreviated value, and the JSON body 
carried an invalid escape (`\uD83D` with no low surrogate).
   
   A `char` is not a code point. The cap is now counted in code points and the 
cut uses `offsetByCodePoints`, which always lands on a code point boundary, so 
a supplementary character is kept whole or dropped whole and is never split.
   
   **The cap is applied in both `abbreviate` and `hasOversizedProperty`**, and 
the class javadoc and the constant's javadoc now say code point, which is what 
both methods mean. This is the part @lizhimins asked for when closing the 
earlier variants of this fix on #4288:
   
   > please cover `hasOversizedProperty` and the javadoc in the same change — a 
fix that only pins `abbreviate` still lets an oversized value fall back to 
`value.length()` and split a surrogate pair
   
   and the code-point reading is the one he called the better reading of the 
constant's name. Counting chars in `hasOversizedProperty` while `abbreviate` 
counts code points would make the panel report a value as abbreviated that it 
leaves untouched, and the reverse.
   
   ### Supersedes #4370
   
   #4370 fixes the cut in `abbreviate` only and leaves `hasOversizedProperty` 
comparing `value.length()`, so it is the "only pins `abbreviate`" variant the 
review asked not to resubmit. It is authored by another account of mine; I 
cannot push to that fork, so rather than leave two patches on the same file I 
am resubmitting the complete change here and will close #4370. Say the word if 
you would rather keep #4370 and I will rebase this onto it instead.
   
   There is no web-side counterpart to change: 
`web/src/utils/messageProperties.ts` parses property text and does not 
re-truncate, so the server value is what the panel shows.
   
   ### How Did You Test This Change?
   
   ```
   cd server && mvn -B -ntp test
   [INFO] Tests run: 2430, Failures: 0, Errors: 0, Skipped: 0
   
   mvn -B -ntp test -Dtest=MessagePropertyDisplayTest
   [INFO] Tests run: 8, Failures: 0, Errors: 0, Skipped: 0
   ```
   
   The 17 errors reported next to the full run are the `@SpringBootTest` 
classes that need a reachable MySQL 8; they fail identically on the untouched 
baseline.
   
   Test coverage:
   
   - `limitPropertiesShouldNotSplitASurrogatePairTest` — the cap lands inside 
the emoji; asserts the result holds no unpaired surrogate and keeps the emoji 
whole.
   - `limitPropertiesShouldKeepASupplementaryCharacterThatFitsTheCapTest` — the 
boundary case just inside the cap.
   - `hasOversizedPropertyShouldCountCodePointsNotCharsTest` — a value of 
**1024 code points but 1624 chars**: `hasOversizedProperty` is false and 
`limitProperties` returns it untouched, which is what the char-based count got 
wrong; one more code point and both flip.
   - The existing ASCII tests still pass unchanged (`"x".repeat(1500)` still 
abbreviates to 1024 chars plus `...`).
   
   ### 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 — no new source files
   - [x] Documentation touched where behaviour changed — the javadoc of the 
constant and the class now state code points
   


-- 
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