Fix MessagePackReader for multi-segment strings (MessagePack-CSharp#430)

* Fix MessagePackReader for multi-segment strings * Add failing unit test to show the bug * Fix issues in ReadStringSlow in NS1.6 code path

Author: pranavkm

src/MessagePack/MessagePackReader.cs

@@ -779,8 +779,7 @@ private string ReadStringSlow(int byteLength)
  // We need to concatenate all the bytes together into one array.
  byte[] byteArray = ArrayPool<byte>.Shared.Rent(byteLength);
  ThrowInsufficientBufferUnless(this.reader.TryCopyTo(byteArray));
- this.reader.Advance(byteArray.Length);
- string value = StringEncoding.UTF8.GetString(byteArray);
+ string value = StringEncoding.UTF8.GetString(byteArray, 0, byteLength);
  ArrayPool<byte>.Shared.Return(byteArray);
  return value;
 #else
@@ -808,6 +807,7 @@ private string ReadStringSlow(int byteLength)
  }
  }
 #endif
+ this.reader.Advance(bytesRead);
  }
 
  string value = new string(charArray, 0, initializedChars);

src/MessagePack/SequenceReader.cs

@@ -438,7 +438,7 @@ public bool TryCopyTo(Span<T> destination)
 
  internal bool TryCopyMultisegment(Span<T> destination)
  {
- if (Remaining < destination.Length)
+ if (destination.Length < Remaining)
  return false;
 
  ReadOnlySpan<T> firstSpan = UnreadSpan;

tests/MessagePack.Internal.Tests/MessagePack.Internal.Tests.csproj

@@ -19,6 +19,7 @@
  <Compile Include="..\MessagePack.Tests\MessagePackBinaryTest.cs" />
  <Compile Include="..\MessagePack.Tests\MessagePackReaderTests.cs" />
  <Compile Include="..\MessagePack.Tests\MessagePackReaderTests.ReadInt.cs" />
+ <Compile Include="..\MessagePack.Tests\MessagePackReaderTests.ReadString.cs" />
  <Compile Include="..\MessagePack.Tests\Utils\ChainingAssertion.Xunit.cs" />
  </ItemGroup>
 </Project>

tests/MessagePack.Tests/MessagePackReaderTests.ReadString.cs

@@ -0,0 +1,68 @@
+using System;
+using System.Buffers;
+using Xunit;
+
+namespace MessagePack.Tests
+{
+ partial class MessagePackReaderTests
+ {
+ [Fact]
+ public void ReadString_HandlesSingleSegment()
+ {
+ var seq = BuildSequence(new[] {
+ (byte)(MessagePackCode.MinFixStr + 2),
+ (byte)'A', (byte)'B' });
+
+ var reader = new MessagePackReader(seq);
+ var result = reader.ReadString();
+ Assert.Equal("AB", result);
+ }
+
+ [Fact]
+ public void ReadString_HandlesMultipleSegments()
+ {
+ var seq = BuildSequence(
+ new[] { (byte)(MessagePackCode.MinFixStr + 2), (byte)'A' },
+ new[] { (byte)'B' });
+
+ var reader = new MessagePackReader(seq);
+ var result = reader.ReadString();
+ Assert.Equal("AB", result);
+ }
+
+ ReadOnlySequence<T> BuildSequence<T>(params T[][] segmentContents)
+ {
+ if (segmentContents.Length == 1)
+ {
+ return new ReadOnlySequence<T>(segmentContents[0].AsMemory());
+ }
+
+ var bufferSegment = new BufferSegment<T>(segmentContents[0].AsMemory());
+ BufferSegment<T> last = default;
+ for (var i = 1; i < segmentContents.Length; i++)
+ {
+ last = bufferSegment.Append(segmentContents[i]);
+ }
+
+ return new ReadOnlySequence<T>(bufferSegment, 0, last, last.Memory.Length);
+ }
+
+ internal class BufferSegment<T> : ReadOnlySequenceSegment<T>
+ {
+ public BufferSegment(ReadOnlyMemory<T> memory)
+ {
+ Memory = memory;
+ }
+
+ public BufferSegment<T> Append(ReadOnlyMemory<T> memory)
+ {
+ var segment = new BufferSegment<T>(memory)
+ {
+ RunningIndex = RunningIndex + Memory.Length
+ };
+ Next = segment;
+ return segment;
+ }
+ }
+ }
+}