[fix]: Fixed Unknown App0 Marker add profileResolver missing #2566
Conversation
|
|
| @@ -73,7 +73,8 @@ public static bool TryParse(ReadOnlySpan<byte> bytes, out JFifMarker marker) | |||
| { | |||
| // Some images incorrectly use JFXX as the App0 marker (Issue 2478) | |||
| if (ProfileResolver.IsProfile(bytes, ProfileResolver.JFifMarker) | |||
| || ProfileResolver.IsProfile(bytes, ProfileResolver.JFxxMarker)) | |||
| || ProfileResolver.IsProfile(bytes, ProfileResolver.JFxxMarker) | |||
| || ProfileResolver.IsProfile(bytes, ProfileResolver.OSQidMaker)) | |||
There was a problem hiding this comment.
While this works, I'm not sure it's the correct fix.
I think we might be able to simply remove the throw when the marker doesn't match a JFIF one and skip the remaining bytes.
What is OSQidMaker? Can you please supply more information?
There was a problem hiding this comment.
In fact, in my research I didn't find markers that specified such a conversion starting with these bytes, but I have images of a camera that always has these bytes at the beginning. I also had success in all tests when ignoring this if but I don't know what more problems this could cause. I'm running a fork of the lib in my application and I don't get any more errors
There was a problem hiding this comment.
The JFIF JFXX headers contain metadata and we can actually operate without them. Your images actually contain a JFIF header in an APP0 block just after the camera junk.
I can鈥檛 accept the fix as-is because the values are non-standard so we should try a more robust solution as suggested
| public static ReadOnlySpan<byte> OSQidMaker => new[] | ||
| { | ||
| (byte)'\n', (byte)'[', (byte)'I', (byte)'D', (byte)' ' | ||
| }; |
There was a problem hiding this comment.
| public static ReadOnlySpan<byte> OSQidMaker => new[] | |
| { | |
| (byte)'\n', (byte)'[', (byte)'I', (byte)'D', (byte)' ' | |
| }; | |
| public static ReadOnlySpan<byte> OSQidMaker => new[] { (byte)'\n', (byte)'[', (byte)'I', (byte)'D', (byte)' ' }; |
I'd like to have it written as one-liner, because when we are able to use ut8-literals it's easier to find & replace these.
There was a problem hiding this comment.
It's a good suggestion, but the other properties of the class already follow this same pattern, so I chose to keep it.
|
Hi @JoseEliasSantos I can see you updated your PR from main. As commented, we won't be able to proceed #2566 (comment) with the fix you have applied. If you are not able to make the suggested changes, that's OK. I can close this and create a new PR. |
Prerequisites
Description
Fixed discussion and issue bug
https://github.com/SixLabors/ImageSharp/discussions/2564https://github.com/SixLabors/ImageSharp/issues/2565