Review adjustments

src/ImageSharp/Formats/Gif/GifDecoderCore.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;
@@ -334,7 +334,7 @@ private void ReadComments()
                 {
                     this.stream.Read(commentsBuffer.Array, 0, length);
                     string comments = this.TextEncoding.GetString(commentsBuffer.Array, 0, length);
-                    this.metadata.Properties.Add(new ImageProperty(GifConstants.Comments, comments));
+                    this.metadata.GifTextProperties.Add(new GifTextData(GifConstants.Comments, comments));
                 }
             }
         }
@@ -632,4 +632,4 @@ private void ReadLogicalScreenDescriptorAndGlobalColorTable(Stream stream)
             }
         }
     }
-}
+}

src/ImageSharp/Formats/Gif/GifEncoderCore.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;
@@ -335,7 +335,7 @@ private void WriteApplicationExtension(Stream stream, ushort repeatCount)
         /// <param name="stream">The stream to write to.</param>
         private void WriteComments(ImageMetadata metadata, Stream stream)
         {
-            if (!metadata.TryGetProperty(GifConstants.Comments, out ImageProperty property)
+            if (!metadata.TryGetGifTextProperty(GifConstants.Comments, out GifTextData property)
                 || string.IsNullOrEmpty(property.Value))
             {
                 return;
@@ -458,4 +458,4 @@ private void WriteImageData<TPixel>(IQuantizedFrame<TPixel> image, Stream stream
             }
         }
     }
-}
+}

src/ImageSharp/Formats/Png/PngConstants.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.Collections.Generic;
@@ -54,5 +54,15 @@ internal static class PngConstants
             [PngColorType.GrayscaleWithAlpha] = new byte[] { 8, 16 },
             [PngColorType.RgbWithAlpha] = new byte[] { 8, 16 }
         };
+
+        /// <summary>
+        /// The maximum length of keyword in a text chunk is 79 bytes.
+        /// </summary>
+        public const int MaxTextKeywordLength = 79;
+
+        /// <summary>
+        /// The minimum length of a keyword in a text chunk is 1 byte.
+        /// </summary>
+        public const int MinTextKeywordLength = 1;
     }
-}
+}

src/ImageSharp/Formats/Png/PngDecoderCore.cs

@@ -5,7 +5,6 @@
 using System.Buffers.Binary;
 using System.Collections.Generic;
 using System.IO;
-using System.Linq;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Text;
@@ -129,7 +128,7 @@ internal sealed class PngDecoderCore
         /// <summary>
         /// Latin encoding is used for text chunks.
         /// </summary>
-        private Encoding latinEncoding = Encoding.GetEncoding("ISO-8859-1");
+        private static readonly Encoding LatinEncoding = Encoding.GetEncoding("ISO-8859-1");
 
         /// <summary>
         /// Initializes a new instance of the <see cref="PngDecoderCore"/> class.
@@ -884,20 +883,20 @@ private void ReadTextChunk(ImageMetadata metadata, ReadOnlySpan<byte> data)
             int zeroIndex = data.IndexOf((byte)0);
 
             // Keywords are restricted to 1 to 79 bytes in length.
-            if (zeroIndex <= 0 || zeroIndex > 79)
+            if (zeroIndex < PngConstants.MinTextKeywordLength || zeroIndex > PngConstants.MaxTextKeywordLength)
             {
                 return;
             }
 
             ReadOnlySpan<byte> keywordBytes = data.Slice(0, zeroIndex);
-            if (this.TryReadTextKeyword(keywordBytes, out string name))
+            if (!this.TryReadTextKeyword(keywordBytes, out string name))
             {
                 return;
             }
 
-            string value = this.latinEncoding.GetString(data.Slice(zeroIndex + 1));
+            string value = LatinEncoding.GetString(data.Slice(zeroIndex + 1));
 
-            metadata.Properties.Add(new ImageProperty(name, value));
+            metadata.PngTextProperties.Add(new PngTextData(name, value, string.Empty, string.Empty));
         }
 
         /// <summary>
@@ -913,7 +912,7 @@ private void ReadCompressedTextChunk(ImageMetadata metadata, ReadOnlySpan<byte>
             }
 
             int zeroIndex = data.IndexOf((byte)0);
-            if (zeroIndex <= 0 || zeroIndex > 79)
+            if (zeroIndex < PngConstants.MinTextKeywordLength || zeroIndex > PngConstants.MaxTextKeywordLength)
             {
                 return;
             }
@@ -926,13 +925,13 @@ private void ReadCompressedTextChunk(ImageMetadata metadata, ReadOnlySpan<byte>
             }
 
             ReadOnlySpan<byte> keywordBytes = data.Slice(0, zeroIndex);
