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

Add more data to error message #8316

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Yay295
Copy link
Contributor

@Yay295 Yay295 commented Aug 19, 2024

This changes the error message from
offset is neither short nor long
to
offset is neither short nor long: Tag ID 273 Field Type 16 Field Size 8 Count 1

@radarhere
Copy link
Member

Could you give an example of how to trigger this error?

@radarhere radarhere added the TIFF label Aug 19, 2024
@Yay295
Copy link
Contributor Author

Yay295 commented Aug 19, 2024

With #8317, test_bigtiff() in test_file_tiff.py triggers this error if del im.tag_v2[273] is removed from the test.

@radarhere
Copy link
Member

radarhere commented Aug 19, 2024

Would it not be more consistent with the rest of TiffImagePlugin to log the extra information instead?

if samples_per_pixel > MAX_SAMPLESPERPIXEL:
# DOS check, samples_per_pixel can be a Long, and we extend the tuple below
logger.error(
"More samples per pixel than can be decoded: %s", samples_per_pixel
)
msg = "Invalid value for samples per pixel"
raise SyntaxError(msg)

@Yay295
Copy link
Contributor Author

Yay295 commented Aug 19, 2024

It might. Logging is only used in five files in Pillow though (Image, PcxImagePlugin, PngImagePlugin, TiffImagePlugin, helper).

@radarhere
Copy link
Member

radarhere commented Nov 25, 2024

With #8317, test_bigtiff() in test_file_tiff.py triggers this error if del im.tag_v2[273] is removed from the test.

Just a note: this specific example stopped triggering the error after #8417 added support for LONG8.

@radarhere
Copy link
Member

I've created #8568 as an alternative to this, using my suggestion of logging.

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.

2 participants