-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
testing needed: only use image loaders which could potentially read file #17368
base: master
Are you sure you want to change the base?
Conversation
Fully agreed on this generally. |
I know this would render the code a little more complicated and is probably out of scope, but one could test if a jpeg file also has the 0xFFD9 end marker to determine a possible premature EOF. |
@HansBull I would expect that libJPEG reports that as corruption, but haven't actually checked into it. |
c86d76d
to
228477e
Compare
Rebased and merge conflicts fixed. |
If someone can tell my why CI keeps throwing an error (with empty log) on the "Check if it runs" step, I'll address that on the weekend. |
228477e
to
3fb947c
Compare
I was getting nowhere fast trying to trace the data flow to where temperature.c was spitting out its color matrix error message when an image file fails to load and/or aborting the pixelpipe before execution gets there, so I decided to simply suppress the message if the load failed.... |
@HansBull libJPEG doesn't report corruption if the codestream for the image data proper is truncated, it just fills the missing pixels with gray. For JPEGs, showing as much of the image as is available is probably better behavior than refusing to show it at all. I haven't played around with actual file corruption other than truncation. |
Rather than blindly applying image loaders until one succeeds, use magic numbers at the start of the image file to determine which loader(s) to use, and apply only those. Also detect a variety of magic numbers for common non-image formats so that we don't even attempt to load them and can give an appropriately specific error to the user.
This PR will need thorough testing because it changes such a core operation.
It will also have merge conflicts with #17367, which I will fix if both are accepted.