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

Fix ASCII85 decoding in a specific case #600

Merged
merged 1 commit into from Jun 12, 2023
Merged

Conversation

Seb35
Copy link
Contributor

@Seb35 Seb35 commented May 2, 2023

When the start or the end of the string is <~>. In this case we must be sure of the exact position of this substring: e.g. when there is no <~ header but <~> at the end, we must not remove the first two characters even if <~ is found somewhere.

Issue: #599

@Seb35
Copy link
Contributor Author

Seb35 commented May 3, 2023

It will be complicated to create a PDF whose the gzencode then ASCII85-encoding has the problematic ending, but I can add a test similarly to the existing ones whose the end of the decoded string is (hex) 215B 33C0 (=![3+0xC0) which is ASCII85-encoded as +^6b<~> – I found this in my original PDF.

@k00ni
Copy link
Collaborator

k00ni commented May 3, 2023

@Seb35 thank you for this pull request.

but I can add a test

That is sufficient, if it demonstrates the change. Providing a whole PDF is not required, most of the time it is only a bad trade in comparison to a tailored unit test.

@k00ni
Copy link
Collaborator

k00ni commented May 22, 2023

@Seb35 what is the status here?

@Seb35
Copy link
Contributor Author

Seb35 commented May 22, 2023

Sorry, I didn’t take the time to work on it, I do it soon (i.e. add a test).

@Seb35
Copy link
Contributor Author

Seb35 commented May 27, 2023

I added 3 tests:

  • the optional initial sequence <~
  • the optional final sequence ~>
  • the specific end sequence seemingly mixing initial and end sequences but legal since < and > are in the target characters (althought not ~ only authorised in the initial and final sequences)

PS: I was a bit surprised not to find PHPUnit in the root composer.json, but found the dev doc which is very clear, and I have to say I like this way of separating dev tools from ordinary dependencies.

@k00ni
Copy link
Collaborator

k00ni commented May 29, 2023

Good work!

Coding style rules changed with latest release of PHP-CS-Fixer. It should be sufficient to jut run it once to solve those errors (e.g. by using make run-php-cs-fixer).

PS: I was a bit surprised not to find PHPUnit in the root composer.json, but found the dev doc which is very clear, and I have to say I like this way of separating dev tools from ordinary dependencies.

Yeah, it was a pain in the ass to handle PHP version differences as well as drag PHPUnit along. Separate dev tools from library did us a good service ever since.

@Seb35
Copy link
Contributor Author

Seb35 commented May 30, 2023

Coding style rules changed with latest release of PHP-CS-Fixer. It should be sufficient to jut run it once to solve those errors (e.g. by using make run-php-cs-fixer).

I submitted #602 for that and will rebase this PR just after, to separate the two tasks.

When the start or the end of the string is "<~>". In this case
we must be sure of the exact position of this substring: e.g.
when there is no "<~" header but "<~>" at the end, we must not
remove the first two characters even if "<~" is found somewhere.

Issue: smalot#599
@Seb35
Copy link
Contributor Author

Seb35 commented Jun 10, 2023

Sorry for not answering before on the other PR, thanks for finalising it. I’ve done a simple rebase (no conflicts) on this one.

@k00ni k00ni merged commit fc01f29 into smalot:master Jun 12, 2023
26 checks passed
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.

Incorrect handling of "<~" in ASCII85 encoding
2 participants