Skip to content

Commit

Permalink
[release/2.1] PBM decoder robustness improvements and BufferedReadStr…
Browse files Browse the repository at this point in the history 
…eam observability (#2555)

* PBM decoder robustness improvements and BufferedReadStream observability

Backport of #2551 & #2552

* Remove DoesNotReturn attribute

---------

Co-authored-by: James Jackson-South <james_south@hotmail.com>
  • Loading branch information
@antonfirsov @JimBobSquarePants
antonfirsov and JimBobSquarePants authored Oct 16, 2023
1 parent 749b1c0 commit e74a55f
Show file tree
Hide file tree
Showing 11 changed files with 256 additions and 62 deletions.
10 changes: 9 additions & 1 deletion src/ImageSharp/Formats/ImageDecoderUtilities.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ public static Image<TPixel> Decode<TPixel>(
CancellationToken cancellationToken)
where TPixel : unmanaged, IPixel<TPixel>
{
using var bufferedReadStream = new BufferedReadStream(configuration, stream);
// Test may pass a BufferedReadStream in order to monitor EOF hits, if so, use the existing instance.
BufferedReadStream bufferedReadStream = stream as BufferedReadStream ?? new BufferedReadStream(configuration, stream);

try
{
Expand All @@ -56,6 +57,13 @@ public static Image<TPixel> Decode<TPixel>(
{
throw largeImageExceptionFactory(ex, decoder.Dimensions);
}
finally
{
if (bufferedReadStream != stream)
{
bufferedReadStream.Dispose();
}
}
}

private static InvalidImageContentException DefaultLargeImageExceptionFactory(
Expand Down
45 changes: 27 additions & 18 deletions src/ImageSharp/Formats/Pbm/BinaryDecoder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,11 @@ private static void ProcessGrayscale<TPixel>(Configuration configuration, Buffer

for (int y = 0; y < height; y++)
{
stream.Read(rowSpan);
if (stream.Read(rowSpan) < rowSpan.Length)
{
return;
}

Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromL8Bytes(
configuration,
Expand All @@ -94,7 +98,11 @@ private static void ProcessWideGrayscale<TPixel>(Configuration configuration, Bu

for (int y = 0; y < height; y++)
{
stream.Read(rowSpan);
if (stream.Read(rowSpan) < rowSpan.Length)
{
return;
}

Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromL16Bytes(
configuration,
Expand All @@ -116,7 +124,11 @@ private static void ProcessRgb<TPixel>(Configuration configuration, Buffer2D<TPi

for (int y = 0; y < height; y++)
{
stream.Read(rowSpan);
if (stream.Read(rowSpan) < rowSpan.Length)
{
return;
}

Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromRgb24Bytes(
configuration,
Expand All @@ -138,7 +150,11 @@ private static void ProcessWideRgb<TPixel>(Configuration configuration, Buffer2D

for (int y = 0; y < height; y++)
{
stream.Read(rowSpan);
if (stream.Read(rowSpan) < rowSpan.Length)
{
return;
}

Span<TPixel> pixelSpan = pixels.DangerousGetRowSpan(y);
PixelOperations<TPixel>.Instance.FromRgb48Bytes(
configuration,
Expand All @@ -153,7 +169,6 @@ private static void ProcessBlackAndWhite<TPixel>(Configuration configuration, Bu
{
int width = pixels.Width;
int height = pixels.Height;
int startBit = 0;
MemoryAllocator allocator = configuration.MemoryAllocator;
using IMemoryOwner<L8> row = allocator.Allocate<L8>(width);
Span<L8> rowSpan = row.GetSpan();
Expand All @@ -163,23 +178,17 @@ private static void ProcessBlackAndWhite<TPixel>(Configuration configuration, Bu
for (int x = 0; x < width;)
{
int raw = stream.ReadByte();
int bit = startBit;
startBit = 0;
for (; bit < 8; bit++)
if (raw < 0)
{
return;
}

int stopBit = Math.Min(8, width - x);
for (int bit = 0; bit < stopBit; bit++)
{
bool bitValue = (raw & (0x80 >> bit)) != 0;
rowSpan[x] = bitValue ? black : white;
x++;
if (x == width)
{
startBit = (bit + 1) & 7; // Round off to below 8.
if (startBit != 0)
{
stream.Seek(-1, System.IO.SeekOrigin.Current);
}

break;
}
}
}

Expand Down
41 changes: 32 additions & 9 deletions src/ImageSharp/Formats/Pbm/BufferedReadStreamExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,14 +12,20 @@ namespace SixLabors.ImageSharp.Formats.Pbm
internal static class BufferedReadStreamExtensions
{
/// <summary>
/// Skip over any whitespace or any comments.
/// Skip over any whitespace or any comments and signal if EOF has been reached.
/// </summary>
public static void SkipWhitespaceAndComments(this BufferedReadStream stream)
/// <param name="stream">The buffered read stream.</param>
/// <returns><see langword="false"/> if EOF has been reached while reading the stream; see langword="true"/> otherwise.</returns>
public static bool SkipWhitespaceAndComments(this BufferedReadStream stream)
{
bool isWhitespace;
do
{
int val = stream.ReadByte();
if (val < 0)
{
return false;
}

// Comments start with '#' and end at the next new-line.
if (val == 0x23)
Expand All @@ -28,8 +34,12 @@ public static void SkipWhitespaceAndComments(this BufferedReadStream stream)
do
{
innerValue = stream.ReadByte();
if (innerValue < 0)
{
return false;
}
}
while (innerValue is not 0x0a and not -0x1);
while (innerValue is not 0x0a);

// Continue searching for whitespace.
val = innerValue;
Expand All @@ -39,18 +49,31 @@ public static void SkipWhitespaceAndComments(this BufferedReadStream stream)
}
while (isWhitespace);
stream.Seek(-1, SeekOrigin.Current);
return true;
}

/// <summary>
/// Read a decimal text value.
/// Read a decimal text value and signal if EOF has been reached.
/// </summary>
/// <returns>The integer value of the decimal.</returns>
public static int ReadDecimal(this BufferedReadStream stream)
/// <param name="stream">The buffered read stream.</param>
/// <param name="value">The read value.</param>
/// <returns><see langword="false"/> if EOF has been reached while reading the stream; <see langword="true"/> otherwise.</returns>
/// <remarks>
/// A 'false' return value doesn't mean that the parsing has been failed, since it's possible to reach EOF while reading the last decimal in the file.
/// It's up to the call site to handle such a situation.
/// </remarks>
public static bool ReadDecimal(this BufferedReadStream stream, out int value)
{
int value = 0;
value = 0;
while (true)
{
int current = stream.ReadByte() - 0x30;
int current = stream.ReadByte();
if (current < 0)
{
return false;
}

current -= 0x30;
if ((uint)current > 9)
{
break;
Expand All @@ -59,7 +82,7 @@ public static int ReadDecimal(this BufferedReadStream stream)
value = (value * 10) + current;
}

return value;
return true;
}
}
}
23 changes: 17 additions & 6 deletions src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public IImageInfo Identify(BufferedReadStream stream, CancellationToken cancella
/// Processes the ppm header.
/// </summary>
/// <param name="stream">The input stream.</param>
/// <exception cref="InvalidImageContentException">An EOF marker has been read before the image has been decoded.</exception>
private void ProcessHeader(BufferedReadStream stream)
{
Span<byte> buffer = stackalloc byte[2];
Expand Down Expand Up @@ -139,14 +140,22 @@ private void ProcessHeader(BufferedReadStream stream)
throw new InvalidImageContentException("Unknown of not implemented image type encountered.");
}

stream.SkipWhitespaceAndComments();
int width = stream.ReadDecimal();
stream.SkipWhitespaceAndComments();
int height = stream.ReadDecimal();
stream.SkipWhitespaceAndComments();
if (!stream.SkipWhitespaceAndComments() ||
!stream.ReadDecimal(out int width) ||
!stream.SkipWhitespaceAndComments() ||
!stream.ReadDecimal(out int height) ||
!stream.SkipWhitespaceAndComments())
{
ThrowPrematureEof();
}

if (this.ColorType != PbmColorType.BlackAndWhite)
{
this.maxPixelValue = stream.ReadDecimal();
if (!stream.ReadDecimal(out this.maxPixelValue))
{
ThrowPrematureEof();
}

if (this.maxPixelValue > 255)
{
this.ComponentType = PbmComponentType.Short;
Expand All @@ -169,6 +178,8 @@ private void ProcessHeader(BufferedReadStream stream)
meta.Encoding = this.Encoding;
meta.ColorType = this.ColorType;
meta.ComponentType = this.ComponentType;

static void ThrowPrematureEof() => throw new InvalidImageContentException("Reached EOF while reading the header.");
}

private void ProcessPixels<TPixel>(BufferedReadStream stream, Buffer2D<TPixel> pixels)
Expand Down
Loading

0 comments on commit e74a55f

Please sign in to comment.