Skip to content

fix: fill crash on a stray '}' before a template placeholder - #1090

Open
BigDataDZ wants to merge 1 commit into
apache:mainfrom
BigDataDZ:fix/fill-stray-suffix
Open

fix: fill crash on a stray '}' before a template placeholder#1090
BigDataDZ wants to merge 1 commit into
apache:mainfrom
BigDataDZ:fix/fill-stray-suffix

Conversation

@BigDataDZ

Copy link
Copy Markdown

Purpose of the pull request

Filling a template crashed with java.lang.StringIndexOutOfBoundsException whenever a template
cell contained a stray } before a {placeholder} (e.g. long prose or a JSON snippet in the
same cell as the placeholder — see #191, where the fill died with Range [1211, 1027)).

Fixes #191

What's changed?

In ExcelWriteFillExecutor#prepareData, the closing suffix of a placeholder was searched with
value.indexOf(FILL_SUFFIX, startIndex + 1). startIndex only tracks how far escaped
placeholders have been consumed, so the } search could start before the current {. A stray
} preceding the { was then treated as the placeholder's suffix, inverting the bounds of
value.substring(prefixIndex + 1, suffixIndex).

The suffix search now starts after the {:
value.indexOf(FILL_SUFFIX, Math.max(startIndex + 1, prefixIndex + 1)). A stray } is kept as
literal text (a}b{name} fills to a}bfilled); escape handling (\{, \}) and multi-placeholder
cells are unchanged.

Tests

New FillWithStraySuffixTest (round-trip): a template cell containing a}b{name} filled with
{name: "filled"} — crashes on current main, fills to a}bfilled with this change.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

Signed-off-by: BigDataDZ <76271875+BigDataDZ@users.noreply.github.com>

@skytin1004 skytin1004 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. I reproduced the failure and verified that this change fixes it. All tested cases passed. LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

列表填充报错

2 participants