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

Calling pdf.image() might closes passed BytesIO object #881

Closed
Niwo1403 opened this issue Aug 10, 2023 · 6 comments
Closed

Calling pdf.image() might closes passed BytesIO object #881

Niwo1403 opened this issue Aug 10, 2023 · 6 comments

Comments

@Niwo1403
Copy link

If pdf.image() gets called with a BytesIO object this object might be closed while processing (making the bytes in it unavailable after the execution of the method). Whether the object will be closed depends on the image format (e.g. PNG will be closed, JPG not). Since this might break compatibility with previous versions and acts differently depending on the image format, I assume this is not done on purpose, right?

Error details

Raised error:

Traceback (most recent call last):
  File ".../runfile.py", line 15, in <module>
    pdf.image(image, h=80)
  File "...\venv\Lib\site-packages\fpdf\fpdf.py", line 217, in wrapper
    return fn(self, *args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^
  File "...\venv\Lib\site-packages\fpdf\fpdf.py", line 3766, in image
    if isinstance(name, io.BytesIO) and _is_svg(name.getvalue().strip()):
                                                ^^^^^^^^^^^^^^^
ValueError: I/O operation on closed file.

Reason for error:

  1. Here the BytesIO object is used to create a PIL Image object.
  2. The Image object gets used...
  3. Here the PIL Image object isn't required anymore and gets closed for some image formats (depending on the taken branch & return of the get_img_info method). Closing the PIL Image object then closes the BytesIO object as well (maybe accidentally).

First appearance of error:
I think this error was added in this commit around 5 months ago and is included since version 2.7.0.

Minimal code

# std
from io import BytesIO
# 3rd party
from fpdf import FPDF
from requests import get

# get image from URL and save it as BytesIO
WORKING_LINK = "https://cdn.mos.cms.futurecdn.net/5PMe5hr8tjSS9Nq5d6Cebe.jpg"
NOT_WORKING_LINK = "https://logos-world.net/wp-content/uploads/2021/10/Python-Logo.png"
image = BytesIO(get(NOT_WORKING_LINK).content)
# create a pdf containing two times the same image, passed as BytesIO
pdf = FPDF()
pdf.add_page()
pdf.image(image, h=80)
pdf.image(image, h=80)  # raises ValueError
pdf.output(name="./out.pdf")

Environment

  • Operating System: Windows 10
  • Python version: 3.11.0
  • fpdf2 version used: 2.7.5 & current master
  • (requests version: 2.28.2)
@Niwo1403 Niwo1403 added the bug label Aug 10, 2023
@Lucas-C
Copy link
Member

Lucas-C commented Aug 13, 2023

Thank you for the detailed report @Niwo1403!

Since this might break compatibility with previous versions and acts differently depending on the image format, I assume this is not done on purpose, right?

Agreed!

I introduced that change in PR #721 as an attempt to avoid memory leaks.
But this was probably not done right...

We should only close BytesIO instances created by fpdf2!

@Lucas-C
Copy link
Member

Lucas-C commented Aug 13, 2023

Closing the PIL Image object then closes the BytesIO object as well (maybe accidentally).

This is indeed a side effect of PIL.Image.close():

print(img_bytes.closed)  # False
img = Image.open(img_bytes)
img.close()
print(img_bytes.closed)  # True

It's done there in Pillow source code:
https://github.com/python-pillow/Pillow/blob/10.0.0/src/PIL/Image.py#L551

@Lucas-C
Copy link
Member

Lucas-C commented Aug 13, 2023

I opened #887 to fix this

@Lucas-C
Copy link
Member

Lucas-C commented Aug 13, 2023

Fixed!

While waiting for a new fpdf2 release including this bugfix,
you can already test it by installing fpdf2 from git:

pip install git+https://github.com/py-pdf/fpdf2.git@master

@Lucas-C
Copy link
Member

Lucas-C commented Aug 13, 2023

@allcontributors please add @Niwo1403 for bug

@allcontributors
Copy link

@Lucas-C

I've put up a pull request to add @Niwo1403! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants