Skip to content

Fix parseBuffer with offset buffers - #437

Open
dxg wants to merge 1 commit into
modesty:masterfrom
locomote:fix_buffer_with_offset
Open

dxg wants to merge 1 commit into
modesty:masterfrom
locomote:fix_buffer_with_offset

Conversation

@dxg

@dxg dxg commented Oct 8, 2026

Copy link
Copy Markdown

parseBuffer failed on buffers that are views into a larger ArrayBuffer at a non-zero byteOffset, because PDF.js's makeSubStream reads from bytes.buffer with absolute positions and the old guard rebuilt the buffer as another view instead of copying it.
Node 24.18 raised Buffer.poolSize to 64 KiB, so any Buffer.from/subarray result under 32 KiB now lands at an arbitrary pool offset and parsing fails with "Invalid XRef stream header".

Whilst I encountered the bug after a Node 24.15 -> 24.21 upgrade, tests don't depend on using the latest Node version.

Given that #424 hasn't moved forward in the last 3 months, I've created a new PR.

@modesty modesty left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

The actual bug is in the engine and is still there base/core/stream.js: Stream.makeSubStream builds sub-streams from this.bytes.buffer, which loses the view's byte offset. This is the root cause.

Changing it to new Stream(this.bytes, start, length, dict) passes all 12 new tests on Node 22 and 24 without any copying.

The approach in this PR copies the whole PDF on every parse of a buffer it doesn't own, doubling peak memory for large inputs. And it copies again each time a cached buffer is re-parsed, and copies small stream inputs twice; This would leave the same offset bug in place for other engine code that builds a Stream from a view (for example fonts.js:5437, fonts.js:5448). Fix the root cause would avoid the memory issue and other offset places would benefit too.

Currently, the new test fails 8 of 12 cases on master and passes all 12 on the PR branch, on both Node 22.23.3 and Node 24.21.0.

Comment thread pdfparser.js Outdated
return bytes;
}
return Buffer.from(new Uint8Array(bytes).buffer);
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this would break typed arrays other than Uint8Array : #toOwnedBuffer uses new
Uint8Array(bytes), which converts value by value instead of copying the raw bytes. So parseBuffer(new Uint16Array(pdfBytes.buffer)) parses on master but fails on this branch with "Invalid XRef stream".
The fix is to always build a Uint8Array view over the same bytes and, when a copy is needed, use
buffer.slice(byteOffset, byteOffset + byteLength).

@dxg
dxg force-pushed the fix_buffer_with_offset branch from 5a9af2c to 054a4dd Compare October 12, 2026 09:05
@dxg

dxg commented Oct 12, 2026

Copy link
Copy Markdown
Author

@modesty feedback addressed.

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