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 Name field length when saving IM images #4424

Merged
merged 8 commits into from Feb 27, 2020

Conversation

hugovk
Copy link
Member

@hugovk hugovk commented Feb 17, 2020

For #4193.

Converting Tests/test_file_im.py to use pytest surfaced a bug in saving IM format files.

When roundtripping the file, it failed opening the saved file, at this check:

if len(s) > 100:
raise SyntaxError("not an IM file")

It's falling over the Name: field, which is saved like this:

if filename:
fp.write(("Name: %s\r\n" % filename).encode("ascii"))

The difference is the temp path+filename under the limit with PillowTestCase.tempfile:

Name: /var/folders/kt/j77sf4_n6fnbx6pg199rbx700000gn/T/tempx72ebkj5.im

And over the limit with pytest's tmp_path:

Name: /private/var/folders/kt/j77sf4_n6fnbx6pg199rbx700000gn/T/pytest-of-hugo/pytest-126/test_roundtrip0/temp.im

This PR trims the path+filename to fit, and I suggest to keep the last part which includes the filename and extension, rather than cropping it off.

Name: olders/kt/j77sf4_n6fnbx6pg199rbx700000gn/T/pytest-of-hugo/pytest-126/test_roundtrip0/temp.im

@hugovk hugovk added Bug Any unexpected behavior, until confirmed feature. Testing labels Feb 17, 2020
@hugovk

This comment has been minimized.

@radarhere
Copy link
Member

My personal opinion, in the case of /private/var/folders/kt/j77sf4_n6fnbx6pg199rbx700000gn/T/pytest-of-hugo/pytest-126/test_roundtrip0/temp.im, I find

Name: olders/kt/j77sf4_n6fnbx6pg199rbx700000gn/T/pytest-of-hugo/pytest-126/test_roundtrip0/temp.im

to be slightly odd. I would feel better about just trimming it down to the base name in this situation, because I don't think I need to know the absolute path of an image's original location.

Name: temp.im

@hugovk
Copy link
Member Author

hugovk commented Feb 22, 2020

Sounds sensible, updated.

Of course, it's technically possible for a long basename to exceed the limit. This is less likely, do we fix it, or let it be until someone stumbles over it? There's an easy workaround.

I doubt the IM format gets much use, pretty much the only references I could find online for the format were in PIL/Pillow.

@python-pillow python-pillow deleted a comment from codecov bot Feb 26, 2020
@radarhere radarhere merged commit 1c1ad65 into python-pillow:master Feb 27, 2020
@hugovk hugovk deleted the fix-im-long-name branch February 27, 2020 10:01
@radarhere radarhere changed the title Fix saving IM images in dir with long path Fix Name field length when saving IM images Mar 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Any unexpected behavior, until confirmed feature. Testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants