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

Fixed UNDEFINED TIFF tag of length 0 being changed in roundtrip #5426

Merged
merged 1 commit into from
May 3, 2021

Conversation

radarhere
Copy link
Member

from PIL import Image
im = Image.open("Tests/images/hopper.tif")
im.tag_v2[45059] = b"test"
print(im.tag_v2[45059])  # (b'test',)

# You would think that self-assignment shouldn't change the value
im.tag_v2[45059] = im.tag_v2[45059]

# But it does
print(im.tag_v2[45059])  # ((b'test',),)

I would maintain that this should not be the case, so this PR fixes that.

What is special about this tag is that it is UNDEFINED with length 0.

45059: ("ImageUIDList", UNDEFINED, 0), # UNDONE, check

The value of an UNDEFINED tag is always turned into a list in _setitem - meaning that if it is already a sequence, it gets wrapped again.

if self.tagtype[tag] == TiffTags.UNDEFINED:
values = [
value.encode("ascii", "replace") if isinstance(value, str) else value
]

And because the length is 0, at the end of _setitem it isn't unwrapped.

if not is_ifd and (
(info.length == 1)
or self.tagtype[tag] == TiffTags.BYTE
or (info.length is None and len(values) == 1 and not legacy_api)
):
# Don't mess with the legacy api, since it's frozen.
if legacy_api and self.tagtype[tag] in [
TiffTags.RATIONAL,
TiffTags.SIGNED_RATIONAL,
]: # rationals
values = (values,)
try:
(dest[tag],) = values
except ValueError:
# We've got a builtin tag with 1 expected entry
warnings.warn(
f"Metadata Warning, tag {tag} had too many entries: "
f"{len(values)}, expected 1"
)
dest[tag] = values[0]
else:
# Spec'd length > 1 or undefined
# Unspec'd, and length > 1
dest[tag] = values

While I think this problem is worth fixing in it's own right, this does also resolve #2278. So if this is merged first, then #5425 becomes just about tidying up the tag value by turning it from a tuple into a bytestring.

@radarhere radarhere added the TIFF label Apr 21, 2021
@radarhere radarhere force-pushed the undefined_zero branch 2 times, most recently from 0dda822 to f70432b Compare April 25, 2021 11:37
@hugovk hugovk merged commit d2f2fba into python-pillow:master May 3, 2021
@radarhere radarhere deleted the undefined_zero branch May 3, 2021 06:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AttributeError: 'tuple' object has no attribute 'ljust' when trying to save image
2 participants