From 3a8e5a216fdf1f698e568cb159d1b74f36d04e73 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 10 Jan 2022 11:12:48 +1100 Subject: [PATCH] Only decode App0 1x. Fix #1932 --- .../Formats/Jpeg/Components/Decoder/JFifMarker.cs | 10 +++------- src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs | 4 +++- .../Formats/Jpg/JpegDecoderTests.Metadata.cs | 9 +++++---- tests/ImageSharp.Tests/TestImages.cs | 1 + .../Input/Jpg/issues/issue-1932-app0-resolution.jpg | 3 +++ 5 files changed, 15 insertions(+), 12 deletions(-) create mode 100644 tests/Images/Input/Jpg/issues/issue-1932-app0-resolution.jpg diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs index c7b71f75a8..a95e6c16c2 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/JFifMarker.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors. +// Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. using System; @@ -103,26 +103,22 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder /// public bool Equals(JFifMarker other) - { - return this.MajorVersion == other.MajorVersion + => this.MajorVersion == other.MajorVersion && this.MinorVersion == other.MinorVersion && this.DensityUnits == other.DensityUnits && this.XDensity == other.XDensity && this.YDensity == other.YDensity; - } /// public override bool Equals(object obj) => obj is JFifMarker other && this.Equals(other); /// public override int GetHashCode() - { - return HashCode.Combine( + => HashCode.Combine( this.MajorVersion, this.MinorVersion, this.DensityUnits, this.XDensity, this.YDensity); - } } } diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index a21f1a71d7..5f01c04688 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -625,7 +625,9 @@ namespace SixLabors.ImageSharp.Formats.Jpeg private void ProcessApplicationHeaderMarker(BufferedReadStream stream, int remaining) { // We can only decode JFif identifiers. - if (remaining < JFifMarker.Length) + // Some bad images contain multiple App0 markers (Issue 1932) so we check to see + // if it's already been read. + if (remaining < JFifMarker.Length || (!this.jFif.Equals(default))) { // Skip the application header length stream.Skip(remaining); diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs index 7b3e20aa2a..0ef5090cc8 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Metadata.cs @@ -22,7 +22,7 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg // TODO: A JPEGsnoop & metadata expert should review if the Exif/Icc expectations are correct. // I'm seeing several entries with Exif-related names in images where we do not decode an exif profile. (- Anton) public static readonly TheoryData MetadataTestData = - new TheoryData + new() { { false, TestImages.Jpeg.Progressive.Progress, 24, false, false }, { false, TestImages.Jpeg.Progressive.Fb, 24, false, true }, @@ -42,15 +42,16 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg }; public static readonly TheoryData RatioFiles = - new TheoryData + new() { { TestImages.Jpeg.Baseline.Ratio1x1, 1, 1, PixelResolutionUnit.AspectRatio }, { TestImages.Jpeg.Baseline.Snake, 300, 300, PixelResolutionUnit.PixelsPerInch }, - { TestImages.Jpeg.Baseline.GammaDalaiLamaGray, 72, 72, PixelResolutionUnit.PixelsPerInch } + { TestImages.Jpeg.Baseline.GammaDalaiLamaGray, 72, 72, PixelResolutionUnit.PixelsPerInch }, + { TestImages.Jpeg.Issues.MultipleApp01932, 400, 400, PixelResolutionUnit.PixelsPerInch } }; public static readonly TheoryData QualityFiles = - new TheoryData + new() { { TestImages.Jpeg.Baseline.Calliphora, 80 }, { TestImages.Jpeg.Progressive.Fb, 75 }, diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index deed0b2404..3c3c12c91d 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -257,6 +257,7 @@ namespace SixLabors.ImageSharp.Tests public const string IdentifyMultiFrame1211 = "Jpg/issues/issue-1221-identify-multi-frame.jpg"; public const string WrongColorSpace = "Jpg/issues/Issue1732-WrongColorSpace.jpg"; public const string MalformedUnsupportedComponentCount = "Jpg/issues/issue-1900-malformed-unsupported-255-components.jpg"; + public const string MultipleApp01932 = "Jpg/issues/issue-1932-app0-resolution.jpg"; public static class Fuzz { diff --git a/tests/Images/Input/Jpg/issues/issue-1932-app0-resolution.jpg b/tests/Images/Input/Jpg/issues/issue-1932-app0-resolution.jpg new file mode 100644 index 0000000000..42a90c68c2 --- /dev/null +++ b/tests/Images/Input/Jpg/issues/issue-1932-app0-resolution.jpg @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:b9d1a583e5c8c0a1a7362d06befd82883e1dc4231129ddb11d11d8c428691ff5 +size 1126332