Skip to content

Commit

Permalink
PBM decoder robustness improvements and BufferedReadStream observability
Browse files Browse the repository at this point in the history 
Backport of #2551 & #2552
  • Loading branch information
@antonfirsov
antonfirsov committed Oct 15, 2023
1 parent 688e242 commit 1f3aa0c
Show file tree
Hide file tree
Showing 11 changed files with 258 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 @@ internal static class ImageDecoderUtilities
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 @@ internal static class ImageDecoderUtilities
{
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;
}
}
}
25 changes: 19 additions & 6 deletions src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0.

using System;
using System.Diagnostics.CodeAnalysis;
using System.Threading;
using SixLabors.ImageSharp.IO;
using SixLabors.ImageSharp.Memory;
Expand Down Expand Up @@ -90,6 +91,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 +141,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 +179,9 @@ private void ProcessHeader(BufferedReadStream stream)
meta.Encoding = this.Encoding;
meta.ColorType = this.ColorType;
meta.ComponentType = this.ComponentType;

[DoesNotReturn]

Check failure on line 183 in src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs

View workflow job for this annotation

GitHub Actions / Build (windows-latest, net472, -x64, false) 

'DoesNotReturnAttribute' is inaccessible due to its protection level

Check failure on line 183 in src/ImageSharp/Formats/Pbm/PbmDecoderCore.cs

View workflow job for this annotation

GitHub Actions / Build (windows-latest, net472, -x64, false) 

'DoesNotReturnAttribute' is inaccessible due to its protection level
static void ThrowPrematureEof() => throw new InvalidImageContentException("Reached EOF while reading the header.");
}

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

0 comments on commit 1f3aa0c

Please sign in to comment.