-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Added ImageSourceData to TAGS_V2 #7053
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
Conversation
src/encode.c
Outdated
| } else if (type == TIFF_LONG) { | ||
| status = ImagingLibTiffSetField( | ||
| &encoder->state, (ttag_t)key_int, (UINT32)PyLong_AsLong(value)); | ||
| &encoder->state, (ttag_t)key_int, (uint64_t)PyLong_AsLong(value)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't fully understand this change and can't look into it right now (I'm not sure why the TIFF_LONG8 error was triggered when this change is for TIFF_LONG), but at least on Windows, the PyLong_AsLong function returns the equivalent of int32_t. I think PyLong_AsLongLong should return int64_t on all platforms, although I'm not sure if it matters here.
To be clear, using PyLong_AsLong here instead of PyLong_AsLongLong is not a security issue, I'm just not sure whether it could save the wrong values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, that seems better than casting the value. I've updated the commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PyLong_AsLongLong returns int64_t, not uint64_t. It probably doesn't matter due to how signedness casting works, but it's not the same type. I was just pointing out that PyLong_AsLong has a different output range on Windows vs Linux.
|
This change broke Pillow on PowerPC: #8522 To be honest, I don't really understand what's being done here. Given that |
|
I've created #8529 to address the problem. |
In #7008, two images were reported to cause a RuntimeError. Testing on my machine, I found that this caused a segfault instead.
It turns out that this was due to an incorrect tag type being used for ImageSourceData.
Except then I found the RuntimeError, happening on 32-bit Windows.
When it states 'Bad LONG8 or IFD8 value 1000052142389717520 for "EXIFIFDOffset" tag 34665', that number wasn't the value being passed through for the EXIF IFD offset from Python. When we are about to pass the value through to libtiff in C, we cast it to
UINT32.Pillow/src/encode.c
Lines 906 to 907 in 3166901
When libtiff is throwing the error, the variable is
uint64_t. ChangingUINT32touint64_tfixed the RunTimeError.