ROB: Let _build_destination skip in case of missing /D key

[nickryand]

Calling _build_destination with a dictionary can cause it to error if the code which slices the array is executed. This skips the scenario where DictionaryObject does not have a "/D" key and is currently passed to _build_destination.

[MartinThoma]

Do you have a PDF / some code that shows the issue you want to fix?

[nickryand]

This is running against:

poetry show | grep pypdf
pypdf             3.14.0 A pure-python PDF library capable of splitting, me...

import sys

from pypdf import PdfReader, PdfWriter


def main():
    file = PdfReader(sys.argv[1])

    new_pdf = PdfWriter()
    new_pdf.append(fileobj=file, pages=(0, 20))

    with open("test_copy.pdf", "wb") as fp:
        new_pdf.write(fp)


if __name__ == "__main__":
    main()

$ python test.py test.pdf

Traceback (most recent call last):
  File "/tmp/pypdf/test.py", line 17, in <module>
    main()
  File "/tmp/pypdf/test.py", line 10, in main
    new_pdf.append(fileobj=file, pages=(0, 20))
  File "/tmp/py_env/.devenv/state/venv/lib/python3.10/site-packages/pypdf/_writer.py", line 2812, in append
    self.merge(
  File "/tmp/py_env/.devenv/state/venv/lib/python3.10/site-packages/pypdf/_utils.py", line 466, in wrapper
    return func(*args, **kwargs)
  File "/tmp/py_env/.devenv/state/venv/lib/python3.10/site-packages/pypdf/_writer.py", line 2904, in merge
    reader.named_destinations
  File "/tmp/py_env/.devenv/state/venv/lib/python3.10/site-packages/pypdf/_reader.py", line 513, in named_destinations
    return self._get_named_destinations()
  File "/tmp/py_env/.devenv/state/venv/lib/python3.10/site-packages/pypdf/_reader.py", line 777, in _get_named_destinations
    self._get_named_destinations(kid.get_object(), retval)
  File "/tmp/py_env/.devenv/state/venv/lib/python3.10/site-packages/pypdf/_reader.py", line 794, in _get_named_destinations
    dest = self._build_destination(key, value)  # type: ignore
  File "/tmp/py_env/.devenv/state/venv/lib/python3.10/site-packages/pypdf/_reader.py", line 1001, in _build_destination
    page, typ = array[0:2]  # type: ignore
  File "/tmp/py_env/.devenv/state/venv/lib/python3.10/site-packages/pypdf/generic/_data_structures.py", line 320, in __getitem__
    return dict.__getitem__(self, key).get_object()
TypeError: unhashable type: 'slice'

[pubpub-zz]

@nickryand can you provide the test.pdf file you are using ?

[nickryand]

I cannot provide it as it's part of a collection I'm working on for someone else. I can provide metadata information about it.

[pubpub-zz]

do you accept to send it by mail to @MartinThoma info@martin-thoma.de ? it will be easier for analysis

[MartinThoma]

@nickryand Your PR breaks CI and cannot be merged like this. See
pytest tests/test_reader.py::test_named_destination\[stored_directly\].

You could either remove the else: continue or change the block to:

                if isinstance(val, DictionaryObject):
                    if "/D" in val:
                        val = val["/D"].get_object()
                    else:
                        continue

[pubpub-zz]

do you accept to send it by mail to @MartinThoma info@martin-thoma.de ? it will be easier for analysis

Is this request still active?

[MartinThoma]

I haven't seen any e-mail, but even without that I can see that this kind of structure can be problematic. In general, we likely always have to assume that some key does not exist (even if the specs require it).

At some point I hope we can find more general solutions, e.g. defining the specs formally in code and defining the fallback (or exception) we raise if pypdf wants to access a key that does not exist.

[codecov]

Codecov Report

Patch coverage: 42.85% and project coverage change: +0.04% 🎉

Comparison is base (11ee648) 94.07% compared to head (acab21a) 94.12%.
Report is 38 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2018      +/-   ##
==========================================
+ Coverage   94.07%   94.12%   +0.04%     
==========================================
  Files          33       41       +8     
  Lines        7077     7391     +314     
  Branches     1413     1460      +47     
==========================================
+ Hits         6658     6957     +299     
- Misses        262      269       +7     
- Partials      157      165       +8     

Files Changed	Coverage Δ
pypdf/_reader.py	91.33% <42.85%> (-0.17%)	⬇️

... and 20 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[MartinThoma]

@nickryand Thank you for your contribution 🙏 It will be part of the pypdf release on 10.12.2023 (Sunday).

[MartinThoma]

@nickryand If you want I can add you to https://pypdf.readthedocs.io/en/latest/meta/CONTRIBUTORS.html :-)

[MartinThoma]

Oh, wow, I just noticed it took me way too long to merge this. I'm sorry for that 🙈