From 8af9a8068e4b4f54a0a33438fc9937609f28ed79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Robert=20Mutnia=C5=84ski?= Date: Wed, 28 Feb 2024 11:01:01 +0100 Subject: [PATCH] PR comments changes --- .../Formats/Jpeg/JpegDecoderCore.cs | 11 ++-- .../Formats/Jpeg/JpegEncoderCore.cs | 53 ++++++++++++------- .../Formats/Jpg/JpegEncoderTests.Metadata.cs | 23 ++++++++ 3 files changed, 65 insertions(+), 22 deletions(-) diff --git a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs index e43c20a7d9..cf5e449e71 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs +++ b/src/ImageSharp/Formats/Jpeg/JpegDecoderCore.cs @@ -523,13 +523,16 @@ internal sealed class JpegDecoderCore : IRawJpegData, IImageDecoderInternals /// The remaining bytes in the segment block. private void ProcessComMarker(BufferedReadStream stream, int markerContentByteSize) { - Span temp = new byte[markerContentByteSize]; + char[] temp = new char[markerContentByteSize]; JpegMetadata metadata = this.Metadata.GetFormatMetadata(JpegFormat.Instance); - stream.Read(temp); - string comment = Encoding.ASCII.GetString(temp); + for (int i = 0; i < markerContentByteSize; i++) + { + int read = stream.ReadByte(); + temp[i] = (char)read; + } - metadata.Comments.Add(JpegComData.FromString(comment)); + metadata.Comments.Add(new JpegComData(temp)); } /// diff --git a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs b/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs index a55435c532..4ef4cea2d5 100644 --- a/src/ImageSharp/Formats/Jpeg/JpegEncoderCore.cs +++ b/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 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 currentComment = comment.Value; - if (comment.Length > maxCommentLength) + if (comment.Value.Length > maxCommentLength) { - string splitComment = comment.Substring(maxCommentLength, comment.Length - maxCommentLength); - metadata.Comments.Insert(i + 1, JpegComData.FromString(splitComment)); + ReadOnlyMemory 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 payload = new byte[totalPayloadLength]; + int currentCommentStartingIndex = 0; - Span commentSpan = new byte[commentLength]; - Span markers = commentSpan.Slice(0, 2); - Span payloadSize = commentSpan.Slice(2, 2); - Span payload = commentSpan.Slice(4, comment.Length); + for (int i = 0; i < comments.Count; i++) + { + ReadOnlyMemory comment = comments[i].Value; // Beginning of comment ff fe - markers[0] = JpegConstants.Markers.XFF; - markers[1] = JpegConstants.Markers.COM; + payload[currentCommentStartingIndex] = JpegConstants.Markers.XFF; + payload[currentCommentStartingIndex + 1] = JpegConstants.Markers.COM; // Write payload size - int comWithoutMarker = commentLength - 2; - payloadSize[0] = (byte)((comWithoutMarker >> 8) & 0xFF); - payloadSize[1] = (byte)(comWithoutMarker & 0xFF); + int comWithoutMarker = comment.Length + 2; + payload[currentCommentStartingIndex + 2] = (byte)((comWithoutMarker >> 8) & 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); } /// diff --git a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs b/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs index e49afedbb5..f0593b462c 100644 --- a/tests/ImageSharp.Tests/Formats/Jpg/JpegEncoderTests.Metadata.cs +++ b/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(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 output = Image.Load(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)]