Skip to content

Fix: Set tag type accordingly if IFDRational with 0 denominator #8966

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

epou
Copy link

@epou epou commented May 21, 2025

Fixes #8965

Changes proposed in this pull request:

  • override __float__ method in IFDRational
  • Setting tagtype to TiffTags.SIGNED_RATIONAL if IFDRational is nan but the numerator is negative.

After investigating the issue, I found that the EXIF tag contains the value (-1, 0), which results in float('nan') via the IFDRational class. This was previously handled more gracefully, but starting from commit 5ff2027 (included in Pillow 11.1.0), all NaN IFDRational values are considered as TiffTags.RATIONAL.

This causes a problem when the writer attempts to serialize these values using write_rational, which fails because struct.pack("2L", ...) cannot handle negative values. In such cases, write_signed_rational should be used instead.

@epou epou force-pushed the fix_ifdrational_tagtype_negative_numerator branch from ecfae11 to 3ac0420 Compare May 21, 2025 13:23
@epou
Copy link
Author

epou commented May 22, 2025

Clarification regarding:

This was previously handled more gracefully, but starting from commit 5ff2027 (included in Pillow 11.1.0), all NaN IFDRational values are considered as TiffTags.RATIONAL.

This change in behavior is due to how float('nan') operates in Python:

>>> float('nan') == float('nan')
False
>>> float('nan') > 0
False
>>> float('nan') < 0
False
>>> float('nan') == 0
False

Before commit 5ff2027, the logic used all(v >= 0 for v in values) to determine the tag type. Since any comparison involving float('nan') returns False, the presence of a NaN caused the expression to evaluate to False, resulting in the tag being set to TiffTags.SIGNED_RATIONAL:

                    self.tagtype[tag] = (
                        TiffTags.RATIONAL
                        if all(v >= 0 for v in values)
                        else TiffTags.SIGNED_RATIONAL
                    )

After the commit, the logic was changed to inspect values individually:

                    for v in values:
                        assert isinstance(v, IFDRational)
                        if v < 0:
                            self.tagtype[tag] = TiffTags.SIGNED_RATIONAL
                            break
                        else:
                            self.tagtype[tag] = TiffTags.RATIONAL

This change means that when a value is float('nan'), it does not satisfy the v < 0 condition (nor would it satisfy any other typical comparison), so the tag is now set to TiffTags.RATIONAL by default

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 this pull request may close these issues.

ImageOps.exif_transpose() raises struct.error on invalid EXIF rational
1 participant