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

PdfWriter.add_named_destination() does not maintain the name tree sort order #1927

Closed
robertkearns opened this issue Jun 29, 2023 · 2 comments · Fixed by #1930
Closed

PdfWriter.add_named_destination() does not maintain the name tree sort order #1927

robertkearns opened this issue Jun 29, 2023 · 2 comments · Fixed by #1930
Labels
is-bug From a users perspective, this is a bug - a violation of the expected behavior with a compliant PDF needs-example-code The issue needs a minimal and complete (e.g. all imports) example showing the problem needs-pdf The issue needs a PDF file to show the problem

Comments

@robertkearns
Copy link
Contributor

robertkearns commented Jun 29, 2023

When a named destination is added via writer.add_named_destination() the resulting (name, destination) combo in the named destination list is always pushed to the back of the names list.

This will cause anything (annotations, etc.) using those destinations to not work correctly with some pdf viewers as they are relying on the name list being sorted in lexical order (which is required by the spec). add_named_destination_array() and add_named_destination_object() both insert the new destination at the correct index.

PDF Specification

To quote the "Table 36 – Entries in a name tree node dictionary" the part about the Names key:

The keys shall be sorted in lexical order, as described below. [...]

The Names entries in the leaf (or root) nodes shall contain the tree’s keys and their
associated values, arranged in key-value pairs and shall be sorted lexically in ascending order by key. Shorter
keys shall appear before longer ones beginning with the same byte sequence. Any encoding of the keys may
be used as long as it is self-consistent; keys shall be compared for equality on a simple byte-by-byte basis.

Sample Code

Creating a writer and adding 2 named destinations should be sufficient.

writer = PyPDF2.PdfWriter()
writer.add_blank_page(200, 200)
writer.add_named_destination('b', 0)
writer.add_named_destination('a', 0)
print(writer.get_named_dest_root()) # 'b' will come before 'a'

I do think the best proof of this however is reading the source code for PdfWriter.add_named_destination().

Affected Versions

This issue occurs in pypdf==3.11.1 and likely many prior versions

@pubpub-zz pubpub-zz added needs-pdf The issue needs a PDF file to show the problem needs-example-code The issue needs a minimal and complete (e.g. all imports) example showing the problem labels Jun 29, 2023
@pubpub-zz
Copy link
Collaborator

I agree this is a bug. Please create a PR and become a active contributor 🙂

@MartinThoma MartinThoma added the is-bug From a users perspective, this is a bug - a violation of the expected behavior with a compliant PDF label Jun 30, 2023
@py-pdf py-pdf deleted a comment from pubpub-zz Jun 30, 2023
@py-pdf py-pdf deleted a comment from robertkearns Jun 30, 2023
MartinThoma pushed a commit that referenced this issue Jun 30, 2023
…der (#1930)

To quote the "Table 36 – Entries in a name tree node dictionary" the part about the `Names` key:

> The keys shall be sorted in lexical order, as described below. [...]
>
> The Names entries in the leaf (or root) nodes shall contain the tree’s keys and their
associated values, arranged in key-value pairs and shall be sorted lexically in ascending order by key. Shorter
keys shall appear before longer ones beginning with the same byte sequence. Any encoding of the keys may
be used as long as it is self-consistent; keys shall be compared for equality on a simple byte-by-byte basis.

Fixes #1927
@MartinThoma
Copy link
Member

@pubpub-zz I love to see that you encourage people to get active as well ❤️ It's awesome that you're contributing that much, but on the long run we will need other contributors to ensure pypdf keeps improving :-) Well done 👏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is-bug From a users perspective, this is a bug - a violation of the expected behavior with a compliant PDF needs-example-code The issue needs a minimal and complete (e.g. all imports) example showing the problem needs-pdf The issue needs a PDF file to show the problem
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants