Skip to content
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

Support PDF files with magic byte in the header #451

Merged
merged 1 commit into from
Oct 21, 2024

Conversation

karthik-bashetty
Copy link
Contributor

No description provided.

@tonimelisma
Copy link
Collaborator

Hey @karthik-bashetty, thanks for this. I'm a bit confused, it looks like your PR adds two golden reference image GIFs? Was that intentional?

Is the only change that you define the first 1024 bytes of the buffer for the PDF? And then you're shuffling the if/else control structure a little bit?

Could you add one of the offending PDFs as an example golden test please? I'd love to merge if you can provide tests for this (even though it looks like a small change).

@coveralls
Copy link

Coverage Status

coverage: 73.557%. remained the same
when pulling 9b7e960 on karthik-bashetty:master
into d925c83 on davidbyttow:master.

@karthik-bashetty
Copy link
Contributor Author

Hi @tonimelisma

Yes the only change is to check the first 1024 bytes of the pdf and restructuring the pdf check in the if else.

I will be need to get approvals or find an alternative way to generate a non standard pdf to add a golden test. I will do that and update the PR.

As for the two two golden reference image GIFs, they were generated when i ran make test so added them to the diff. I have now removed the files.

@karthik-bashetty
Copy link
Contributor Author

Hi @tonimelisma,

Added the tests as requested. I could not add the files i had already as they had confidential data and redacting ended up removing the extra bytes at the start of the PDF. I found the test file attached at https://github.com/pdf-association/pdf20examples/blob/master/PDF%202.0%20with%20offset%20start.pdf.

@tonimelisma tonimelisma merged commit f0e2192 into davidbyttow:master Oct 21, 2024
1 check failed
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.

3 participants