Skip to content

Bug fix for issue 745 : Fix string duplication on escaped chunk boundaries#746

Merged
satabin merged 2 commits into
gnieh:mainfrom
dwalend:fix/string-chunk-escape-duplication
Mar 13, 2026
Merged

Bug fix for issue 745 : Fix string duplication on escaped chunk boundaries#746
satabin merged 2 commits into
gnieh:mainfrom
dwalend:fix/string-chunk-escape-duplication

Conversation

@dwalend

@dwalend dwalend commented Mar 4, 2026

Copy link
Copy Markdown

Bug #745 tokens[F, String] has the same chunk-boundary escape duplication bug as #515

Description

Issue #515 (fixed in 1.8.1 by commit 18a8ce2) fixed a bug where chunk boundaries falling inside JSON escape sequences caused string content duplication. The fix added T.mark(context) in slowString_ (the slow path) after seeing a backslash, but missed the identical pattern in string_ (the fast path) at line 55-58.

When the fast-path string_ encounters a backslash, it calls appendMarked, advance, then enters slowString_ -- but without calling mark first. If the chunk boundary falls right after the backslash, slowString_ calls appendMarked again using the stale mark position, re-appending already-consumed content.

This affects tokens[F, String] because String chunks are typically small (one per emit). The original #515 test used tokens[F, Byte] with chunkLimit(55), which happened to align chunks differently and not hit the fast-path code.

Fix

One line -- add T.mark(context) in string_ after seeing a backslash, matching what slowString_ already does:

--- a/json/src/main/scala/fs2/data/json/internal/JsonTokenParser.scala
+++ b/json/src/main/scala/fs2/data/json/internal/JsonTokenParser.scala
@@ -55,6 +55,7 @@
         case '\\' =>
           T.appendMarked(context, acc)
           T.advance(context)
+          T.mark(context)
           slowString_(key, StringState.SeenBackslash, 0, acc)

PR with fix and tests: closes #745.

@dwalend dwalend requested a review from a team as a code owner March 4, 2026 22:17
@satabin

satabin commented Mar 9, 2026

Copy link
Copy Markdown
Member

Hi @dwalend, thanks for reporting and proposing a fix for this. I will look into this change later today.

@dwalend

dwalend commented Mar 9, 2026

Copy link
Copy Markdown
Author

Thanks! Let me know if you want me to make any changes.

@satabin satabin left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just two small comments on the test, otherwise LGTM. Thanks a lot for spotting this.

Comment thread json/src/test/scala/fs2/data/json/Issue745Spec.scala Outdated
Comment thread json/src/test/scala/fs2/data/json/Issue745Spec.scala Outdated
@dwalend

dwalend commented Mar 13, 2026

Copy link
Copy Markdown
Author

Both changes are in. Thanks again for taking up my bug. Let me know if you want any other changes.

I've got an AI shaggy dog story to go with it. ("The left coprocessor does not know what the right coprocessor is up to.")

@satabin

satabin commented Mar 13, 2026

Copy link
Copy Markdown
Member

Thanks for the contribution. I will merge it and backport it to release 1.12.1 with this fix immediately.

@satabin satabin merged commit 3f6dff0 into gnieh:main Mar 13, 2026
13 checks passed
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.

tokens[F, String] has the same chunk-boundary escape duplication bug as #515

2 participants