Bug 105674 - Spurious warning in PDFDoc::checkHeader() with tiny PDF files
Summary: Spurious warning in PDFDoc::checkHeader() with tiny PDF files
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-03-22 02:55 UTC by Evangelos Foutras
Modified: 2018-04-02 18:14 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
[PATCH] Fix PDFDoc::checkHeader() for PDFs smaller than 1 KiB (1.79 KB, patch)
2018-03-22 02:57 UTC, Evangelos Foutras
Details | Splinter Review
Sample PDF under 1 KiB for testing (967 bytes, application/pdf)
2018-04-02 11:54 UTC, Evangelos Foutras
Details
[PATCH v2] Fix PDFDoc::checkHeader() for PDFs smaller than 1 KiB (1.97 KB, patch)
2018-04-02 13:47 UTC, Evangelos Foutras
Details | Splinter Review

Description Evangelos Foutras 2018-03-22 02:55:52 UTC
The fix for bug 104502 causes small PDF files (under 1024 bytes) to print a warning. This in turn triggers test suite failures in pdf2djvu because it uses really tiny PDFs in three of its tests.

My proposed patch makes use of an idiom also found elsewhere in PDFDoc.cc; it reads up to headerSearchSize bytes and stops if it encounters EOF, null-terminates the string in the buffer at the current position and continues to process the data it has read up to that point.

With this patch on top of Poppler 0.63.0, the test suite of pdf2djvu 0.9.8 passes successfully. Hope the patch is correct; I don't trust me not to introduce off-by-one errors. FWIW, the tests pass and Evince displays the correct PDF version.
Comment 1 Evangelos Foutras 2018-03-22 02:57:20 UTC
Created attachment 138268 [details] [review]
[PATCH] Fix PDFDoc::checkHeader() for PDFs smaller than 1 KiB
Comment 2 Albert Astals Cid 2018-04-02 10:59:50 UTC
Can you attach such a small pdf for testing?

Also what do you think of introducing a GBool headerFound instead of int n? 

And then make the check be over headerFound instead of i >= n - 5, i think it would make the code easier to understand.
Comment 3 Evangelos Foutras 2018-04-02 11:54:04 UTC
Created attachment 138490 [details]
Sample PDF under 1 KiB for testing

I tried to mimic what other parts of the code did, but you're right in that it can be made much more readable. I'll have another go at it with this in mind.

Attaching a sample PDF for testing purposes (generated by one of pdf2djvu's tests).

The warning can be seen by running pdfinfo on it (outputs a warning and fails to parse the document's PDF version):

  $ pdfinfo test-empty-outline.pdf 
  Syntax Warning: EOF while reading header (continuing anyway)
  [..]
  PDF version:    0.0
Comment 4 Evangelos Foutras 2018-04-02 13:47:07 UTC
Created attachment 138492 [details] [review]
[PATCH v2] Fix PDFDoc::checkHeader() for PDFs smaller than 1 KiB

Same approach, more readable code. Testing with `./utils/pdfinfo test-empty-outline.pdf` shows the correct PDF version and doesn't print any warnings.

Thanks for the feedback on these patches.
Comment 5 Albert Astals Cid 2018-04-02 18:14:34 UTC
Pushed, thanks :)


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.