Repository navigation
fix(sheet): escape hex sequences in head and multi-variable fill cells - #1226
Open
Sean-Walker0 wants to merge 1 commit into
Open
Sean-Walker0 wants to merge 1 commit into
Sean-Walker0 wants to merge 1 commit into
Conversation
The default EscapeHexCellWriteHandler only sees cells that pass
through the converter flow (afterCellDataConverted), so two XLSX
write paths stored literal _xHHHH_ sequences unescaped and POI's
reader decoded them into the characters they name:
- head cells: WriteContextImpl wrote the head string via
setCellValue directly;
- multi-variable fill cells: ExcelWriteFillExecutor converted each
variable value but assembled the surrounding template text itself
and stored the joined string directly.
A head "_x0041_ col" round-tripped as "A col" and a fill cell
"p {a} s" with a=_x0041_ x as "p A x s".
Share the escape scanner as XlsxEscapeUtils.escape (the write half of
the existing utfDecode), have the handler delegate to it, apply it to
the head string for XLSX writes, and apply it to the joined fill
string for XLSX writes. Both paths stay outside the handler event
flow, keeping the documented CellWriteHandler call matrix untouched.
Verified with EscapeHexHeadAndFillWriteTest (both cases decode and
corrupt before the change and pass after), the existing escape suites
and WriteHandlerTest unchanged, plus the full fesod-sheet suite
(1049 tests) and spotless:check.
skytin1004
approved these changes
Oct 16, 2026
skytin1004
left a comment
Contributor
There was a problem hiding this comment.
@Sean-Walker0 Thanks for the fix! I verified it locally, and everything looks good. LGTM.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose of the pull request
The default
EscapeHexCellWriteHandleronly sees cells that pass through the converter flow (afterCellDataConverted), so two XLSX write paths stored literal_xHHHH_sequences unescaped, and POI's reader decoded them into the characters they name:WriteContextImpl.addOneRowOfHeadDataToExcelwrote the head string viasetCellValuedirectly — a head"_x0041_ col"round-tripped as"A col";ExcelWriteFillExecutorconverted each variable value but assembled the surrounding template text itself and stored the joined string directly — fillingp {a} switha = "_x0041_ x"round-tripped as"p A x s".This closes the coverage gap disclosed in the Notes of #1223 (data cells, streaming and in-memory, are covered there).
What's changed?
XlsxEscapeUtils.escape(String)— the write half of the existingutfDecode— and the handler delegates to it (single implementation for both directions, as the class javadoc already documents).WriteContextImplappliesXlsxEscapeUtils.escapeto head strings for XLSX writes.ExcelWriteFillExecutorapplies it to the joined multi-variable fill string for XLSX writes (the variable values there never pass throughafterCellDataConverted, so nothing is pre-escaped and double-escaping cannot occur).afterCellDataConvertedfor heads breaks the documentedCellWriteHandlercall matrix thatWriteHandlerTestpins (19/23 of its cases fail with the hook variant). Escaping stays a property of the default handler set for XLSX, now applied wherever text reaches a cell.Checklist
Verifying this change
EscapeHexHeadAndFillWriteTest(default handler registration, both red before, green after):headCellRoundTripsLiteralEscapeSequences—expected: <_x0041_ col> but was: <A col>before;fillMultiVariableCellRoundTripsLiteralEscapeSequences—expected: <p _x0041_ x s> but was: <p A x s>before.All existing escape suites pass unmodified (
EscapeHexCellWriteHandlerTest19,EscapeHexCellWriteHandlerRoundTripTest4,EscapeHexInMemoryWriteTest2) andWriteHandlerTest23/23 confirms the handler call matrix is unchanged../mvnw clean package -B -Dmaven.test.skip=false -pl fesod-common,fesod-shaded,fesod-sheet: 1049 tests, 0 failures, 0 errors../mvnw -pl fesod-sheet spotless:check: clean.Notes
_xHHHH_-shaped text are now preserved literally instead of being decoded by the reader. XLS and CSV writes are untouched (no_xHHHH_semantics there).ExcelWriteFillExecutor(prepareDatare-stored around line 512) re-emits template static text unescaped as well; folding it in would grow this change's scope and it needs a template-authoring-intent discussion, so it is left as is.