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

Fix some conversion errors on non conform PDF #932

Merged
merged 5 commits into from
Jun 5, 2022

Conversation

pubpub-zz
Copy link
Collaborator

@pubpub-zz pubpub-zz commented May 31, 2022

tracked in #925

Iss due to an error in the stream pointing onto \n before the object

fix: move to next non whitespace

pubpub-zz added 2 commits May 31, 2022 20:11
track in py-pdf#925

Iss due to an error in the stream pointing onto \n before the object

fix: move to next non whitespace
@codecov
Copy link

codecov bot commented May 31, 2022

Codecov Report

Merging #932 (3e60f4c) into main (f261bad) will increase coverage by 4.50%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #932      +/-   ##
==========================================
+ Coverage   78.32%   82.83%   +4.50%     
==========================================
  Files          16       17       +1     
  Lines        4342     4106     -236     
  Branches      821      882      +61     
==========================================
  Hits         3401     3401              
+ Misses        754      515     -239     
- Partials      187      190       +3     
Impacted Files Coverage Δ
PyPDF2/_reader.py 81.71% <100.00%> (+4.55%) ⬆️
PyPDF2/_page.py 71.70% <0.00%> (-4.74%) ⬇️
PyPDF2/_adobe_glyphs.py 100.00% <0.00%> (ø)
PyPDF2/xmp.py 60.86% <0.00%> (+0.24%) ⬆️
PyPDF2/_utils.py 91.52% <0.00%> (+0.84%) ⬆️
PyPDF2/_merger.py 67.90% <0.00%> (+2.29%) ⬆️
PyPDF2/_security.py 97.40% <0.00%> (+2.73%) ⬆️
PyPDF2/_writer.py 88.00% <0.00%> (+8.98%) ⬆️
PyPDF2/generic.py 89.82% <0.00%> (+12.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba6614d...3e60f4c. Read the comment docs.

@MartinThoma MartinThoma added the is-robustness-issue From a users perspective, this is about robustness label Jun 1, 2022
@MartinThoma
Copy link
Member

MartinThoma commented Jun 2, 2022

@pubpub-zz Just a short heads-up: The reason why I didn't merge so far is copyright concerns. I have contacted the email mentioned in the PDF and asked for permission to add it to PyPDF2. If that permission is not given, we cannot add it.

Without the permission, I see three ways to merge the PR:

  1. Removing the PDF (and the test, sadly)
  2. Removing the PDF, but adding the PDF download within the test (instead of relying on the PDF being in the repo)
  3. Removing the PDF, commenting the Test out, pointing to the PDF

@MartinThoma
Copy link
Member

I fear that we will not get an answer and thus we will not be able to add this PDF.

@pubpub-zz I've tred to craft a PDF with that specific problem, but I failed. This is what I had:

    pdf_data = (
        b"%%PDF-1.7\n"
        b"1 0 obj << /Count 1 /Kids [4 0 R] /Type /Pages >> endobj\n"
        b"2 0 obj << >> endobj\n"
        b"3 0 obj << >> endobj\n"
        b"4 0 obj << /Contents 3 0 R /CropBox [0.0 0.0 2550.0 3508.0]"
        b" /MediaBox [0.0 0.0 2550.0 3508.0] /Parent 1 0 R"
        b" /Resources << /Font << >> >>"
        b" /Rotate 0 /Type /Page >> endobj\n"
        b"5 0 obj << /Pages 1 0 R /Type /Catalog >> endobj\n"
        b"xref 1 5\n"
        b"%010d 00000 n\n"
        b"%010d 00000 n\n"
        b"%010d 00000 n\n"
        b"%010d 00000 n\n"
        b"%010d 00000 n\n"
        b"trailer << /Root 5 0 R /Size 6 >>\n"
        b"startxref\n%d\n"
        b"%%%%EOF"
    )
    startx_correction = -1
    pdf_data = pdf_data % (
        pdf_data.find(b"1 0 obj") + startx_correction,
        pdf_data.find(b"2 0 obj") + startx_correction,
        pdf_data.find(b"3 0 obj") + startx_correction,
        pdf_data.find(b"4 0 obj") + startx_correction,
        pdf_data.find(b"5 0 obj") + startx_correction,
        # startx_correction should be -1 due to double % at the beginning indiducing an error on startxref computation
        pdf_data.find(b"xref") + startx_correction,
    )
    pdf_stream = io.BytesIO(pdf_data)
    reader = PdfReader(pdf_stream)

@pubpub-zz
Copy link
Collaborator Author

This is more tricky to reproduce: the fix deals with Objects stream. we can produce like that. :(
I've started to try to use pdftk but I think it will be difficult too...
we should focus on option 2...

preventive measurs for possible copyright issues
@pubpub-zz
Copy link
Collaborator Author

@MartinThoma, ready for merging 😉

@MartinThoma
Copy link
Member

@pubpub-zz Could you please run git rm resources/iss_925.pdf? The file is still in the PR and I cannot remove it without creating a new PR.

@MartinThoma MartinThoma merged commit 81a9987 into py-pdf:main Jun 5, 2022
MartinThoma added a commit that referenced this pull request Jun 6, 2022
The highlight of the 2.1.0 release is the most massive improvement to the
text extraction capabilities of PyPDF2 since 2016 🥳🎊 A very big thank you goes
to [pubpub-zz](https://github.com/pubpub-zz) who took a lot of time and
knowledge about the PDF format to finally get those improvements into PyPDF2.
Thank you 🤗💚

In case the new function causes any issues, you can use `_extract_text_old`
for the old functionality. Please also open a bug ticket in that case.

There were several people who have attempted to bring similar improvements to
PyPDF2. All of those were valuable. The main reason why they didn't get merged
is the big amount of open PRs / issues. pubpub-zz was the most comprehensive
PR which also incorporated the latest changes of PyPDF2 2.0.0.

Thank you to [VictorCarlquist](https://github.com/VictorCarlquist) for #858 and
[asabramo](https://github.com/asabramo) for #464 🤗

New Features (ENH):
-  Massive text extraction improvement (#924). Closed many open issues:
    - Exceptions / missing spaces in extract_text() method (#17) 🕺
      - Whitespace issues in extract_text() (#42) 💃
      - pypdf2 reads the hifenated words in a new line (#246)
    - PyPDF2 failing to read unicode character (#37)
      - Unable to read bullets (#230)
    - ExtractText yields nothing for apparently good PDF (#168) 🎉
    - Encoding issue in extract_text() (#235)
    - extractText() doesn't work on Chinese PDF (#252)
    - encoding error (#260)
    - Trouble with apostophes in names in text "O'Doul" (#384)
    - extract_text works for some PDF files, but not the others (#437)
    - Euro sign not being recognized by extractText (#443)
    - Failed extracting text from French texts (#524)
    - extract_text doesn't extract ligatures correctly (#598)
    - reading spanish text - mark convert issue (#635)
    - Read PDF changed from text to random symbols (#654)
    - .extractText() reads / as 1. (#789)
-  Update glyphlist (#947) - inspired by #464
-  Allow adding PageRange objects (#948)

Bug Fixes (BUG):
-  Delete .python-version file (#944)
-  Compare StreamObject.decoded_self with None (#931)

Robustness (ROB):
-  Fix some conversion errors on non conform PDF (#932)

Documentation (DOC):
-  Elaborate on PDF text extraction difficulties (#939)
-  Add logo (#942)
-  rotate vs Transformation().rotate (#937)
-  Example how to use PyPDF2 with AWS S3 (#938)
-  How to deprecate (#930)
-  Fix typos on robustness page (#935)
-  Remove scripts (pdfcat) from docs (#934)

Developer Experience (DEV):
-  Ignore .python-version file
-  Mark deprecated code with no-cover (#943)
-  Automatically create Github releases from tags (#870)

Testing (TST):
-  Text extraction for non-latin alphabets (#954)
-  Ignore PdfReadWarning in benchmark (#949)
-  writer.remove_text (#946)
-  Add test for Tree and _security (#945)

Code Style (STY):
-  black, isort, Flake8, splitting buildCharMap (#950)

Full Changelog: 2.0.0...2.1.0
@pubpub-zz pubpub-zz deleted the Fix925 branch August 8, 2022 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
is-robustness-issue From a users perspective, this is about robustness
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants