Browse Source

Improve robustness of huffman decoder.

pull/924/head
James Jackson-South 7 years ago
parent
commit
84256b6180
  1. 5
      Directory.Build.props
  2. 77
      src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanBuffer.cs
  3. 24
      src/ImageSharp/Formats/Jpeg/Components/Decoder/HuffmanScanDecoder.cs
  4. 4
      tests/ImageSharp.Tests/Formats/Jpg/JpegDecoderTests.Images.cs

5
Directory.Build.props

@ -27,9 +27,12 @@
<PackageRequireLicenseAcceptance>true</PackageRequireLicenseAcceptance>
<SignAssembly>false</SignAssembly>
<SuppressNETCoreSdkPreviewMessage>true</SuppressNETCoreSdkPreviewMessage>
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)' == 'Release'">
<TreatWarningsAsErrors>true</TreatWarningsAsErrors>
</PropertyGroup>
<!--TODO: Check what this is testing for and why does it fail?-->
<PropertyGroup Condition="'$(Configuration)' == 'Debug'">
<CheckForOverflowUnderflow>false</CheckForOverflowUnderflow>

77
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;
}
/// <summary>
/// Gets or sets the current, if any, marker in the input stream.
/// Gets the current, if any, marker in the input stream.
/// </summary>
public byte Marker { get; set; }
public byte Marker { get; private set; }
/// <summary>
/// Gets or sets the opening position of an identified marker.
/// Gets the opening position of an identified marker.
/// </summary>
public long MarkerPosition { get; set; }
public long MarkerPosition { get; private set; }
/// <summary>
/// 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
/// </summary>
public bool BadMarker { get; set; }
public bool BadMarker { get; private set; }
/// <summary>
/// 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.
/// </summary>
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;
}
}
}
}

24
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;
}
}
}
}

4
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,

Loading…
Cancel
Save