From 2c12c78e83bff820cb974302030ad5eff712fd29 Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Sun, 5 Dec 2021 15:20:03 -0800 Subject: [PATCH 01/17] Added support for loading exif data from pre-2017 pngs from the "raw profile type exif" text chunk. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 104 +++++++++++++++++- .../Formats/Png/PngDecoderTests.cs | 19 ++++ tests/ImageSharp.Tests/TestImages.cs | 3 + .../Input/Png/raw-profile-type-exif.png | 3 + 4 files changed, 124 insertions(+), 5 deletions(-) create mode 100644 tests/Images/Input/Png/raw-profile-type-exif.png diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index cf3cd7eb14..82fc5e8159 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -112,6 +112,11 @@ internal sealed class PngDecoderCore : IImageDecoderInternals /// private PngChunk? nextChunk; + /// + /// "Exif" and two zero bytes. Used for the legacy exif parsing. + /// + private static readonly byte[] ExifHeader = new byte[] { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00 }; + /// /// Initializes a new instance of the class. /// @@ -182,7 +187,7 @@ public Image Decode(BufferedReadStream stream, CancellationToken this.ReadTextChunk(pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.CompressedText: - this.ReadCompressedTextChunk(pngMetadata, chunk.Data.GetSpan()); + this.ReadCompressedTextChunk(metadata, pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.InternationalText: this.ReadInternationalTextChunk(pngMetadata, chunk.Data.GetSpan()); @@ -192,7 +197,7 @@ public Image Decode(BufferedReadStream stream, CancellationToken { var exifData = new byte[chunk.Length]; chunk.Data.GetSpan().CopyTo(exifData); - metadata.ExifProfile = new ExifProfile(exifData); + this.MergeOrSetExifProfile(metadata, new ExifProfile(exifData), replaceExistingKeys: true); } break; @@ -255,7 +260,7 @@ public IImageInfo Identify(BufferedReadStream stream, CancellationToken cancella this.ReadTextChunk(pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.CompressedText: - this.ReadCompressedTextChunk(pngMetadata, chunk.Data.GetSpan()); + this.ReadCompressedTextChunk(metadata, pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.InternationalText: this.ReadInternationalTextChunk(pngMetadata, chunk.Data.GetSpan()); @@ -265,7 +270,7 @@ public IImageInfo Identify(BufferedReadStream stream, CancellationToken cancella { var exifData = new byte[chunk.Length]; chunk.Data.GetSpan().CopyTo(exifData); - metadata.ExifProfile = new ExifProfile(exifData); + this.MergeOrSetExifProfile(metadata, new ExifProfile(exifData), replaceExistingKeys: true); } break; @@ -937,9 +942,10 @@ private void ReadTextChunk(PngMetadata metadata, ReadOnlySpan data) /// /// Reads the compressed text chunk. Contains a uncompressed keyword and a compressed text string. /// + /// The object. /// The metadata to decode to. /// The containing the data. - private void ReadCompressedTextChunk(PngMetadata metadata, ReadOnlySpan data) + private void ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata metadata, ReadOnlySpan data) { if (this.ignoreMetadata) { @@ -971,6 +977,94 @@ private void ReadCompressedTextChunk(PngMetadata metadata, ReadOnlySpan da { metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty)); } + + if (name.Equals("Raw profile type exif", StringComparison.OrdinalIgnoreCase)) + { + this.ReadLegacyExifTextChunk(baseMetadata, uncompressed); + } + } + + /// + /// Reads exif data encoded into a text chunk with the name "raw profile type exif". + /// This method was used by ImageMagick, exiftool, exiv2, digiKam, etc, before the + /// 2017 update to png that allowed a true exif chunk. We load + /// + /// The to store the decoded exif tags into. + /// The contents of the "raw profile type exif" text chunk. + private void ReadLegacyExifTextChunk(ImageMetadata metadata, string data) + { + ReadOnlySpan dataSpan = data.AsSpan(); + dataSpan = dataSpan.TrimStart(); + + if (!dataSpan.Slice(0, 4).ToString().Equals("exif", StringComparison.OrdinalIgnoreCase)) + { + // "exif" identifier is missing from the beginning of the text chunk + return; + } + + // Skip to the data length + dataSpan = dataSpan.Slice(4).TrimStart(); + int dataLengthEnd = dataSpan.IndexOf('\n'); + int dataLength = int.Parse(dataSpan.Slice(0, dataSpan.IndexOf('\n')).ToString()); + + // Skip to the hex-encoded data + dataSpan = dataSpan.Slice(dataLengthEnd).Trim(); + string dataSpanString = dataSpan.ToString().Replace("\n", string.Empty); + if (dataSpanString.Length != (dataLength * 2)) + { + // Invalid length + return; + } + + // Parse the hex-encoded data into the byte array we are going to hand off to ExifProfile + byte[] dataBlob = new byte[dataLength - ExifHeader.Length]; + for (int i = 0; i < dataLength; i++) + { + byte parsed = Convert.ToByte(dataSpanString.Substring(i * 2, 2), 16); + if (i < ExifHeader.Length) + { + if (parsed != ExifHeader[i]) + { + // Invalid exif header in the actual data blob + return; + } + } + else + { + dataBlob[i - ExifHeader.Length] = parsed; + } + } + + this.MergeOrSetExifProfile(metadata, new ExifProfile(dataBlob), replaceExistingKeys: false); + } + + /// + /// Sets the in to , + /// or copies exif tags if already contains an . + /// + /// The to store the exif data in. + /// The to copy exif tags from. + /// If already contains an , + /// controls whether existing exif tags in will be overwritten with any conflicting + /// tags from . + private void MergeOrSetExifProfile(ImageMetadata metadata, ExifProfile newProfile, bool replaceExistingKeys) + { + if (metadata.ExifProfile is null) + { + // No exif metadata was loaded yet, so just assign it + metadata.ExifProfile = newProfile; + } + else + { + // Try to merge existing keys with the ones from the new profile + foreach (IExifValue newKey in newProfile.Values) + { + if (replaceExistingKeys || metadata.ExifProfile.GetValueInternal(newKey.Tag) is null) + { + metadata.ExifProfile.SetValueInternal(newKey.Tag, newKey.GetValue()); + } + } + } } /// diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 9fc4d03dda..215cd14a1a 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -444,5 +444,24 @@ static void RunTest(string providerDump, string nonContiguousBuffersStr) "Disco") .Dispose(); } + + [Theory] + [WithFile(TestImages.Png.Issue1875, PixelTypes.Rgba32)] + public void PngDecoder_CanDecode_LegacyTextExifChunk(TestImageProvider provider) + { + using Image image = provider.GetImage(PngDecoder); + + Assert.Equal(0, image.Metadata.ExifProfile.InvalidTags.Count); + Assert.Equal(3, image.Metadata.ExifProfile.Values.Count); + + Assert.Equal( + "A colorful tiling of blue, red, yellow, and green 4x4 pixel blocks.", + image.Metadata.ExifProfile.GetValue(ImageSharp.Metadata.Profiles.Exif.ExifTag.ImageDescription).Value); + Assert.Equal( + "Duplicated from basn3p02.png, then image metadata modified with exiv2", + image.Metadata.ExifProfile.GetValue(ImageSharp.Metadata.Profiles.Exif.ExifTag.ImageHistory).Value); + + Assert.Equal(42, (int)image.Metadata.ExifProfile.GetValue(ImageSharp.Metadata.Profiles.Exif.ExifTag.ImageNumber).Value); + } } } diff --git a/tests/ImageSharp.Tests/TestImages.cs b/tests/ImageSharp.Tests/TestImages.cs index e003649135..41e5e15d3d 100644 --- a/tests/ImageSharp.Tests/TestImages.cs +++ b/tests/ImageSharp.Tests/TestImages.cs @@ -117,6 +117,9 @@ public static class Png // Issue 1765: https://github.com/SixLabors/ImageSharp/issues/1765 public const string Issue1765_Net6DeflateStreamRead = "Png/issues/Issue_1765_Net6DeflateStreamRead.png"; + // Discussion 1875: https://github.com/SixLabors/ImageSharp/discussions/1875 + public const string Issue1875 = "Png/raw-profile-type-exif.png"; + public static class Bad { public const string MissingDataChunk = "Png/xdtn0g01.png"; diff --git a/tests/Images/Input/Png/raw-profile-type-exif.png b/tests/Images/Input/Png/raw-profile-type-exif.png new file mode 100644 index 0000000000..efd9b35aaa --- /dev/null +++ b/tests/Images/Input/Png/raw-profile-type-exif.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:2259b08fd0c4681ecd068244df358b486f5eca1fcd18edbc7d9207eeef3ca5ed +size 392 From d5ddc4696e851d93569e7b9c356691b386993ca2 Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Sun, 5 Dec 2021 15:57:05 -0800 Subject: [PATCH 02/17] Fixed an incomplete comment --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 82fc5e8159..042bcb92b6 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -987,7 +987,7 @@ private void ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata met /// /// Reads exif data encoded into a text chunk with the name "raw profile type exif". /// This method was used by ImageMagick, exiftool, exiv2, digiKam, etc, before the - /// 2017 update to png that allowed a true exif chunk. We load + /// 2017 update to png that allowed a true exif chunk. /// /// The to store the decoded exif tags into. /// The contents of the "raw profile type exif" text chunk. From c8a191d2968fe2ed61302e35675f449317435150 Mon Sep 17 00:00:00 2001 From: jubilant-enigma <54286000+jubilant-enigma@users.noreply.github.com> Date: Mon, 6 Dec 2021 10:18:14 -0800 Subject: [PATCH 03/17] Update src/ImageSharp/Formats/Png/PngDecoderCore.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 042bcb92b6..6db2328f58 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -115,7 +115,8 @@ internal sealed class PngDecoderCore : IImageDecoderInternals /// /// "Exif" and two zero bytes. Used for the legacy exif parsing. /// - private static readonly byte[] ExifHeader = new byte[] { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00 }; + // This uses C# compiler's optimization to refer to the static data directly, no intermediate array allocations happen. + private static ReadOnlySpan ExifHeader => new byte[] { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00 }; /// /// Initializes a new instance of the class. From 7f4c9cdd8fd337d13e27c79bb045928ccb2fd70b Mon Sep 17 00:00:00 2001 From: jubilant-enigma <54286000+jubilant-enigma@users.noreply.github.com> Date: Mon, 6 Dec 2021 10:33:08 -0800 Subject: [PATCH 04/17] Update src/ImageSharp/Formats/Png/PngDecoderCore.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Günther Foidl --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 6db2328f58..a08242e687 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1022,7 +1022,7 @@ private void ReadLegacyExifTextChunk(ImageMetadata metadata, string data) for (int i = 0; i < dataLength; i++) { byte parsed = Convert.ToByte(dataSpanString.Substring(i * 2, 2), 16); - if (i < ExifHeader.Length) + if ((uint)i < (uint)ExifHeader.Length) { if (parsed != ExifHeader[i]) { From 8b9c334bca8f9363893d54235ec5f883c4dc5777 Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Mon, 6 Dec 2021 12:58:02 -0800 Subject: [PATCH 05/17] Moved the ExifHeader property to after the constructor to satisfy StyleCop. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index a08242e687..5a18826d14 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -112,12 +112,6 @@ internal sealed class PngDecoderCore : IImageDecoderInternals /// private PngChunk? nextChunk; - /// - /// "Exif" and two zero bytes. Used for the legacy exif parsing. - /// - // This uses C# compiler's optimization to refer to the static data directly, no intermediate array allocations happen. - private static ReadOnlySpan ExifHeader => new byte[] { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00 }; - /// /// Initializes a new instance of the class. /// @@ -130,6 +124,12 @@ public PngDecoderCore(Configuration configuration, IPngDecoderOptions options) this.ignoreMetadata = options.IgnoreMetadata; } + /// + /// Gets the sequence of bytes for the exif header ("Exif" ASCII and two zero bytes). Used for the legacy exif parsing. + /// + // This uses C# compiler's optimization to refer to the static data directly, no intermediate array allocations happen. + private static ReadOnlySpan ExifHeader => new byte[] { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00 }; + /// public Configuration Configuration { get; } From c2b906ee12262957c1b91d70d89edd9dd4c4d2a5 Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Mon, 6 Dec 2021 13:04:56 -0800 Subject: [PATCH 06/17] Removed unnecessary temporary allocations. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 134 +++++++++++++++++-- 1 file changed, 120 insertions(+), 14 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 5a18826d14..236c447ab2 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -997,7 +997,7 @@ private void ReadLegacyExifTextChunk(ImageMetadata metadata, string data) ReadOnlySpan dataSpan = data.AsSpan(); dataSpan = dataSpan.TrimStart(); - if (!dataSpan.Slice(0, 4).ToString().Equals("exif", StringComparison.OrdinalIgnoreCase)) + if (!StringEquals(dataSpan.Slice(0, 4), "exif".AsSpan(), StringComparison.OrdinalIgnoreCase)) { // "exif" identifier is missing from the beginning of the text chunk return; @@ -1006,37 +1006,143 @@ private void ReadLegacyExifTextChunk(ImageMetadata metadata, string data) // Skip to the data length dataSpan = dataSpan.Slice(4).TrimStart(); int dataLengthEnd = dataSpan.IndexOf('\n'); - int dataLength = int.Parse(dataSpan.Slice(0, dataSpan.IndexOf('\n')).ToString()); + int dataLength = ParseInt32(dataSpan.Slice(0, dataSpan.IndexOf('\n'))); // Skip to the hex-encoded data dataSpan = dataSpan.Slice(dataLengthEnd).Trim(); - string dataSpanString = dataSpan.ToString().Replace("\n", string.Empty); - if (dataSpanString.Length != (dataLength * 2)) + + if (dataLength < ExifHeader.Length) { - // Invalid length + // Not enough room for the required exif header, this data couldn't possibly be valid return; } // Parse the hex-encoded data into the byte array we are going to hand off to ExifProfile - byte[] dataBlob = new byte[dataLength - ExifHeader.Length]; - for (int i = 0; i < dataLength; i++) + byte[] exifBlob = new byte[dataLength - ExifHeader.Length]; + + try { - byte parsed = Convert.ToByte(dataSpanString.Substring(i * 2, 2), 16); - if ((uint)i < (uint)ExifHeader.Length) + // Check for the presence of the exif header in the hex-encoded binary data + byte[] tempExifBuf = exifBlob; + if (exifBlob.Length < ExifHeader.Length) + { + // Need to allocate a temporary array, this should be an extremely uncommon (TODO: impossible?) case + tempExifBuf = new byte[ExifHeader.Length]; + } + + HexStringToBytes(dataSpan.Slice(0, ExifHeader.Length * 2), tempExifBuf.AsSpan()); + if (!tempExifBuf.AsSpan().Slice(0, ExifHeader.Length).SequenceEqual(ExifHeader)) { - if (parsed != ExifHeader[i]) + // Exif header in the hex data is not valid + return; + } + + // Skip over the exif header we just tested + dataSpan = dataSpan.Slice(ExifHeader.Length * 2); + dataLength -= ExifHeader.Length; + + // Load the hex-encoded data, one line at a time + for (int i = 0; i < dataLength;) + { + ReadOnlySpan lineSpan = dataSpan; + + int newlineIndex = dataSpan.IndexOf('\n'); + if (newlineIndex != -1) { - // Invalid exif header in the actual data blob - return; + lineSpan = dataSpan.Slice(0, newlineIndex); } + + i += HexStringToBytes(lineSpan, exifBlob.AsSpan().Slice(i)); + + dataSpan = dataSpan.Slice(newlineIndex + 1); + } + } + catch + { + return; + } + + this.MergeOrSetExifProfile(metadata, new ExifProfile(exifBlob), replaceExistingKeys: false); + } + + private static bool StringEquals(ReadOnlySpan span1, ReadOnlySpan span2, StringComparison comparisonType) + { +#pragma warning disable IDE0022 // Use expression body for methods +#if NETSTANDARD2_1 || NETCOREAPP2_1_OR_GREATER + return span1.Equals(span2, comparisonType); +#else + return span1.ToString().Equals(span2.ToString(), comparisonType); +#endif +#pragma warning restore IDE0022 // Use expression body for methods + } + + /// + /// int.Parse() a ReadOnlySpan<char>, with a fallback for older frameworks. + /// + /// The to parse. + /// The of the integer to parse. + /// The to use when parsing the integer. + /// The parsed . + private static int ParseInt32( + ReadOnlySpan span, + System.Globalization.NumberStyles style = System.Globalization.NumberStyles.Integer, + IFormatProvider provider = null) + { +#pragma warning disable IDE0022 // Use expression body for methods +#if NETSTANDARD2_1 || NETCOREAPP2_1_OR_GREATER + return int.Parse(span, style, provider); +#else + return int.Parse(span.ToString(), style, provider); +#endif +#pragma warning restore IDE0022 // Use expression body for methods + } + + /// + /// Parses a hexadecimal string into a byte array without allocations. + /// Adapted from https://stackoverflow.com/a/9995303/871842 + /// + /// The hexadecimal string to parse. + /// The destination for the parsed bytes. Must be at least .Length / 2 bytes long. + /// The number of bytes written to . + private static int HexStringToBytes(ReadOnlySpan hexString, Span outputBytes) + { + if ((hexString.Length % 2) != 0) + { + throw new ArgumentException("Input string length must be a multiple of 2", nameof(hexString)); + } + + if ((outputBytes.Length * 2) < hexString.Length) + { + throw new ArgumentException("Output span must be at least half the length of the input string"); + } + + static int GetHexVal(char hexChar) + { + if (hexChar >= '0' && hexChar <= '9') + { + return hexChar - '0'; + } + else if (hexChar >= 'A' && hexChar <= 'F') + { + return 10 + (hexChar - 'A'); + } + else if (hexChar >= 'a' && hexChar <= 'f') + { + return 10 + (hexChar - 'a'); } else { - dataBlob[i - ExifHeader.Length] = parsed; + throw new ArgumentException($"Invalid hexadecimal value {hexChar}"); } } - this.MergeOrSetExifProfile(metadata, new ExifProfile(dataBlob), replaceExistingKeys: false); + int inputByteCount = hexString.Length / 2; + for (int i = 0; i < inputByteCount; i++) + { + outputBytes[i] = (byte)((GetHexVal(hexString[i * 2]) << 4) + GetHexVal(hexString[(i * 2) + 1])); + } + + return inputByteCount; } /// From c8e2902be2e1ad4e271e316b36e946eaa2d35e43 Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Mon, 6 Dec 2021 13:14:35 -0800 Subject: [PATCH 07/17] Moved legacy exif data loading test from PngDecoderTests to PngMetadataTests. --- .../Formats/Png/PngDecoderTests.cs | 19 -------------- .../Formats/Png/PngMetadataTests.cs | 26 +++++++++++++++++++ 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs index 215cd14a1a..9fc4d03dda 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs @@ -444,24 +444,5 @@ static void RunTest(string providerDump, string nonContiguousBuffersStr) "Disco") .Dispose(); } - - [Theory] - [WithFile(TestImages.Png.Issue1875, PixelTypes.Rgba32)] - public void PngDecoder_CanDecode_LegacyTextExifChunk(TestImageProvider provider) - { - using Image image = provider.GetImage(PngDecoder); - - Assert.Equal(0, image.Metadata.ExifProfile.InvalidTags.Count); - Assert.Equal(3, image.Metadata.ExifProfile.Values.Count); - - Assert.Equal( - "A colorful tiling of blue, red, yellow, and green 4x4 pixel blocks.", - image.Metadata.ExifProfile.GetValue(ImageSharp.Metadata.Profiles.Exif.ExifTag.ImageDescription).Value); - Assert.Equal( - "Duplicated from basn3p02.png, then image metadata modified with exiv2", - image.Metadata.ExifProfile.GetValue(ImageSharp.Metadata.Profiles.Exif.ExifTag.ImageHistory).Value); - - Assert.Equal(42, (int)image.Metadata.ExifProfile.GetValue(ImageSharp.Metadata.Profiles.Exif.ExifTag.ImageNumber).Value); - } } } diff --git a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs index b4307af5d1..d763d2ba00 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs @@ -289,5 +289,31 @@ private static void VerifyTextDataIsPresent(PngMetadata meta) Assert.Contains(meta.TextData, m => m.Keyword is "NoLang" && m.Value is "this text chunk is missing a language tag"); Assert.Contains(meta.TextData, m => m.Keyword is "NoTranslatedKeyword" && m.Value is "dieser chunk hat kein übersetztes Schlüßelwort"); } + + [Theory] + [InlineData(TestImages.Png.Issue1875)] + public void Identify_ReadsLegacyExifData(string imagePath) + { + var testFile = TestFile.Create(imagePath); + using (var stream = new MemoryStream(testFile.Bytes, false)) + { + IImageInfo imageInfo = Image.Identify(stream); + Assert.NotNull(imageInfo); + Assert.NotNull(imageInfo.Metadata.ExifProfile); + ExifProfile exif = imageInfo.Metadata.ExifProfile; + + Assert.Equal(0, exif.InvalidTags.Count); + Assert.Equal(3, exif.Values.Count); + + Assert.Equal( + "A colorful tiling of blue, red, yellow, and green 4x4 pixel blocks.", + exif.GetValue(ExifTag.ImageDescription).Value); + Assert.Equal( + "Duplicated from basn3p02.png, then image metadata modified with exiv2", + exif.GetValue(ExifTag.ImageHistory).Value); + + Assert.Equal(42, (int)exif.GetValue(ExifTag.ImageNumber).Value); + } + } } } From 0c65e13a4ec6becac842f3fda5c4bca92913f97a Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Mon, 6 Dec 2021 21:40:26 -0800 Subject: [PATCH 08/17] Don't save the exif text chunk if it is successfully parsed --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 27 +++++++++++-------- .../Formats/Png/PngMetadataTests.cs | 6 ++++- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 236c447ab2..436e706e82 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -976,12 +976,16 @@ private void ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata met if (this.TryUncompressTextData(compressedData, PngConstants.Encoding, out string uncompressed)) { - metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty)); - } - - if (name.Equals("Raw profile type exif", StringComparison.OrdinalIgnoreCase)) - { - this.ReadLegacyExifTextChunk(baseMetadata, uncompressed); + if (name.Equals("Raw profile type exif", StringComparison.OrdinalIgnoreCase) && + this.TryReadLegacyExifTextChunk(baseMetadata, uncompressed)) + { + // Successfully parsed exif data stored as text in this chunk + } + else + { + // Seems to be regular old text data, or we failed to parse it in any special way + metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty)); + } } } @@ -992,7 +996,7 @@ private void ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata met /// /// The to store the decoded exif tags into. /// The contents of the "raw profile type exif" text chunk. - private void ReadLegacyExifTextChunk(ImageMetadata metadata, string data) + private bool TryReadLegacyExifTextChunk(ImageMetadata metadata, string data) { ReadOnlySpan dataSpan = data.AsSpan(); dataSpan = dataSpan.TrimStart(); @@ -1000,7 +1004,7 @@ private void ReadLegacyExifTextChunk(ImageMetadata metadata, string data) if (!StringEquals(dataSpan.Slice(0, 4), "exif".AsSpan(), StringComparison.OrdinalIgnoreCase)) { // "exif" identifier is missing from the beginning of the text chunk - return; + return false; } // Skip to the data length @@ -1014,7 +1018,7 @@ private void ReadLegacyExifTextChunk(ImageMetadata metadata, string data) if (dataLength < ExifHeader.Length) { // Not enough room for the required exif header, this data couldn't possibly be valid - return; + return false; } // Parse the hex-encoded data into the byte array we are going to hand off to ExifProfile @@ -1034,7 +1038,7 @@ private void ReadLegacyExifTextChunk(ImageMetadata metadata, string data) if (!tempExifBuf.AsSpan().Slice(0, ExifHeader.Length).SequenceEqual(ExifHeader)) { // Exif header in the hex data is not valid - return; + return false; } // Skip over the exif header we just tested @@ -1059,10 +1063,11 @@ private void ReadLegacyExifTextChunk(ImageMetadata metadata, string data) } catch { - return; + return false; } this.MergeOrSetExifProfile(metadata, new ExifProfile(exifBlob), replaceExistingKeys: false); + return true; } private static bool StringEquals(ReadOnlySpan span1, ReadOnlySpan span2, StringComparison comparisonType) diff --git a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs index d763d2ba00..8db1d1aaf2 100644 --- a/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs +++ b/tests/ImageSharp.Tests/Formats/Png/PngMetadataTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Six Labors. // Licensed under the Apache License, Version 2.0. +using System; using System.Collections.Generic; using System.IO; using System.Linq; @@ -300,8 +301,11 @@ public void Identify_ReadsLegacyExifData(string imagePath) IImageInfo imageInfo = Image.Identify(stream); Assert.NotNull(imageInfo); Assert.NotNull(imageInfo.Metadata.ExifProfile); - ExifProfile exif = imageInfo.Metadata.ExifProfile; + PngMetadata meta = imageInfo.Metadata.GetFormatMetadata(PngFormat.Instance); + Assert.DoesNotContain(meta.TextData, t => t.Keyword.Equals("Raw profile type exif", StringComparison.OrdinalIgnoreCase)); + + ExifProfile exif = imageInfo.Metadata.ExifProfile; Assert.Equal(0, exif.InvalidTags.Count); Assert.Equal(3, exif.Values.Count); From bf3035f06b4ad4fd528a6d7826ea8935a4a764ac Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Mon, 6 Dec 2021 21:46:59 -0800 Subject: [PATCH 09/17] Don't include unnecessary parameters for helper functions that are only used once. Added a missing comment for StringEqualsInsensitive. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 26 +++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 436e706e82..721a052401 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1001,7 +1001,7 @@ private bool TryReadLegacyExifTextChunk(ImageMetadata metadata, string data) ReadOnlySpan dataSpan = data.AsSpan(); dataSpan = dataSpan.TrimStart(); - if (!StringEquals(dataSpan.Slice(0, 4), "exif".AsSpan(), StringComparison.OrdinalIgnoreCase)) + if (!StringEqualsInsensitive(dataSpan.Slice(0, 4), "exif".AsSpan())) { // "exif" identifier is missing from the beginning of the text chunk return false; @@ -1070,13 +1070,20 @@ private bool TryReadLegacyExifTextChunk(ImageMetadata metadata, string data) return true; } - private static bool StringEquals(ReadOnlySpan span1, ReadOnlySpan span2, StringComparison comparisonType) + /// + /// Compares two ReadOnlySpan<char>s in a case-insensitive method. + /// This is only needed because older frameworks are missing the extension method. + /// + /// The first to compare. + /// The second to compare. + /// True if the spans were identical, false otherwise. + private static bool StringEqualsInsensitive(ReadOnlySpan span1, ReadOnlySpan span2) { #pragma warning disable IDE0022 // Use expression body for methods #if NETSTANDARD2_1 || NETCOREAPP2_1_OR_GREATER - return span1.Equals(span2, comparisonType); + return span1.Equals(span2, StringComparison.OrdinalIgnoreCase); #else - return span1.ToString().Equals(span2.ToString(), comparisonType); + return span1.ToString().Equals(span2.ToString(), StringComparison.OrdinalIgnoreCase); #endif #pragma warning restore IDE0022 // Use expression body for methods } @@ -1085,19 +1092,14 @@ private static bool StringEquals(ReadOnlySpan span1, ReadOnlySpan sp /// int.Parse() a ReadOnlySpan<char>, with a fallback for older frameworks. /// /// The to parse. - /// The of the integer to parse. - /// The to use when parsing the integer. /// The parsed . - private static int ParseInt32( - ReadOnlySpan span, - System.Globalization.NumberStyles style = System.Globalization.NumberStyles.Integer, - IFormatProvider provider = null) + private static int ParseInt32(ReadOnlySpan span) { #pragma warning disable IDE0022 // Use expression body for methods #if NETSTANDARD2_1 || NETCOREAPP2_1_OR_GREATER - return int.Parse(span, style, provider); + return int.Parse(span); #else - return int.Parse(span.ToString(), style, provider); + return int.Parse(span.ToString()); #endif #pragma warning restore IDE0022 // Use expression body for methods } From 2a7ec5dc66de7387c08f93d678d8e4e84b5b7eed Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Tue, 28 Dec 2021 03:29:31 -0800 Subject: [PATCH 10/17] Moved ExifHeader to a local variable since it's only used in one function anyway. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 26 +++++++++----------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 721a052401..cb1fc3e45a 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -124,12 +124,6 @@ public PngDecoderCore(Configuration configuration, IPngDecoderOptions options) this.ignoreMetadata = options.IgnoreMetadata; } - /// - /// Gets the sequence of bytes for the exif header ("Exif" ASCII and two zero bytes). Used for the legacy exif parsing. - /// - // This uses C# compiler's optimization to refer to the static data directly, no intermediate array allocations happen. - private static ReadOnlySpan ExifHeader => new byte[] { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00 }; - /// public Configuration Configuration { get; } @@ -1015,35 +1009,39 @@ private bool TryReadLegacyExifTextChunk(ImageMetadata metadata, string data) // Skip to the hex-encoded data dataSpan = dataSpan.Slice(dataLengthEnd).Trim(); - if (dataLength < ExifHeader.Length) + // Sequence of bytes for the exif header ("Exif" ASCII and two zero bytes). + // This doesn't actually allocate. + ReadOnlySpan exifHeader = new byte[] { 0x45, 0x78, 0x69, 0x66, 0x00, 0x00 }; + + if (dataLength < exifHeader.Length) { // Not enough room for the required exif header, this data couldn't possibly be valid return false; } // Parse the hex-encoded data into the byte array we are going to hand off to ExifProfile - byte[] exifBlob = new byte[dataLength - ExifHeader.Length]; + byte[] exifBlob = new byte[dataLength - exifHeader.Length]; try { // Check for the presence of the exif header in the hex-encoded binary data byte[] tempExifBuf = exifBlob; - if (exifBlob.Length < ExifHeader.Length) + if (exifBlob.Length < exifHeader.Length) { // Need to allocate a temporary array, this should be an extremely uncommon (TODO: impossible?) case - tempExifBuf = new byte[ExifHeader.Length]; + tempExifBuf = new byte[exifHeader.Length]; } - HexStringToBytes(dataSpan.Slice(0, ExifHeader.Length * 2), tempExifBuf.AsSpan()); - if (!tempExifBuf.AsSpan().Slice(0, ExifHeader.Length).SequenceEqual(ExifHeader)) + HexStringToBytes(dataSpan.Slice(0, exifHeader.Length * 2), tempExifBuf.AsSpan()); + if (!tempExifBuf.AsSpan().Slice(0, exifHeader.Length).SequenceEqual(exifHeader)) { // Exif header in the hex data is not valid return false; } // Skip over the exif header we just tested - dataSpan = dataSpan.Slice(ExifHeader.Length * 2); - dataLength -= ExifHeader.Length; + dataSpan = dataSpan.Slice(exifHeader.Length * 2); + dataLength -= exifHeader.Length; // Load the hex-encoded data, one line at a time for (int i = 0; i < dataLength;) From 82e664aad3a21dac75f07a55df6c95eeed3e3b23 Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Tue, 28 Dec 2021 04:01:03 -0800 Subject: [PATCH 11/17] New, faster HexStringToBytes implementation based off the reference source implementation of Convert.FromHexString(): https://source.dot.net/#System.Private.CoreLib/Convert.cs,c9e4fbeaca708991 --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 95 ++++++++++++++------ 1 file changed, 67 insertions(+), 28 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index cb1fc3e45a..ef133decee 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -1032,7 +1032,7 @@ private bool TryReadLegacyExifTextChunk(ImageMetadata metadata, string data) tempExifBuf = new byte[exifHeader.Length]; } - HexStringToBytes(dataSpan.Slice(0, exifHeader.Length * 2), tempExifBuf.AsSpan()); + HexStringToBytes(dataSpan.Slice(0, exifHeader.Length * 2), tempExifBuf); if (!tempExifBuf.AsSpan().Slice(0, exifHeader.Length).SequenceEqual(exifHeader)) { // Exif header in the hex data is not valid @@ -1103,51 +1103,90 @@ private static int ParseInt32(ReadOnlySpan span) } /// - /// Parses a hexadecimal string into a byte array without allocations. - /// Adapted from https://stackoverflow.com/a/9995303/871842 + /// Parses a hexadecimal string into a byte array without allocations. Throws on non-hexadecimal character. + /// Adapted from https://source.dot.net/#System.Private.CoreLib/Convert.cs,c9e4fbeaca708991. /// - /// The hexadecimal string to parse. - /// The destination for the parsed bytes. Must be at least .Length / 2 bytes long. - /// The number of bytes written to . - private static int HexStringToBytes(ReadOnlySpan hexString, Span outputBytes) + /// The hexadecimal string to parse. + /// The destination for the parsed bytes. Must be at least .Length / 2 bytes long. + /// The number of bytes written to . + private static int HexStringToBytes(ReadOnlySpan chars, Span bytes) { - if ((hexString.Length % 2) != 0) + if ((chars.Length % 2) != 0) { - throw new ArgumentException("Input string length must be a multiple of 2", nameof(hexString)); + throw new ArgumentException("Input string length must be a multiple of 2", nameof(chars)); } - if ((outputBytes.Length * 2) < hexString.Length) + if ((bytes.Length * 2) < chars.Length) { throw new ArgumentException("Output span must be at least half the length of the input string"); } + else + { + // Slightly better performance in the loop below, allows us to skip a bounds check + // while still supporting output buffers that are larger than necessary + bytes = bytes.Slice(0, chars.Length / 2); + } - static int GetHexVal(char hexChar) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + static int FromChar(int c) { - if (hexChar >= '0' && hexChar <= '9') - { - return hexChar - '0'; - } - else if (hexChar >= 'A' && hexChar <= 'F') + // Map from an ASCII char to its hex value, e.g. arr['b'] == 11. 0xFF means it's not a hex digit. + // This doesn't actually allocate. + ReadOnlySpan charToHexLookup = new byte[] { - return 10 + (hexChar - 'A'); - } - else if (hexChar >= 'a' && hexChar <= 'f') - { - return 10 + (hexChar - 'a'); - } - else + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 15 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 31 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 47 + 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 63 + 0xFF, 0xA, 0xB, 0xC, 0xD, 0xE, 0xF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 79 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 95 + 0xFF, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 111 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 127 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 143 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 159 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 175 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 191 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 207 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 223 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 239 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 255 + }; + + return c >= charToHexLookup.Length ? 0xFF : charToHexLookup[c]; + } + + // See https://source.dot.net/#System.Private.CoreLib/HexConverter.cs,4681d45a0aa0b361 + int i = 0; + int j = 0; + int byteLo = 0; + int byteHi = 0; + while (j < bytes.Length) + { + byteLo = FromChar(chars[i + 1]); + byteHi = FromChar(chars[i]); + + // byteHi hasn't been shifted to the high half yet, so the only way the bitwise or produces this pattern + // is if either byteHi or byteLo was not a hex character. + if ((byteLo | byteHi) == 0xFF) { - throw new ArgumentException($"Invalid hexadecimal value {hexChar}"); + break; } + + bytes[j++] = (byte)((byteHi << 4) | byteLo); + i += 2; + } + + if (byteLo == 0xFF) + { + i++; } - int inputByteCount = hexString.Length / 2; - for (int i = 0; i < inputByteCount; i++) + if ((byteLo | byteHi) == 0xFF) { - outputBytes[i] = (byte)((GetHexVal(hexString[i * 2]) << 4) + GetHexVal(hexString[(i * 2) + 1])); + throw new ArgumentException("Input string contained non-hexadecimal characters", nameof(chars)); } - return inputByteCount; + return j; } /// From 76261ff808d4b78cee3c392c0d0418c510986909 Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Mon, 3 Jan 2022 22:23:52 +1100 Subject: [PATCH 12/17] Update shared-infrastructure --- shared-infrastructure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-infrastructure b/shared-infrastructure index a042aba176..59ce17f5a4 160000 --- a/shared-infrastructure +++ b/shared-infrastructure @@ -1 +1 @@ -Subproject commit a042aba176cdb840d800c6ed4cfe41a54fb7b1e3 +Subproject commit 59ce17f5a4e1f956811133f41add7638e74c2836 From 6dba6cf1f8fb07847d0e5036557679a202a4d9c6 Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Mon, 3 Jan 2022 14:45:35 -0800 Subject: [PATCH 13/17] Moved HexStringToBytes into a SixLabors.ImageSharp.Common.Helpers.HexConverter. --- src/ImageSharp/Common/Helpers/HexConverter.cs | 98 +++++++++++++++++++ src/ImageSharp/Formats/Png/PngDecoderCore.cs | 92 +---------------- 2 files changed, 101 insertions(+), 89 deletions(-) create mode 100644 src/ImageSharp/Common/Helpers/HexConverter.cs diff --git a/src/ImageSharp/Common/Helpers/HexConverter.cs b/src/ImageSharp/Common/Helpers/HexConverter.cs new file mode 100644 index 0000000000..c55e9bbd9d --- /dev/null +++ b/src/ImageSharp/Common/Helpers/HexConverter.cs @@ -0,0 +1,98 @@ +// Copyright (c) Six Labors. +// Licensed under the Apache License, Version 2.0. + +using System; +using System.Runtime.CompilerServices; + +namespace SixLabors.ImageSharp.Common.Helpers +{ + internal static class HexConverter + { + /// + /// Parses a hexadecimal string into a byte array without allocations. Throws on non-hexadecimal character. + /// Adapted from https://source.dot.net/#System.Private.CoreLib/Convert.cs,c9e4fbeaca708991. + /// + /// The hexadecimal string to parse. + /// The destination for the parsed bytes. Must be at least .Length / 2 bytes long. + /// The number of bytes written to . + public static int HexStringToBytes(ReadOnlySpan chars, Span bytes) + { + if ((chars.Length % 2) != 0) + { + throw new ArgumentException("Input string length must be a multiple of 2", nameof(chars)); + } + + if ((bytes.Length * 2) < chars.Length) + { + throw new ArgumentException("Output span must be at least half the length of the input string"); + } + else + { + // Slightly better performance in the loop below, allows us to skip a bounds check + // while still supporting output buffers that are larger than necessary + bytes = bytes.Slice(0, chars.Length / 2); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + static int FromChar(int c) + { + // Map from an ASCII char to its hex value, e.g. arr['b'] == 11. 0xFF means it's not a hex digit. + // This doesn't actually allocate. + ReadOnlySpan charToHexLookup = new byte[] + { + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 15 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 31 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 47 + 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 63 + 0xFF, 0xA, 0xB, 0xC, 0xD, 0xE, 0xF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 79 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 95 + 0xFF, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 111 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 127 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 143 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 159 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 175 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 191 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 207 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 223 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 239 + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 255 + }; + + return c >= charToHexLookup.Length ? 0xFF : charToHexLookup[c]; + } + + // See https://source.dot.net/#System.Private.CoreLib/HexConverter.cs,4681d45a0aa0b361 + int i = 0; + int j = 0; + int byteLo = 0; + int byteHi = 0; + while (j < bytes.Length) + { + byteLo = FromChar(chars[i + 1]); + byteHi = FromChar(chars[i]); + + // byteHi hasn't been shifted to the high half yet, so the only way the bitwise or produces this pattern + // is if either byteHi or byteLo was not a hex character. + if ((byteLo | byteHi) == 0xFF) + { + break; + } + + bytes[j++] = (byte)((byteHi << 4) | byteLo); + i += 2; + } + + if (byteLo == 0xFF) + { + i++; + } + + if ((byteLo | byteHi) == 0xFF) + { + throw new ArgumentException("Input string contained non-hexadecimal characters", nameof(chars)); + } + + return j; + } + } +} diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 182d6e7bea..708cd7f5f4 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -11,6 +11,7 @@ using System.Runtime.InteropServices; using System.Text; using System.Threading; +using SixLabors.ImageSharp.Common.Helpers; using SixLabors.ImageSharp.Compression.Zlib; using SixLabors.ImageSharp.Formats.Png.Chunks; using SixLabors.ImageSharp.Formats.Png.Filters; @@ -1093,7 +1094,7 @@ private bool TryReadLegacyExifTextChunk(ImageMetadata metadata, string data) tempExifBuf = new byte[exifHeader.Length]; } - HexStringToBytes(dataSpan.Slice(0, exifHeader.Length * 2), tempExifBuf); + HexConverter.HexStringToBytes(dataSpan.Slice(0, exifHeader.Length * 2), tempExifBuf); if (!tempExifBuf.AsSpan().Slice(0, exifHeader.Length).SequenceEqual(exifHeader)) { // Exif header in the hex data is not valid @@ -1115,7 +1116,7 @@ private bool TryReadLegacyExifTextChunk(ImageMetadata metadata, string data) lineSpan = dataSpan.Slice(0, newlineIndex); } - i += HexStringToBytes(lineSpan, exifBlob.AsSpan().Slice(i)); + i += HexConverter.HexStringToBytes(lineSpan, exifBlob.AsSpan().Slice(i)); dataSpan = dataSpan.Slice(newlineIndex + 1); } @@ -1163,93 +1164,6 @@ private static int ParseInt32(ReadOnlySpan span) #pragma warning restore IDE0022 // Use expression body for methods } - /// - /// Parses a hexadecimal string into a byte array without allocations. Throws on non-hexadecimal character. - /// Adapted from https://source.dot.net/#System.Private.CoreLib/Convert.cs,c9e4fbeaca708991. - /// - /// The hexadecimal string to parse. - /// The destination for the parsed bytes. Must be at least .Length / 2 bytes long. - /// The number of bytes written to . - private static int HexStringToBytes(ReadOnlySpan chars, Span bytes) - { - if ((chars.Length % 2) != 0) - { - throw new ArgumentException("Input string length must be a multiple of 2", nameof(chars)); - } - - if ((bytes.Length * 2) < chars.Length) - { - throw new ArgumentException("Output span must be at least half the length of the input string"); - } - else - { - // Slightly better performance in the loop below, allows us to skip a bounds check - // while still supporting output buffers that are larger than necessary - bytes = bytes.Slice(0, chars.Length / 2); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - static int FromChar(int c) - { - // Map from an ASCII char to its hex value, e.g. arr['b'] == 11. 0xFF means it's not a hex digit. - // This doesn't actually allocate. - ReadOnlySpan charToHexLookup = new byte[] - { - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 15 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 31 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 47 - 0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 63 - 0xFF, 0xA, 0xB, 0xC, 0xD, 0xE, 0xF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 79 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 95 - 0xFF, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 111 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 127 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 143 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 159 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 175 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 191 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 207 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 223 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 239 - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, // 255 - }; - - return c >= charToHexLookup.Length ? 0xFF : charToHexLookup[c]; - } - - // See https://source.dot.net/#System.Private.CoreLib/HexConverter.cs,4681d45a0aa0b361 - int i = 0; - int j = 0; - int byteLo = 0; - int byteHi = 0; - while (j < bytes.Length) - { - byteLo = FromChar(chars[i + 1]); - byteHi = FromChar(chars[i]); - - // byteHi hasn't been shifted to the high half yet, so the only way the bitwise or produces this pattern - // is if either byteHi or byteLo was not a hex character. - if ((byteLo | byteHi) == 0xFF) - { - break; - } - - bytes[j++] = (byte)((byteHi << 4) | byteLo); - i += 2; - } - - if (byteLo == 0xFF) - { - i++; - } - - if ((byteLo | byteHi) == 0xFF) - { - throw new ArgumentException("Input string contained non-hexadecimal characters", nameof(chars)); - } - - return j; - } - /// /// Sets the in to , /// or copies exif tags if already contains an . From bdb69d1027c13d8b3ded75bb83703c1d6fb3d3e6 Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Mon, 3 Jan 2022 15:03:18 -0800 Subject: [PATCH 14/17] Allow reading legacy exif data from uncompressed text chunks as well. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 49 ++++++++++++++------ 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 708cd7f5f4..2a8f9614b7 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -188,7 +188,7 @@ public Image Decode(BufferedReadStream stream, CancellationToken this.AssignTransparentMarkers(alpha, pngMetadata); break; case PngChunkType.Text: - this.ReadTextChunk(pngMetadata, chunk.Data.GetSpan()); + this.ReadTextChunk(metadata, pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.CompressedText: this.ReadCompressedTextChunk(metadata, pngMetadata, chunk.Data.GetSpan()); @@ -298,7 +298,7 @@ public IImageInfo Identify(BufferedReadStream stream, CancellationToken cancella break; } - this.ReadTextChunk(pngMetadata, chunk.Data.GetSpan()); + this.ReadTextChunk(metadata, pngMetadata, chunk.Data.GetSpan()); break; case PngChunkType.CompressedText: if (this.colorMetadataOnly) @@ -970,7 +970,7 @@ private void ReadHeaderChunk(PngMetadata pngMetadata, ReadOnlySpan data) /// /// The metadata to decode to. /// The containing the data. - private void ReadTextChunk(PngMetadata metadata, ReadOnlySpan data) + private void ReadTextChunk(ImageMetadata baseMetadata, PngMetadata metadata, ReadOnlySpan data) { if (this.ignoreMetadata) { @@ -993,7 +993,10 @@ private void ReadTextChunk(PngMetadata metadata, ReadOnlySpan data) string value = PngConstants.Encoding.GetString(data.Slice(zeroIndex + 1)); - metadata.TextData.Add(new PngTextData(name, value, string.Empty, string.Empty)); + if (!this.TryReadSpecialTextData(baseMetadata, name, value)) + { + metadata.TextData.Add(new PngTextData(name, value, string.Empty, string.Empty)); + } } /// @@ -1030,19 +1033,35 @@ private void ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata met ReadOnlySpan compressedData = data.Slice(zeroIndex + 2); - if (this.TryUncompressTextData(compressedData, PngConstants.Encoding, out string uncompressed)) + if (this.TryUncompressTextData(compressedData, PngConstants.Encoding, out string uncompressed) && + !this.TryReadSpecialTextData(baseMetadata, name, uncompressed)) { - if (name.Equals("Raw profile type exif", StringComparison.OrdinalIgnoreCase) && - this.TryReadLegacyExifTextChunk(baseMetadata, uncompressed)) - { - // Successfully parsed exif data stored as text in this chunk - } - else - { - // Seems to be regular old text data, or we failed to parse it in any special way - metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty)); - } + metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty)); + } + } + + /// + /// Checks if the given text chunk is actually storing parsable metadata. + /// + /// + /// + /// + /// True if metadata was successfully parsed from the text chunk. False if the + /// text chunk was not identified as metadata, and should be stored in the metadata + /// object unmodified. + private bool TryReadSpecialTextData(ImageMetadata baseMetadata, string chunkName, string chunkText) + { + if (chunkName.Equals("Raw profile type exif", StringComparison.OrdinalIgnoreCase) && + this.TryReadLegacyExifTextChunk(baseMetadata, chunkText)) + { + // Successfully parsed legacy exif data from text + return true; } + + // TODO: "Raw profile type iptc", potentially others? + + // No special chunk data identified + return false; } /// From 6cdc595583fa6798c2fec04de8d016a4b13b4047 Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Mon, 3 Jan 2022 15:04:00 -0800 Subject: [PATCH 15/17] Merge with remote --- shared-infrastructure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-infrastructure b/shared-infrastructure index 59ce17f5a4..a042aba176 160000 --- a/shared-infrastructure +++ b/shared-infrastructure @@ -1 +1 @@ -Subproject commit 59ce17f5a4e1f956811133f41add7638e74c2836 +Subproject commit a042aba176cdb840d800c6ed4cfe41a54fb7b1e3 From 7e7ea93882d7721dfcc7a83844e26a343bbd3a3d Mon Sep 17 00:00:00 2001 From: "WINDEV2110EVAL\\User" Date: Mon, 3 Jan 2022 15:12:34 -0800 Subject: [PATCH 16/17] Fixed comments. --- src/ImageSharp/Formats/Png/PngDecoderCore.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ImageSharp/Formats/Png/PngDecoderCore.cs b/src/ImageSharp/Formats/Png/PngDecoderCore.cs index 2a8f9614b7..c9f0ce3755 100644 --- a/src/ImageSharp/Formats/Png/PngDecoderCore.cs +++ b/src/ImageSharp/Formats/Png/PngDecoderCore.cs @@ -968,6 +968,7 @@ private void ReadHeaderChunk(PngMetadata pngMetadata, ReadOnlySpan data) /// /// Reads a text chunk containing image properties from the data. /// + /// The object. /// The metadata to decode to. /// The containing the data. private void ReadTextChunk(ImageMetadata baseMetadata, PngMetadata metadata, ReadOnlySpan data) @@ -993,7 +994,7 @@ private void ReadTextChunk(ImageMetadata baseMetadata, PngMetadata metadata, Rea string value = PngConstants.Encoding.GetString(data.Slice(zeroIndex + 1)); - if (!this.TryReadSpecialTextData(baseMetadata, name, value)) + if (!this.TryReadTextChunkMetadata(baseMetadata, name, value)) { metadata.TextData.Add(new PngTextData(name, value, string.Empty, string.Empty)); } @@ -1034,7 +1035,7 @@ private void ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata met ReadOnlySpan compressedData = data.Slice(zeroIndex + 2); if (this.TryUncompressTextData(compressedData, PngConstants.Encoding, out string uncompressed) && - !this.TryReadSpecialTextData(baseMetadata, name, uncompressed)) + !this.TryReadTextChunkMetadata(baseMetadata, name, uncompressed)) { metadata.TextData.Add(new PngTextData(name, uncompressed, string.Empty, string.Empty)); } @@ -1043,13 +1044,13 @@ private void ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata met /// /// Checks if the given text chunk is actually storing parsable metadata. /// - /// - /// - /// + /// The object to store the parsed metadata in. + /// The name of the text chunk. + /// The contents of the text chunk. /// True if metadata was successfully parsed from the text chunk. False if the /// text chunk was not identified as metadata, and should be stored in the metadata /// object unmodified. - private bool TryReadSpecialTextData(ImageMetadata baseMetadata, string chunkName, string chunkText) + private bool TryReadTextChunkMetadata(ImageMetadata baseMetadata, string chunkName, string chunkText) { if (chunkName.Equals("Raw profile type exif", StringComparison.OrdinalIgnoreCase) && this.TryReadLegacyExifTextChunk(baseMetadata, chunkText)) From 95318b1d9d93d84dc295e0f648d8e2bd6f0d7b4d Mon Sep 17 00:00:00 2001 From: James Jackson-South Date: Tue, 4 Jan 2022 22:55:56 +1100 Subject: [PATCH 17/17] Update shared-infrastructure --- shared-infrastructure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared-infrastructure b/shared-infrastructure index a042aba176..59ce17f5a4 160000 --- a/shared-infrastructure +++ b/shared-infrastructure @@ -1 +1 @@ -Subproject commit a042aba176cdb840d800c6ed4cfe41a54fb7b1e3 +Subproject commit 59ce17f5a4e1f956811133f41add7638e74c2836