diff --git a/Directory.Build.props b/Directory.Build.props index 6e3cc9b3bf..bf004921ea 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -27,9 +27,12 @@ true false true - true + + true + + false diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs index 72bfa38646..dd5160414e 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System.Runtime.CompilerServices; @@ -19,8 +19,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder // The number of valid bits left to read in the buffer. private int remain; - // Whether there is more data to pull from the stream for the current mcu. - private bool noMore; + // Whether there is no more good data to pull from the stream for the current mcu. + private bool badData; public HuffmanScanBuffer(DoubleBufferedStreamReader stream) { @@ -30,29 +30,29 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.Marker = JpegConstants.Markers.XFF; this.MarkerPosition = 0; this.BadMarker = false; - this.noMore = false; - this.Eof = false; + this.badData = false; + this.NoData = false; } /// - /// Gets or sets the current, if any, marker in the input stream. + /// Gets the current, if any, marker in the input stream. /// - public byte Marker { get; set; } + public byte Marker { get; private set; } /// - /// Gets or sets the opening position of an identified marker. + /// Gets the opening position of an identified marker. /// - public long MarkerPosition { get; set; } + public long MarkerPosition { get; private set; } /// - /// Gets or sets a value indicating whether we have a bad marker, I.E. One that is not between RST0 and RST7 + /// Gets a value indicating whether a bad marker has been detected, I.E. One that is not between RST0 and RST7 /// - public bool BadMarker { get; set; } + public bool BadMarker { get; private set; } /// - /// Gets or sets a value indicating whether we have prematurely reached the end of the file. + /// Gets a value indicating whether to continue reading the input stream. /// - public bool Eof { get; set; } + public bool NoData { get; private set; } [MethodImpl(InliningOptions.ShortMethod)] public void CheckBits() @@ -71,8 +71,8 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder this.Marker = JpegConstants.Markers.XFF; this.MarkerPosition = 0; this.BadMarker = false; - this.noMore = false; - this.Eof = false; + this.badData = false; + this.NoData = false; } [MethodImpl(InliningOptions.ShortMethod)] @@ -141,39 +141,28 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder ulong temp = 0; for (int i = 0; i < 6; i++) { - int b = this.noMore ? 0 : this.stream.ReadByte(); - - if (b == -1) - { - // We've encountered the end of the file stream which means there's no EOI marker in the image - // or the SOS marker has the wrong dimensions set. - this.Eof = true; - b = 0; - } + int b = this.ReadStream(); // Found a marker. if (b == JpegConstants.Markers.XFF) { - this.MarkerPosition = this.stream.Position - 1; - int c = this.stream.ReadByte(); + int c = this.ReadStream(); while (c == JpegConstants.Markers.XFF) { - c = this.stream.ReadByte(); - - if (c == -1) - { - this.Eof = true; - c = 0; - break; - } + // Loop here to discard any padding FF's on terminating marker, + // so that we can save a valid marker value. + c = this.ReadStream(); } + // We accept multiple FF's followed by a 0 as meaning a single FF data byte. + // This data pattern is not valid according to the standard. if (c != 0) { this.Marker = (byte)c; - this.noMore = true; + this.badData = true; if (!this.HasRestart()) { + this.MarkerPosition = this.stream.Position - 2; this.BadMarker = true; } } @@ -184,5 +173,21 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder return temp; } + + [MethodImpl(InliningOptions.ShortMethod)] + private int ReadStream() + { + int value = this.badData ? 0 : this.stream.ReadByte(); + if (value == -1) + { + // We've encountered the end of the file stream which means there's no EOI marker + // in the image or the SOS marker has the wrong dimensions set. + this.badData = true; + this.NoData = true; + value = 0; + } + + return value; + } } -} \ No newline at end of file +} diff --git a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs index 76fea92976..c28733250a 100644 --- a/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs +++ b/src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs @@ -1,4 +1,4 @@ -// Copyright (c) Six Labors and contributors. +// Copyright (c) Six Labors and contributors. // Licensed under the Apache License, Version 2.0. using System; @@ -130,6 +130,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int mcu = 0; int mcusPerColumn = this.frame.McusPerColumn; int mcusPerLine = this.frame.McusPerLine; + ref HuffmanScanBuffer buffer = ref this.scanBuffer; // Pre-derive the huffman table to avoid in-loop checks. for (int i = 0; i < this.componentsLength; i++) @@ -171,6 +172,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder for (int x = 0; x < h; x++) { + if (buffer.NoData) + { + return; + } + int blockCol = (mcuCol * h) + x; this.DecodeBlockBaseline( @@ -193,6 +199,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder private unsafe void ParseBaselineDataNonInterleaved() { JpegComponent component = this.components[this.frame.ComponentOrder[0]]; + ref HuffmanScanBuffer buffer = ref this.scanBuffer; int w = component.WidthInBlocks; int h = component.HeightInBlocks; @@ -210,6 +217,11 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder for (int i = 0; i < w; i++) { + if (buffer.NoData) + { + return; + } + this.DecodeBlockBaseline( component, ref Unsafe.Add(ref blockRef, i), @@ -294,6 +306,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int mcu = 0; int mcusPerColumn = this.frame.McusPerColumn; int mcusPerLine = this.frame.McusPerLine; + ref HuffmanScanBuffer buffer = ref this.scanBuffer; // Pre-derive the huffman table to avoid in-loop checks. for (int k = 0; k < this.componentsLength; k++) @@ -316,7 +329,6 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder int order = this.frame.ComponentOrder[k]; JpegComponent component = this.components[order]; ref HuffmanTable dcHuffmanTable = ref this.dcHuffmanTables[component.DCHuffmanTableId]; - ref HuffmanScanBuffer buffer = ref this.scanBuffer; int h = component.HorizontalSamplingFactor; int v = component.VerticalSamplingFactor; @@ -331,7 +343,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder for (int x = 0; x < h; x++) { - if (buffer.Eof) + if (buffer.NoData) { return; } @@ -375,7 +387,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder for (int i = 0; i < w; i++) { - if (buffer.Eof) + if (buffer.NoData) { return; } @@ -404,7 +416,7 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder for (int i = 0; i < w; i++) { - if (buffer.Eof) + if (buffer.NoData) { return; } @@ -691,4 +703,4 @@ namespace SixLabors.ImageSharp.Formats.Jpeg.Components.Decoder return false; } } -} \ No newline at end of file +} diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs index d428000efe..56496fad1b 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs +++ b/tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs @@ -17,6 +17,10 @@ namespace SixLabors.ImageSharp.Tests.Formats.Jpg // BUG: The following image has a high difference compared to the expected output: // TestImages.Jpeg.Baseline.Jpeg420Small, + + // BUG: While we can decode this image we do not return the same output as libjpeg + // based decoders and are inserting a number of lines equal to the corrupted lines + // below said lines. // TestImages.Jpeg.Issues.Fuzz.AccessViolationException922, TestImages.Jpeg.Baseline.Jpeg444,