-            if (this.TryReadTextKeyword(keywordBytes, out string name))
+            if (!this.TryReadTextKeyword(keywordBytes, out string name))
             {
                 return;
             }
 
             ReadOnlySpan<byte> compressedData = data.Slice(zeroIndex + 2);
-            metadata.Properties.Add(new ImageProperty(name, this.UncompressTextData(compressedData, this.latinEncoding)));
+            metadata.PngTextProperties.Add(new PngTextData(name, this.UncompressTextData(compressedData, LatinEncoding), string.Empty, string.Empty));
         }
 
         /// <summary>
@@ -954,7 +953,7 @@ private void ReadInternationalTextChunk(ImageMetadata metadata, ReadOnlySpan<byt
             }
 
             int zeroIndexKeyword = data.IndexOf((byte)0);
-            if (zeroIndexKeyword <= 0 || zeroIndexKeyword > 79)
+            if (zeroIndexKeyword < PngConstants.MinTextKeywordLength || zeroIndexKeyword > PngConstants.MaxTextKeywordLength)
             {
                 return;
             }
@@ -979,14 +978,14 @@ private void ReadInternationalTextChunk(ImageMetadata metadata, ReadOnlySpan<byt
                 return;
             }
 
-            string language = this.latinEncoding.GetString(data.Slice(langStartIdx, languageLength));
+            string language = LatinEncoding.GetString(data.Slice(langStartIdx, languageLength));
 
             int translatedKeywordStartIdx = langStartIdx + languageLength + 1;
             int translatedKeywordLength = data.Slice(translatedKeywordStartIdx).IndexOf((byte)0);
-            string translatedKeyword = this.latinEncoding.GetString(data.Slice(translatedKeywordStartIdx, translatedKeywordLength));
+            string translatedKeyword = LatinEncoding.GetString(data.Slice(translatedKeywordStartIdx, translatedKeywordLength));
 
             ReadOnlySpan<byte> keywordBytes = data.Slice(0, zeroIndexKeyword);
-            if (this.TryReadTextKeyword(keywordBytes, out string name))
+            if (!this.TryReadTextKeyword(keywordBytes, out string name))
             {
                 return;
             }
@@ -995,17 +994,17 @@ private void ReadInternationalTextChunk(ImageMetadata metadata, ReadOnlySpan<byt
             if (compressionFlag == 1)
             {
                 ReadOnlySpan<byte> compressedData = data.Slice(dataStartIdx);
-                metadata.Properties.Add(new ImageProperty(name, this.UncompressTextData(compressedData, Encoding.UTF8)));
+                metadata.PngTextProperties.Add(new PngTextData(name, this.UncompressTextData(compressedData, Encoding.UTF8), language, translatedKeyword));
             }
             else
             {
                 string value = Encoding.UTF8.GetString(data.Slice(dataStartIdx));
-                metadata.Properties.Add(new ImageProperty(name, value));
+                metadata.PngTextProperties.Add(new PngTextData(name, value, language, translatedKeyword));
             }
         }
 
         /// <summary>
-        /// Decompresses a zlib text.
+        /// Decompresses a byte array with zlib compressed text data.
         /// </summary>
         /// <param name="compressedData">Compressed text data bytes.</param>
         /// <param name="encoding">The string encoding to use.</param>
