Skip to content

Fix old-spec raw headers - #1028

Merged
AArnott merged 2 commits into
mainfrom
copilot/fix-oldspec-raw-headers
Jul 31, 2026
Merged

AArnott merged 2 commits into
mainfrom
copilot/fix-oldspec-raw-headers

Conversation

@AArnott

@AArnott AArnott commented Jul 30, 2026

Copy link
Copy Markdown
Owner

Summary

  • Emit legacy-compatible raw16 headers for 32-255 byte binary and UTF-8 string payloads in OldSpec mode.
  • Reject extension formats in OldSpec mode because the legacy specification does not define them.
  • Add boundary and unsupported-format coverage.

Credit

Thanks to @mikegoodspeed for reporting the original MessagePack-CSharp issue (MessagePack-CSharp#2286).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 507e76c5-0c79-42e3-995a-4cb3995ab97e
Copilot AI review requested due to automatic review settings July 30, 2026 22:21
@AArnott
AArnott enabled auto-merge July 30, 2026 22:22

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates MessagePackWriter’s OldSpec behavior to match the legacy MessagePack “raw” encoding rules (avoiding str8) and to explicitly reject extension formats that the old spec does not define.

Changes:

  • In OldSpec mode, emit str16 (raw16) headers for 32–255 byte string payloads (and, via WriteBinHeader, for byte[] payloads encoded as strings).
  • Throw NotSupportedException when writing ExtensionHeader in OldSpec mode.
  • Add test coverage for boundary lengths and unsupported extension writing in OldSpec.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
test/Nerdbank.MessagePack.Tests/BuiltInConverterTests.cs Adds parameterized tests asserting legacy-compatible headers in OldSpec mode and verifies extensions are rejected.
src/Nerdbank.MessagePack/MessagePackWriter.cs Forces OldSpec string headers for 32–255 bytes to use Str16 and blocks extension header writes under OldSpec.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 507e76c5-0c79-42e3-995a-4cb3995ab97e
Copilot AI review requested due to automatic review settings July 31, 2026 01:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

src/Nerdbank.MessagePack/MessagePackWriter.cs:558

  • This XML doc comment says the msgpack code used in OldSpec mode is MinFixStr, but the actual header byte will vary across the fixstr range (MinFixStr..MaxFixStr). Referring to the whole range avoids implying a single constant header code.
	/// When <see cref="OldSpec"/> is <see langword="true"/>, the msgpack code used is <see cref="MessagePackCode.MinFixStr"/>, <see cref="MessagePackCode.Str16"/> or <see cref="MessagePackCode.Str32"/> instead.

src/Nerdbank.MessagePack/MessagePackWriter.cs:584

  • This XML doc comment says the msgpack code used in OldSpec mode is MinFixStr, but the actual header byte will vary across the fixstr range (MinFixStr..MaxFixStr). Referring to the whole range avoids implying a single constant header code.
	/// When <see cref="OldSpec"/> is <see langword="true"/>, the msgpack code used is <see cref="MessagePackCode.MinFixStr"/>, <see cref="MessagePackCode.Str16"/> or <see cref="MessagePackCode.Str32"/> instead.

src/Nerdbank.MessagePack/MessagePackWriter.cs:534

  • This XML doc comment says the msgpack code used in OldSpec mode is MinFixStr, but the actual header byte will vary across the fixstr range (MinFixStr..MaxFixStr). Referring to the whole range avoids implying a single constant header code.

This issue also appears in the following locations of the same file:

  • line 558
  • line 584
	/// When <see cref="OldSpec"/> is <see langword="true"/>, the msgpack code used is <see cref="MessagePackCode.MinFixStr"/>, <see cref="MessagePackCode.Str16"/> or <see cref="MessagePackCode.Str32"/> instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants