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

Check for binary content in formatContent() before a problematic regexp #676

Merged
merged 5 commits into from
Feb 26, 2024

Conversation

GreyWyvern
Copy link
Contributor

Type of pull request

  • Bug fix (involves code and configuration changes)

About

Some PDF files manage to serve binary content (a font, or an image, etc.) to the formatContent() method which only accepts text document streams.

Since (strings) in PDF document streams can contain binary content (that gets decoded by a font library, for instance) we must test the document stream for text-only content EXCLUDING (string) content.

Previously the check for text-only content was done after a regexp removing the (strings) from the stream, recursively extending the regexp to check for balanced parentheses. However for a sufficiently long binary stream, this might create a regexp long enough to cause a PHP error.

The solution is to move the check for binary content before this (string)-removing regexp. Simplify the binary check by truncating the document stream at the first open parenthesis, which indicates the start of a (string), then testing what remains for valid UTF-8.

This makes the later check for binary content unnecessary and it has been removed. Resolves issue #668.

Future work on this issue should be done to determine why PdfParser is creating PDFObject objects from binary data in the first place.

Checklist for code / configuration changes

  • Please add at least one test case (unit test, system test, ...) to demonstrate that the change is working. If existing code was changed, your tests cover these code parts as well.
  • Please run PHP-CS-Fixer before committing, to confirm with our coding styles. See https://github.com/smalot/pdfparser/blob/master/.php-cs-fixer.php for more information about our coding styles.
  • In case you fix an existing issue, please do one of the following:
    • Write in this text something like fixes #1234 to outline that you are providing a fix for the issue #1234.

Some PDF files manage to serve binary content (a font, or an image, etc.) to the formatContent() method which only accepts text document streams.

Since (strings) in PDF document streams can contain binary content (that gets decoded by a font library, for instance) we must test the document stream for text-only content EXCLUDING (string) content.

Previously the check for text-only content was done *after* a regexp removing the (string)s from the stream, recursively extending the regexp to check for balanced parentheses. However for a sufficiently long binary stream, this might create a regexp long enough to cause a PHP error.

The solution is to move the check for binary content *before* this (string)-removing regexp. Simplify the binary check by truncating the document stream at the first open parenthesis, which indicates the start of a (string), then testing what remains for valid UTF-8.

This makes the later check for binary content unnecessary and it has been removed.

Future work on this issue should be done to determine why PdfParser is creating PDFObject objects from binary data in the first place.
Avoid using a throwaway variable.
Don't update this indentation.
@k00ni
Copy link
Collaborator

k00ni commented Feb 15, 2024

I fixed coding style issues in #677, which got merged into master branch.

@k00ni k00ni merged commit 2939dfa into smalot:master Feb 26, 2024
29 checks passed
@k00ni
Copy link
Collaborator

k00ni commented Feb 26, 2024

@GreyWyvern Thanks again for your contribution.

@GreyWyvern GreyWyvern deleted the test-binary branch May 10, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

preg_match(): compilation failed: regular expression is too large to offset 143690
2 participants