PR comments changes

src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs

@@ -523,13 +523,16 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals
     /// <param name="markerContentByteSize">The remaining bytes in the segment block.</param>
     private void ProcessComMarker(BufferedReadStream stream, int markerContentByteSize)
     {
-        Span<byte> temp = new byte[markerContentByteSize];
+        char[] temp = new char[markerContentByteSize];
         JpegMetadata metadata = this.Metadata.GetFormatMetadata(JpegFormat.Instance);
 
-        stream.Read(temp);
+        for (int i = 0; i < markerContentByteSize; i++)
-        string comment = Encoding.ASCII.GetString(temp);
+        {
+            int read = stream.ReadByte();
+            temp[i] = (char)read;
+        }
 
-        metadata.Comments.Add(JpegComData.FromString(comment));
+        metadata.Comments.Add(new JpegComData(temp));
     }
 
     /// <summary>

src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs

@@ -3,6 +3,7 @@
 #nullable disable
 
 using System.Buffers.Binary;
+using System.Collections;
 using System.Text;
 using SixLabors.ImageSharp.Common.Helpers;
 using SixLabors.ImageSharp.Formats.Jpeg.Components;
@@ -184,39 +185,55 @@ internal sealed unsafe partial class JpegEncoderCore : IImageEncoderInternals
             return;
         }
 
-        for (int i = 0; i < metadata.Comments.Count; i++)
+        // We don't want to modify original metadata
+        List<JpegComData> comments = new(metadata.Comments);
+
+        int totalPayloadLength = 0;
+        for (int i = 0; i < comments.Count; i++)
         {
-            string comment = metadata.Comments[i].ToString();
+            JpegComData comment = comments[i];
+            ReadOnlyMemory<char> currentComment = comment.Value;
 
-            if (comment.Length > maxCommentLength)
+            if (comment.Value.Length > maxCommentLength)
             {
-                string splitComment = comment.Substring(maxCommentLength, comment.Length - maxCommentLength);
+                ReadOnlyMemory<char> splitComment =
-                metadata.Comments.Insert(i + 1, JpegComData.FromString(splitComment));
+                    currentComment.Slice(maxCommentLength, currentComment.Length - maxCommentLength);
+                comments.Insert(i + 1, new JpegComData(splitComment));
 
                 // We don't want to keep the extra bytes
-                comment = comment.Substring(0, maxCommentLength);
+                comments[i] = new JpegComData(currentComment.Slice(0, maxCommentLength));
             }
 
-            int commentLength = comment.Length + 4;
+            totalPayloadLength += comment.Value.Length + 4;
+        }
+
+        Span<byte> payload = new byte[totalPayloadLength];
+        int currentCommentStartingIndex = 0;
 
-            Span<byte> commentSpan = new byte[commentLength];
+        for (int i = 0; i < comments.Count; i++)
-            Span<byte> markers = commentSpan.Slice(0, 2);
+        {
-            Span<byte> payloadSize = commentSpan.Slice(2, 2);
+            ReadOnlyMemory<char> comment = comments[i].Value;
-            Span<byte> payload = commentSpan.Slice(4, comment.Length);
 
             // Beginning of comment ff fe
-            markers[0] = JpegConstants.Markers.XFF;
+            payload[currentCommentStartingIndex] = JpegConstants.Markers.XFF;
-            markers[1] = JpegConstants.Markers.COM;
+            payload[currentCommentStartingIndex + 1] = JpegConstants.Markers.COM;
 
             // Write payload size
-            int comWithoutMarker = commentLength - 2;
+            int comWithoutMarker = comment.Length + 2;
-            payloadSize[0] = (byte)((comWithoutMarker >> 8) & 0xFF);
+            payload[currentCommentStartingIndex + 2] = (byte)((comWithoutMarker >> 8) & 0xFF);
-            payloadSize[1] = (byte)(comWithoutMarker & 0xFF);
+            payload[currentCommentStartingIndex + 3] = (byte)(comWithoutMarker & 0xFF);
 
-            Encoding.ASCII.GetBytes(comment, payload);
+            char[] commentChars = comment.ToArray();
+            for (int j = 0; j < commentChars.Length; j++)
+            {
+                // Initial 4 bytes are always reserved
+                payload[4 + currentCommentStartingIndex + j] = (byte)commentChars[j];
+            }
 
-            this.outputStream.Write(commentSpan, 0, commentSpan.Length);
+            currentCommentStartingIndex += comment.Length + 4;
         }
+
+        this.outputStream.Write(payload, 0, payload.Length);
     }
 
     /// <summary>

tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs

@@ -198,6 +198,29 @@ public partial class JpegEncoderTests
         Assert.Equal(meta.Comments.ElementAtOrDefault(1).ToString(), actual.Comments.ElementAtOrDefault(1).ToString());
     }
 
+    [Fact]
+    public void Encode_SaveTooLongComment()
+    {
+        // arrange
+        string longString = new('c', 65534);
+        using var input = new Image<Rgba32>(1, 1);
+        JpegMetadata meta = input.Metadata.GetJpegMetadata();
+        using var memStream = new MemoryStream();
+
+        // act
+        meta.Comments.Add(JpegComData.FromString(longString));
+        input.Save(memStream, JpegEncoder);
+
+        // assert
+        memStream.Position = 0;
+        using Image<Rgba32> output = Image.Load<Rgba32>(memStream);
+        JpegMetadata actual = output.Metadata.GetJpegMetadata();
+        Assert.NotEmpty(actual.Comments);
+        Assert.Equal(2, actual.Comments.Count);
+        Assert.Equal(longString[..65533], actual.Comments.ElementAtOrDefault(0).ToString());
+        Assert.Equal("c", actual.Comments.ElementAtOrDefault(1).ToString());
+    }
+
     [Theory]
     [WithFile(TestImages.Jpeg.Baseline.Floorplan, PixelTypes.Rgb24, JpegEncodingColor.Luminance)]
     [WithFile(TestImages.Jpeg.Baseline.Jpeg444, PixelTypes.Rgb24, JpegEncodingColor.YCbCrRatio444)]