Skip to content

Fix McpHeaderEncoder.DecodeValue throwing on the degenerate base64 wrapper - #1805

Merged
jeffhandley merged 1 commit into
modelcontextprotocol:mainfrom
latent-9:fix-doc-typos
Aug 13, 2026
Merged

Fix McpHeaderEncoder.DecodeValue throwing on the degenerate base64 wrapper#1805
jeffhandley merged 1 commit into
modelcontextprotocol:mainfrom
latent-9:fix-doc-typos

Conversation

@latent-9

@latent-9 latent-9 commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

McpHeaderEncoder.DecodeValue throws ArgumentOutOfRangeException on the input "=?base64?=" instead of returning it unchanged, which contradicts its own contract ("returns the original value" when the value is not a valid base64 wrapper) and its never-throw test suite.

The wrapper uses prefix "=?base64?" (ends with ?) and suffix "?=" (starts with ?), which overlap on the shared ?. So "=?base64?=" (length 10) satisfies both StartsWith(prefix) and EndsWith(suffix). The inner length is then computed as Length - Prefix.Length - Suffix.Length = 10 - 9 - 2 = -1, and Substring(9, -1) throws. Because that Substring is outside the try, the exception propagates.

This is reachable from a client-supplied header (for example Mcp-Name: =?base64?=): every character is a valid header character, so it passes validation and reaches DecodeValue, turning an intended graceful rejection into an unhandled exception.

Fix: guard the wrapper branch with a minimum-length check, so a value too short to hold any content falls through to the literal return (the same behavior as DecodeValue_MissingSuffix_ReturnsLiteralValue). Added a regression test.

@jeffhandley jeffhandley 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.

Thanks! Nice work.

@jeffhandley
jeffhandley enabled auto-merge (squash) August 13, 2026 06:27
@jeffhandley
jeffhandley merged commit bba45c4 into modelcontextprotocol:main Aug 13, 2026
10 checks passed
@jeffhandley jeffhandley mentioned this pull request Aug 13, 2026
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