Skip to content

fix: grow strBuf when reading long escaped strings, for issue #7671 #7676

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wenshao merged 1 commit into from
Aug 2, 2026
+4 −4
Merged

fix: grow strBuf when reading long escaped strings, for issue #7671 #7676

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/src/main/java/com/alibaba/fastjson2/JSONReaderUTF16.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3254,12 +3254,10 @@ public String readString() {
char[] strBuf = this.strBuf;
if (strBuf == null) {
strBuf = new char[stroff + 512];
this.strBuf = strBuf;
} else if (stroff > strBuf.length) {
int newCapacity = newCapacity(stroff, strBuf.length);
strBuf = new char[newCapacity];
this.strBuf = strBuf;
strBuf = new char[newCapacity(stroff, strBuf.length)];
}
this.strBuf = strBuf;
System.arraycopy(chars, start, strBuf, 0, stroff);

while (true) {
Expand All @@ -3272,6 +3270,7 @@ public String readString() {

if (stroff + 4 >= strBuf.length) {
strBuf = Arrays.copyOf(strBuf, newCapacity(stroff + 4, strBuf.length));
this.strBuf = strBuf;
Comment on lines 3272 to +3273

@wenshao wenshao Jul 26, 2026

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.

[Suggestion] No test gates these buffer-reuse write-backs (here and the twin at line 3302): removing either this.strBuf = strBuf; leaves the entire test suite green — utf16test1() creates a fresh JSONReader per iteration, Issue3989 asserts only one integer field, and no test references strBuf or parses two consecutive long escaped strings through the same reader instance. — Failure scenario: a future refactor of the mid-loop growth logic drops one of the assignments → every subsequent readString() on the same reader that needs a large buffer re-allocates from the stale smaller buffer instead of reusing the grown one (increased GC pressure / p99 latency for workloads parsing many long escaped strings), with no test detecting it; output correctness is unaffected.

Fix: add a test that parses two consecutive long escaped strings (e.g. >1024 \\ pairs) through a single JSONReaderUTF16 instance and asserts via reflection on the private strBuf field that the grown buffer was retained (reference equality, or length ≥ the second parse's requirement).

(Follow-up to the now-outdated thread on the removed Issue7671.java — the underlying gap persists, as the PR description acknowledges.)

— qwen3.8-max-preview via Qwen Code /review

}

IOUtils.putLongLE(strBuf, stroff, v);
Expand Down Expand Up @@ -3300,6 +3299,7 @@ public String readString() {
}
if (stroff == strBuf.length) {
strBuf = Arrays.copyOf(strBuf, newCapacity(stroff + 1, strBuf.length));
this.strBuf = strBuf;
}
strBuf[stroff++] = c;
offset++;
Expand Down
Loading