@@ -1017,6 +1016,8 @@ private string UncompressTextData(ReadOnlySpan<byte> compressedData, Encoding en
             {
                 inflateStream.AllocateNewBytes(compressedData.Length);
                 var uncompressedBytes = new List<byte>();
+
+                // Note: this uses the a buffer which is only 4 bytes long to read the stream, maybe allocating a larger buffer makes sense here.
                 int bytesRead = inflateStream.CompressedStream.Read(this.buffer, 0, this.buffer.Length);
                 while (bytesRead != 0)
                 {
@@ -1223,19 +1224,22 @@ private bool TryReadTextKeyword(ReadOnlySpan<byte> keywordBytes, out string name
             name = string.Empty;
 
             // Keywords shall contain only printable Latin-1.
-            if (keywordBytes.ToArray().Any(c => !((c >= 32 && c <= 126) || (c >= 161 && c <= 255))))
+            foreach (byte c in keywordBytes)
             {
-                return true;
+                if (!((c >= 32 && c <= 126) || (c >= 161 && c <= 255)))
+                {
+                    return false;
+                }
             }
 
             // Keywords should not be empty or have leading or trailing whitespace.
-            name = this.latinEncoding.GetString(keywordBytes);
+            name = LatinEncoding.GetString(keywordBytes);
             if (string.IsNullOrWhiteSpace(name) || name.StartsWith(" ") || name.EndsWith(" "))
             {
-                return true;
+                return false;
             }
 
-            return false;
+            return true;
         }
 
         private void SwapBuffers()

src/ImageSharp/Formats/Png/PngEncoderCore.cs

@@ -762,25 +762,30 @@ private void WriteExifChunk(Stream stream, ImageMetadata meta)
         private void WriteTextChunks(Stream stream, ImageMetadata meta)
         {
             const int MaxLatinCode = 255;
-            foreach (ImageProperty imageProperty in meta.Properties)
+            foreach (PngTextData imageProperty in meta.PngTextProperties)
             {
                 bool hasUnicodeCharacters = imageProperty.Value.Any(c => c > MaxLatinCode);
-                if (hasUnicodeCharacters)
+                if (hasUnicodeCharacters || (!string.IsNullOrWhiteSpace(imageProperty.LanguageTag) || !string.IsNullOrWhiteSpace(imageProperty.TranslatedKeyword)))
                 {
                     // Write iTXt chunk.
-                    byte[] keywordBytes = this.latinEncoding.GetBytes(imageProperty.Name);
+                    byte[] keywordBytes = this.latinEncoding.GetBytes(imageProperty.Keyword);
                     byte[] textBytes = imageProperty.Value.Length > this.compressTextThreshold ? this.GetCompressedTextBytes(Encoding.UTF8.GetBytes(imageProperty.Value)) : Encoding.UTF8.GetBytes(imageProperty.Value);
+                    byte[] translatedKeyword = Encoding.UTF8.GetBytes(imageProperty.TranslatedKeyword);
+                    byte[] languageTag = this.latinEncoding.GetBytes(imageProperty.LanguageTag);
 
-                    // Note: The optional language tag and the translated keyword will be omitted.
-                    Span<byte> outputBytes = new byte[keywordBytes.Length + textBytes.Length + 5];
+                    Span<byte> outputBytes = new byte[keywordBytes.Length + textBytes.Length + translatedKeyword.Length + languageTag.Length + 5];
+                    keywordBytes.CopyTo(outputBytes);
                     if (imageProperty.Value.Length > this.compressTextThreshold)
                     {
                         // Indicate that the text is compressed.
                         outputBytes[keywordBytes.Length + 1] = 1;
                     }
 
-                    keywordBytes.CopyTo(outputBytes);
-                    textBytes.CopyTo(outputBytes.Slice(keywordBytes.Length + 5));
+                    int keywordStart = keywordBytes.Length + 3;
+                    languageTag.CopyTo(outputBytes.Slice(keywordStart));
+                    int translatedKeywordStart = keywordStart + languageTag.Length + 1;
+                    translatedKeyword.CopyTo(outputBytes.Slice(translatedKeywordStart));
+                    textBytes.CopyTo(outputBytes.Slice(translatedKeywordStart + translatedKeyword.Length + 1));
                     this.WriteChunk(stream, PngChunkType.InternationalText, outputBytes.ToArray());
                 }
                 else
@@ -789,17 +794,17 @@ private void WriteTextChunks(Stream stream, ImageMetadata meta)
                     {
                         // Write zTXt chunk.
                         byte[] compressedData = this.GetCompressedTextBytes(this.latinEncoding.GetBytes(imageProperty.Value));
-                        Span<byte> outputBytes = new byte[imageProperty.Name.Length + compressedData.Length + 2];
-                        this.latinEncoding.GetBytes(imageProperty.Name).CopyTo(outputBytes);
-                        compressedData.CopyTo(outputBytes.Slice(imageProperty.Name.Length + 2));
+                        Span<byte> outputBytes = new byte[imageProperty.Keyword.Length + compressedData.Length + 2];
+                        this.latinEncoding.GetBytes(imageProperty.Keyword).CopyTo(outputBytes);
+                        compressedData.CopyTo(outputBytes.Slice(imageProperty.Keyword.Length + 2));
                         this.WriteChunk(stream, PngChunkType.CompressedText, outputBytes.ToArray());
                     }
                     else
                     {
                         // Write tEXt chunk.
-                        Span<byte> outputBytes = new byte[imageProperty.Name.Length + imageProperty.Value.Length + 1];
-                        this.latinEncoding.GetBytes(imageProperty.Name).CopyTo(outputBytes);
-                        this.latinEncoding.GetBytes(imageProperty.Value).CopyTo(outputBytes.Slice(imageProperty.Name.Length + 1));
+                        Span<byte> outputBytes = new byte[imageProperty.Keyword.Length + imageProperty.Value.Length + 1];
+                        this.latinEncoding.GetBytes(imageProperty.Keyword).CopyTo(outputBytes);
+                        this.latinEncoding.GetBytes(imageProperty.Value).CopyTo(outputBytes.Slice(imageProperty.Keyword.Length + 1));
                         this.WriteChunk(stream, PngChunkType.Text, outputBytes.ToArray());
                     }
                 }

src/ImageSharp/MetaData/ImageProperty.cs → src/ImageSharp/MetaData/GifTextData.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;
@@ -10,14 +10,14 @@ namespace SixLabors.ImageSharp.Metadata
     /// the copyright information, the date, where the image was created
     /// or some other information.
     /// </summary>
-    public readonly struct ImageProperty : IEquatable<ImageProperty>
+    public readonly struct GifTextData : IEquatable<GifTextData>
     {
         /// <summary>
-        /// Initializes a new instance of the <see cref="ImageProperty"/> struct.
+        /// Initializes a new instance of the <see cref="GifTextData"/> struct.
         /// </summary>
         /// <param name="name">The name of the property.</param>
         /// <param name="value">The value of the property.</param>
-        public ImageProperty(string name, string value)
+        public GifTextData(string name, string value)
         {
             Guard.NotNullOrWhiteSpace(name, nameof(name));
 
@@ -26,7 +26,7 @@ public ImageProperty(string name, string value)
         }
 
         /// <summary>
-        /// Gets the name of this <see cref="ImageProperty"/> indicating which kind of
+        /// Gets the name of this <see cref="GifTextData"/> indicating which kind of
         /// information this property stores.
         /// </summary>
         /// <example>
@@ -36,44 +36,44 @@ public ImageProperty(string name, string value)
         public string Name { get; }
 
         /// <summary>
-        /// Gets the value of this <see cref="ImageProperty"/>.
+        /// Gets the value of this <see cref="GifTextData"/>.
         /// </summary>
         public string Value { get; }
 
         /// <summary>
-        /// Compares two <see cref="ImageProperty"/> objects. The result specifies whether the values
-        /// of the <see cref="ImageProperty.Name"/> or <see cref="ImageProperty.Value"/> properties of the two
-        /// <see cref="ImageProperty"/> objects are equal.
+        /// Compares two <see cref="GifTextData"/> objects. The result specifies whether the values
+        /// of the <see cref="GifTextData.Name"/> or <see cref="GifTextData.Value"/> properties of the two
+        /// <see cref="GifTextData"/> objects are equal.
         /// </summary>
         /// <param name="left">
-        /// The <see cref="ImageProperty"/> on the left side of the operand.
+        /// The <see cref="GifTextData"/> on the left side of the operand.
         /// </param>
         /// <param name="right">
-        /// The <see cref="ImageProperty"/> on the right side of the operand.
+        /// The <see cref="GifTextData"/> on the right side of the operand.
         /// </param>
         /// <returns>
         /// True if the current left is equal to the <paramref name="right"/> parameter; otherwise, false.
         /// </returns>
-        public static bool operator ==(ImageProperty left, ImageProperty right)
+        public static bool operator ==(GifTextData left, GifTextData right)
         {
             return left.Equals(right);
         }
 
         /// <summary>
-        /// Compares two <see cref="ImageProperty"/> objects. The result specifies whether the values
-        /// of the <see cref="ImageProperty.Name"/> or <see cref="ImageProperty.Value"/> properties of the two
-        /// <see cref="ImageProperty"/> objects are unequal.
+        /// Compares two <see cref="GifTextData"/> objects. The result specifies whether the values
+        /// of the <see cref="GifTextData.Name"/> or <see cref="GifTextData.Value"/> properties of the two
+        /// <see cref="GifTextData"/> objects are unequal.
         /// </summary>
         /// <param name="left">
-        /// The <see cref="ImageProperty"/> on the left side of the operand.
+        /// The <see cref="GifTextData"/> on the left side of the operand.
         /// </param>
         /// <param name="right">
-        /// The <see cref="ImageProperty"/> on the right side of the operand.
+        /// The <see cref="GifTextData"/> on the right side of the operand.
         /// </param>
         /// <returns>
         /// True if the current left is unequal to the <paramref name="right"/> parameter; otherwise, false.
         /// </returns>
-        public static bool operator !=(ImageProperty left, ImageProperty right)
+        public static bool operator !=(GifTextData left, GifTextData right)
         {
             return !(left == right);
         }
@@ -90,7 +90,7 @@ public ImageProperty(string name, string value)
         /// </returns>
         public override bool Equals(object obj)
         {
-            return obj is ImageProperty other && this.Equals(other);
+            return obj is GifTextData other && this.Equals(other);
         }
 
         /// <summary>
@@ -116,9 +116,9 @@ public override bool Equals(object obj)
         /// True if the current object is equal to the <paramref name="other"/> parameter; otherwise, false.
         /// </returns>
         /// <param name="other">An object to compare with this object.</param>
-        public bool Equals(ImageProperty other)
+        public bool Equals(GifTextData other)
         {
             return this.Name.Equals(other.Name) && Equals(this.Value, other.Value);
         }
     }
-}
+}