Sitelet https://github.com/MessagePack-CSharp/MessagePack-CSharp/pull/2287/files
Skip to content

Fix OldSpec byte[] encoding emitting unsupported str8 headers #2287

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AArnott merged 4 commits into from
Aug 4, 2026
Merged

Fix OldSpec byte[] encoding emitting unsupported str8 headers #2287

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
694c19a
Initial plan
Copilot Jul 31, 2026
d1bdf18
Initial plan
Copilot Jul 31, 2026
2f8e105
Fix OldSpec byte[] encoding to avoid str8 for lengths 32-255
Copilot Jul 31, 2026
7ec5a75
Simplify OldSpec str16 high byte to constant 0
Copilot Jul 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 22 additions & 3 deletions src/MessagePack/MessagePackWriter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,7 +454,7 @@ public void Write(byte[]? src)
/// </summary>
/// <param name="src">The span of bytes to write.</param>
/// <remarks>
/// When <see cref="OldSpec"/> is <see langword="true"/>, the msgpack code used is <see cref="MessagePackCode.Str8"/>, <see cref="MessagePackCode.Str16"/> or <see cref="MessagePackCode.Str32"/> instead.
/// When <see cref="OldSpec"/> is <see langword="true"/>, the msgpack code used is fixstr, <see cref="MessagePackCode.Str16"/> or <see cref="MessagePackCode.Str32"/> instead (never <see cref="MessagePackCode.Str8"/>, which is not defined in the old spec).
/// </remarks>
public void Write(scoped ReadOnlySpan<byte> src)
{
Expand All @@ -473,7 +473,7 @@ public void Write(scoped ReadOnlySpan<byte> src)
/// </summary>
/// <param name="src">The span of bytes to write.</param>
/// <remarks>
/// When <see cref="OldSpec"/> is <see langword="true"/>, the msgpack code used is <see cref="MessagePackCode.Str8"/>, <see cref="MessagePackCode.Str16"/> or <see cref="MessagePackCode.Str32"/> instead.
/// When <see cref="OldSpec"/> is <see langword="true"/>, the msgpack code used is fixstr, <see cref="MessagePackCode.Str16"/> or <see cref="MessagePackCode.Str32"/> instead (never <see cref="MessagePackCode.Str8"/>, which is not defined in the old spec).
/// </remarks>
public void Write(in ReadOnlySequence<byte> src)
{
Expand All @@ -498,7 +498,7 @@ public void Write(in ReadOnlySequence<byte> src)
/// Alternatively a single call to <see cref="Write(ReadOnlySpan{byte})"/> or <see cref="Write(in ReadOnlySequence{byte})"/> will take care of the header and content in one call.
/// </para>
/// <para>
/// When <see cref="OldSpec"/> is <see langword="true"/>, the msgpack code used is <see cref="MessagePackCode.Str8"/>, <see cref="MessagePackCode.Str16"/> or <see cref="MessagePackCode.Str32"/> instead.
/// When <see cref="OldSpec"/> is <see langword="true"/>, the msgpack code used is fixstr, <see cref="MessagePackCode.Str16"/> or <see cref="MessagePackCode.Str32"/> instead (never <see cref="MessagePackCode.Str8"/>, which is not defined in the old spec).
/// </para>
/// </remarks>
public void WriteBinHeader(int length)
Expand Down Expand Up @@ -559,15 +559,34 @@ public void WriteString(ReadOnlySpan<byte> utf8stringBytes)
/// </summary>
/// <param name="byteCount">The number of bytes in the string that will follow this header.</param>
/// <remarks>
/// <para>
/// The caller should use <see cref="WriteRaw(in ReadOnlySequence{byte})"/> or <see cref="WriteRaw(ReadOnlySpan{byte})"/>
/// after calling this method to actually write the content.
/// Alternatively a single call to <see cref="WriteString(ReadOnlySpan{byte})"/> or <see cref="WriteString(in ReadOnlySequence{byte})"/> will take care of the header and content in one call.
/// </para>
/// <para>
/// When <see cref="OldSpec"/> is <see langword="true"/>, <see cref="MessagePackCode.Str8"/> is never used because it is not defined in the old spec;
/// lengths that would otherwise use str8 are encoded with <see cref="MessagePackCode.Str16"/> instead.
/// </para>
/// </remarks>
public void WriteStringHeader(int byteCount)
{
// When we write the header, we'll ask for all the space we need for the payload as well
// as that may help ensure we only allocate a buffer once.
Span<byte> span = this.writer.GetSpan(byteCount + 5);

// The old msgpack spec does not define str8 (0xd9). Use str16 for lengths 32-255 when OldSpec is set.
// This matches WriteString_PostEncoding and keeps binary-as-string (WriteBinHeader) legacy-compatible.
if (this.OldSpec && byteCount > MessagePackRange.MaxFixStringLength && byteCount <= byte.MaxValue)
{
span[0] = MessagePackCode.Str16;
span[1] = 0;
span[2] = unchecked((byte)byteCount);

this.writer.Advance(3);
return;
}

AssumesTrue(MessagePackPrimitives.TryWriteStringHeader(span, (uint)byteCount, out int written));
this.writer.Advance(written);
}
Expand Down
64 changes: 64 additions & 0 deletions tests/MessagePack.Tests/MessagePackWriterTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,70 @@ public void WriteBinHeader()
Assert.Equal(new byte[] { 1, 2, 3, 4, 5 }, reader.ReadBytes().Value.ToArray());
}

/// <summary>
/// Regression for https://github.com/MessagePack-CSharp/MessagePack-CSharp/issues/2286:
/// OldSpec must not emit str8 (0xD9) for lengths 32-255.
/// </summary>
[Theory]
[InlineData(31)] // fixstr
[InlineData(32)] // str16 under OldSpec (would be str8 otherwise)
[InlineData(255)] // str16 under OldSpec (would be str8 otherwise)
[InlineData(256)] // str16
public void Write_ByteArray_OldSpec_AvoidsStr8(int length)
{
byte[] value = new byte[length];
Array.Fill(value, (byte)'A');

var sequence = new Sequence<byte>();
var writer = new MessagePackWriter(sequence) { OldSpec = true };
writer.Write(value);
writer.Flush();

ReadOnlySpan<byte> written = sequence.AsReadOnlySequence.ToArray();
Assert.NotEqual(MessagePackCode.Str8, written[0]);

if (length <= MessagePackRange.MaxFixStringLength)
{
Assert.Equal((byte)(MessagePackCode.MinFixStr | length), written[0]);
Assert.Equal(value, written.Slice(1).ToArray());
}
else if (length <= ushort.MaxValue)
{
Assert.Equal(MessagePackCode.Str16, written[0]);
Assert.Equal((byte)(length >> 8), written[1]);
Assert.Equal((byte)length, written[2]);
Assert.Equal(value, written.Slice(3).ToArray());
}
}

[Theory]
[InlineData(32)]
[InlineData(255)]
public void WriteStringHeader_OldSpec_AvoidsStr8(int length)
{
var sequence = new Sequence<byte>();
var writer = new MessagePackWriter(sequence) { OldSpec = true };
writer.WriteStringHeader(length);
writer.Flush();

ReadOnlySpan<byte> written = sequence.AsReadOnlySequence.ToArray();
Assert.Equal(new byte[] { MessagePackCode.Str16, (byte)(length >> 8), (byte)length }, written.ToArray());
}

[Theory]
[InlineData(32)]
[InlineData(255)]
public void WriteBinHeader_OldSpec_AvoidsStr8(int length)
{
var sequence = new Sequence<byte>();
var writer = new MessagePackWriter(sequence) { OldSpec = true };
writer.WriteBinHeader(length);
writer.Flush();

ReadOnlySpan<byte> written = sequence.AsReadOnlySequence.ToArray();
Assert.Equal(new byte[] { MessagePackCode.Str16, (byte)(length >> 8), (byte)length }, written.ToArray());
}

[Fact]
public void WriteExtensionFormatHeader_NegativeExtension()
{
Expand Down
8 changes: 8 additions & 0 deletions tests/MessagePack.Tests/OldSpecBinaryFormatterTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ public class OldSpecBinaryFormatterTest
{
[Theory]
[InlineData(10)] // fixstr
[InlineData(32)] // str 16 (must not use str8 under OldSpec)
[InlineData(255)] // str 16 (must not use str8 under OldSpec)
[InlineData(1000)] // str 16
[InlineData(100000)] // str 32
public void SerializeSimpleByteArray(int arrayLength)
Expand Down Expand Up @@ -57,6 +59,8 @@ public void SerializeNil()

[Theory]
[InlineData(10)] // fixstr
[InlineData(32)] // str 16 (must not use str8 under OldSpec)
[InlineData(255)] // str 16 (must not use str8 under OldSpec)
[InlineData(1000)] // str 16
[InlineData(100000)] // str 32
public void SerializeObject(int arrayLength)
Expand All @@ -78,6 +82,8 @@ public void SerializeObject(int arrayLength)

[Theory]
[InlineData(10)] // fixstr
[InlineData(32)] // str 16 (must not use str8 under OldSpec)
[InlineData(255)] // str 16 (must not use str8 under OldSpec)
[InlineData(1000)] // str 16
[InlineData(100000)] // str 32
public void DeserializeSimpleByteArray(int arrayLength)
Expand All @@ -103,6 +109,8 @@ public void DeserializeNil()

[Theory]
[InlineData(10)] // fixstr
[InlineData(32)] // str 16 (must not use str8 under OldSpec)
[InlineData(255)] // str 16 (must not use str8 under OldSpec)
[InlineData(1000)] // str 16
[InlineData(100000)] // str 32
public void DeserializeObject(int arrayLength)
Expand Down
Loading