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

Incorrect behaviour of the ImageFile._safe_read function #5871

Closed
leon9812 opened this issue Dec 5, 2021 · 4 comments · Fixed by #5872
Closed

Incorrect behaviour of the ImageFile._safe_read function #5871

leon9812 opened this issue Dec 5, 2021 · 4 comments · Fixed by #5872

Comments

@leon9812
Copy link

leon9812 commented Dec 5, 2021

What did you do?

Randomly came across the ImageFile._safe_read fuction.

What did you expect to happen?

An "OSError: Truncated File Read" if the length of the file-like object fp is less than the value of the size parameter.

What actually happened?

Not always getting an OSError if the value for size is larger than ImageFile.SAFEBLOCK (and the length of fp is less than size).

What are your OS, Python and Pillow versions?

  • OS: Windows 10 (21H1)
  • Python: 3.8.10
  • Pillow: 8.4.0
import io
from PIL import ImageFile

fp = io.BytesIO(b"0" * ImageFile.SAFEBLOCK)
size = ImageFile.SAFEBLOCK + 1

# no error, but expected OSError:
content = ImageFile._safe_read(fp=fp, size=size)

print(len(content))
# 1048576
print(size)
# 1048577
@hugovk
Copy link
Member

hugovk commented Dec 5, 2021

Why are you calling _safe_read? It's a "private" function and not part of the public API, as denoted by the leading underscore.

Is there a concrete problem when calling this via the public API?

@leon9812
Copy link
Author

leon9812 commented Dec 5, 2021

I didn't find a problem via the public API. I just found this function because it raised an exception for an incompatible / truncated image and noticed, that it does not always do what its docstring describes. I just thought it is worth reporting so there are no problems in the future (regarding that function).

@radarhere
Copy link
Member

I've figured out a test case for this using our public APIs, and created #5872 accordingly.

@hugovk
Copy link
Member

hugovk commented Dec 7, 2021

Merged, thanks both!

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

Successfully merging a pull request may close this issue.

3 participants