Repository navigation
Make PDF output reproducible #9175
Description
Activity
Could you explain why it is important to you to produce exactly the same file across versions? I'm concerned that this comment may not be your only obstacle to that goal, and that one day we might make a change like https://pillow.readthedocs.io/en/stable/releasenotes/9.0.0.html#switched-to-libjpeg-turbo-in-macos-and-linux-wheels again, and your images will change within the PDF.
Rather than adding a new argument, I'd personally be more interested in just removing the version string, and have 'created by Pillow PDF driver'. How does that sound?
Yes. The answer is "tests".
I have a backup/restore program which outputs PDFs in Pillow, and it is very easy to write tests if I can just check whether the output is literally the same hash as some expected output. (Currently I am doing this via monkey patching the Pillow code, which is not ideal.)
Of course this is not the only style of tests one can write, but it's a useful one.
Edit: Any such solution sounds great, I would have no objection to yours.
I've created #9176
The version was added back in PIL 1.1.1 from 2000, where it was a
PdfImagePlugin-specific number:We replaced all the plugin-specific numbers with the Pillow version in #4197.
No big objections from me to removing it. Before we do, does the PDF spec say anything about adding the creator tool version? What do other popular tools do?
https://opensource.adobe.com/dc-acrobat-sdk-docs/pdfstandards/PDF32000_2008.pdf states
Comments (other than the %PDF-n.m and %%EOF comments described in 7.5, "File Structure") have no semantics. They are not necessarily preserved by applications that edit PDF files.
So I don't think the spec places a lot of importance on them.
I think the more expected thing would be to use "/Creator"
Page 550 of that PDF shows
/Creator (Adobe FrameMarker 5.5.3 for Power Macintosh®)
GIMP and ImageMagick don't include any such data. Saving a PDF from macOS Preview, I see
/Creator (Preview)
Would it be reasonable to add a test of reproducible PDF output to the Pillow suite, as well? I agree stuff like images or fonts are too finicky, but something like a blank document? A test could provide safety rails against accidentally breaking anything.
It wouldn't have to be a "definitely don't break this" test, but at least one that mentions why, all else equal, it would be better not to break. My thought process is that this is the kind of effect on users most devs wouldn't actively think about while doing things like adding PDF comments.
Since we're doing work on the PDF plugin it sounds reasonable to add a test and test data, including a small PDF document.
I think we already test the PDF output in
Lines 192 to 332 in f9db7a3
def test_pdf_open(tmp_path: Path) -> None: # fail on a buffer full of null bytes with pytest.raises(PdfParser.PdfFormatError): PdfParser.PdfParser(buf=bytearray(65536)) # make an empty PDF object with PdfParser.PdfParser() as empty_pdf: assert len(empty_pdf.pages) == 0 assert len(empty_pdf.info) == 0 assert not empty_pdf.should_close_buf assert not empty_pdf.should_close_file # make a PDF file pdf_filename = helper_save_as_pdf(tmp_path, "RGB") # open the PDF file with PdfParser.PdfParser(filename=pdf_filename) as hopper_pdf: assert len(hopper_pdf.pages) == 1 assert hopper_pdf.should_close_buf assert hopper_pdf.should_close_file # read a PDF file from a buffer with a non-zero offset with open(pdf_filename, "rb") as f: content = b"xyzzy" + f.read() with PdfParser.PdfParser(buf=content, start_offset=5) as hopper_pdf: assert len(hopper_pdf.pages) == 1 assert not hopper_pdf.should_close_buf assert not hopper_pdf.should_close_file # read a PDF file from an already open file with open(pdf_filename, "rb") as f: with PdfParser.PdfParser(f=f) as hopper_pdf: assert len(hopper_pdf.pages) == 1 assert hopper_pdf.should_close_buf assert not hopper_pdf.should_close_file def test_pdf_append_fails_on_nonexistent_file() -> None: im = hopper("RGB") with tempfile.TemporaryDirectory() as temp_dir: with pytest.raises(OSError): im.save(os.path.join(temp_dir, "nonexistent.pdf"), append=True) def check_pdf_pages_consistency(pdf: PdfParser.PdfParser) -> None: assert pdf.pages_ref is not None pages_info = pdf.read_indirect(pdf.pages_ref) assert b"Parent" not in pages_info assert b"Kids" in pages_info kids_not_used = pages_info[b"Kids"] for page_ref in pdf.pages: while True: if page_ref in kids_not_used: kids_not_used.remove(page_ref) page_info = pdf.read_indirect(page_ref) assert b"Parent" in page_info page_ref = page_info[b"Parent"] if page_ref == pdf.pages_ref: break assert pdf.pages_ref == page_info[b"Parent"] assert kids_not_used == [] def test_pdf_append(tmp_path: Path) -> None: # make a PDF file pdf_filename = helper_save_as_pdf(tmp_path, "RGB", producer="PdfParser") # open it, check pages and info with PdfParser.PdfParser(pdf_filename, mode="r+b") as pdf: assert len(pdf.pages) == 1 assert len(pdf.info) == 4 assert pdf.info.Title == os.path.splitext(os.path.basename(pdf_filename))[0] assert pdf.info.Producer == "PdfParser" assert b"CreationDate" in pdf.info assert b"ModDate" in pdf.info check_pdf_pages_consistency(pdf) # append some info pdf.info.Title = "abc" pdf.info.Author = "def" pdf.info.Subject = "ghi\uabcd" pdf.info.Keywords = "qw)e\\r(ty" pdf.info.Creator = "hopper()" pdf.start_writing() pdf.write_xref_and_trailer() # open it again, check pages and info again with PdfParser.PdfParser(pdf_filename) as pdf: assert len(pdf.pages) == 1 assert len(pdf.info) == 8 assert pdf.info.Title == "abc" assert b"CreationDate" in pdf.info assert b"ModDate" in pdf.info check_pdf_pages_consistency(pdf) # append two images mode_cmyk = hopper("CMYK") mode_p = hopper("P") mode_cmyk.save(pdf_filename, append=True, save_all=True, append_images=[mode_p]) # open the PDF again, check pages and info again with PdfParser.PdfParser(pdf_filename) as pdf: assert len(pdf.pages) == 3 assert len(pdf.info) == 8 assert PdfParser.decode_text(pdf.info[b"Title"]) == "abc" assert pdf.info.Title == "abc" assert pdf.info.Producer == "PdfParser" assert pdf.info.Keywords == "qw)e\\r(ty" assert pdf.info.Subject == "ghi\uabcd" assert b"CreationDate" in pdf.info assert b"ModDate" in pdf.info check_pdf_pages_consistency(pdf) def test_pdf_info(tmp_path: Path) -> None: # make a PDF file pdf_filename = helper_save_as_pdf( tmp_path, "RGB", title="title", author="author", subject="subject", keywords="keywords", creator="creator", producer="producer", creationDate=time.strptime("2000", "%Y"), modDate=time.strptime("2001", "%Y"), ) # open it, check pages and info with PdfParser.PdfParser(pdf_filename) as pdf: assert len(pdf.info) == 8 assert pdf.info.Title == "title" assert pdf.info.Author == "author" assert pdf.info.Subject == "subject" assert pdf.info.Keywords == "keywords" assert pdf.info.Creator == "creator" assert pdf.info.Producer == "producer" assert pdf.info.CreationDate == time.strptime("2000", "%Y") assert pdf.info.ModDate == time.strptime("2001", "%Y") check_pdf_pages_consistency(pdf) It wouldn't have to be a "definitely don't break this" test, but at least one that mentions why, all else equal, it would be better not to break.
Just for the record, I don't think automated testing really makes that distinction.
Due to this code:
Pillow/src/PIL/PdfImagePlugin.py
Line 224 in 97a4d1f
It is not possible produce a bitwise identical PDF across versions. Would you accept an argument to set/disable this comment during generation?