writer.add_page not pure?

[thomasuster]

add_page() looks like it transfers data but actually transfers a reference. The writer silently remains dependent on the reader's file handle, violating the user's reasonable expectation that after adding a page, the reader is no longer needed.

def test_writer_does_not_hold_references_to_reader_files():
    reader = PdfReader(RESOURCE_ROOT / "example.pdf")
    writer = PdfWriter()
    
    for page in reader.pages:
        writer.add_page(page)  # Looks like a copy, but holds references back to reader file?
    
    reader.close()  # Closes the file stream that writer's pages still point to
    
    with NamedTemporaryFile() as output_file:
        writer.write(output_file)  # throws exception

This smells to me, users will expect that add_page creates a deep copy of what it needs not a shallow one.

Actual test that fails can be found in this PR.

[stefan6419846]

Given the traceback in your PR, this is related to keeping the links working by copying them over from the source document to the destination document. As they might be split across multiple pages which are only added to the writer later, this is done during the writing process itself.

If you use

def test_writer_does_not_hold_references_to_reader_files():
    writer = PdfWriter(clone_from=RESOURCE_ROOT / "example.pdf")    
    with NamedTemporaryFile() as output_file:
        writer.write(output_file)  # throws exception

there is no such problem and might be more suitable for most use cases.

[thomasuster]

Yea that seems to work. It also appears to "fix" slightly broken pdf's which was the intent of what I was doing with reader/writer add_page. Can you confirm it's intended to do this?

def test_clone_from_repairs():
    writer = PdfWriter(clone_from=RESOURCE_ROOT / "issue-297.pdf") # This pdf would normally throw exception on strict=true

    with NamedTemporaryFile() as output_file:
        writer.write(output_file)
        PdfReader(output_file, strict=True) # But it doesn't (which is desired behavior)

[stefan6419846]

Can you confirm it's intended to do this?

Processing PDF files with pypdf might fix some existing issues, yes, as it does not generate an exact reproduction of the original file in terms of internal structure. (For completeness: There are more complex/reliable tools like Ghostscript or mutool clean for fixing such and more complex issues, although possibly modifying more content.)

[thomasuster]

Thanks!