Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[release/2.1] PBM decoder robustness improvements and BufferedReadStream observability #2555

Merged
merged 2 commits into from
Oct 16, 2023
Merged

[release/2.1] PBM decoder robustness improvements and BufferedReadStream observability #2555

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f3aa0c
PBM decoder robustness improvements and BufferedReadStream observability
antonfirsov Oct 15, 2023
137f7c7
Remove DoesNotReturn attribute
JimBobSquarePants Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/ImageSharp/Formats/ImageDecoderUtilities.cs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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