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.

Sign up for GitHub

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

Jump to bottom

If absent, do not try to close fp when closing image #7557

Merged
merged 4 commits into from
Nov 24, 2023
Merged

If absent, do not try to close fp when closing image #7557

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b25ece3
fix: check object Image has attribute 'fp' when closes
RaphaelVRossi Nov 16, 2023
67e1253
Do not assign new fp attribute to image when closing
radarhere Nov 16, 2023
5f33175
Fixed checking that no debug messages were logged
radarhere Nov 16, 2023
8186c63
Merge pull request #1 from radarhere/check-image-has-fp-when-close
RaphaelVRossi Nov 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions Tests/test_image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,21 @@ def test_fli_overrun2(self):
except OSError as e:
assert str(e) == "buffer overrun when reading image file"

@pytest.fixture(scope="function")
def inject_caplog(self, caplog):
self._caplog = caplog

@pytest.mark.usefixtures("inject_caplog")
def test_close_graceful(self):
RaphaelVRossi marked this conversation as resolved.
Show resolved Hide resolved
with Image.open("Tests/images/hopper.jpg") as im:
copy = im.copy()
im.close()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line isn't strictly necessary to test the change. Are you closing im here just to be thorough and ensure that it also doesn't log any errors?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah! 👍

copy.close()

assert len(self._caplog.records) == 0
assert im.fp is None
assert copy.fp is None


class MockEncoder:
pass
Expand Down
13 changes: 7 additions & 6 deletions src/PIL/Image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,12 +550,13 @@
more information.
"""
try:
if getattr(self, "_fp", False):
if self._fp != self.fp:
self._fp.close()
self._fp = DeferredError(ValueError("Operation on closed image"))
if self.fp:
self.fp.close()
if hasattr(self, "fp"):
if getattr(self, "_fp", False):
if self._fp != self.fp:
self._fp.close()
self._fp = DeferredError(ValueError("Operation on closed image"))
if self.fp:
self.fp.close()

Check warning on line 559 in src/PIL/Image.py

View check run for this annotation

Codecov / codecov/patch

src/PIL/Image.py#L553-L559 

Added lines #L553 - L559 were not covered by tests
self.fp = None
except Exception as msg:
logger.debug("Error closing: %s", msg)
Expand Down