-
Notifications
You must be signed in to change notification settings - Fork 291
move file parsing to single-pass static methods #1102
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.
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
Merged
Conversation
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
for the file 0002973.pdf in the test corpus we need to completely overhaul how initial xref parsing is done since we need to locate the xref stream by brute-force and this is currently broken. i wanted to take this opportunity to change the logic to be more imperative and less like the pdfbox methods with instance data and classes. currently the logic is split between the xref offset validator and parser methods and we call the validator logic twice, followed by brute-force searching again in the actual parser. we're going to move to a single method that performs the following steps: 1. find the first (from the end) occurrence of "startxref" and pull out the location in bytes. this will also support "startref" since some files in the wild have that 2. go to that offset if found and parse the chain of tables or streams by /prev reference 3. if any element in step 2 fails then we perform a single brute-force over the entire file and like pdfbox treat later in file-length xrefs as the ultimate arbiter of the object positions. while we do this we potentially can capture the actual object offsets since the xref positions are probably incorrect too. the aim with this is to avoid as much seeking and re-reading of bytes as possible. while this won't technically be single-pass it gets us much closer. it also removes the more strict logic requiring a "startxref" token to exist and be valid, since we can repair this by brute-force anyway. we will surface as much information as possible from the static method so that we could in future support an object explorer ui for pdfs. this will also be more resilient to invalid xref formats with e.g. comment tokens or missing newlines.
BobLd
requested changes
Aug 4, 2025
Collaborator
|
@EliotJones don't hesitate if you don't agree with my feedback, or if you need help |
Member
Author
|
@BobLd all good I just haven't had time to get back to this yet. I also need to check how it handles the case where the file contains object streams. |
Member
Author
|
@BobLd I've handled a couple of feedback items now and added the test I wanted to include. Let me know if you have any further feedback. |
This was referenced Nov 13, 2025
This was referenced Nov 14, 2025
This was referenced Nov 23, 2025
This was referenced Dec 1, 2025
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.
for the file 0002973.pdf in the test corpus we need to completely overhaul how initial xref parsing is done since we need to locate the xref stream by brute-force and this is currently broken. i wanted to take this opportunity to change the logic to be more imperative and less like the pdfbox methods with instance data and classes.
currently the logic is split between the xref offset validator and parser methods and we call the validator logic twice, followed by brute-force searching again in the actual parser. we're going to move to a single method that performs the following steps:
the aim with this is to avoid as much seeking and re-reading of bytes as possible. while this won't technically be single-pass it gets us much closer. it also removes the more strict logic requiring a "startxref" token to exist and be valid, since we can repair this by brute-force anyway.
we will surface as much information as possible from the static method so that we could in future support an object explorer ui for pdfs.
this will also be more resilient to invalid xref formats with e.g. comment tokens or missing newlines.