Skip to content

mcp: relax application/json requirement#43500

Open
botengyao wants to merge 5 commits intoenvoyproxy:mainfrom
botengyao:mcp-application-json
Open

mcp: relax application/json requirement#43500
botengyao wants to merge 5 commits intoenvoyproxy:mainfrom
botengyao:mcp-application-json

Conversation

@botengyao
Copy link
Member

Relaxed the MCP filter POST Content-Type check from an exact match on application/json to a prefix match, so that application/json; charset=utf-8 and similar media-type parameters are accepted.

Commit Message:
Additional Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

Signed-off-by: Boteng Yao <boteng@google.com>
agrawroh
agrawroh previously approved these changes Feb 15, 2026
Copy link
Contributor

@wdauchy wdauchy left a comment

Choose a reason for hiding this comment

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

lgtm, no blocker. a couple of nits/suggestions below.

absl::StartsWith(content_type, "application/json") would also match application/json-patch+json (RFC 6902), application/json-seq (RFC 7464), application/jsonl, etc. -- these are valid IANA media types that are not plain JSON. there's already a good pattern for this in the codebase: hasGrpcContentType in source/common/grpc/common.cc handles exactly this for application/grpc vs application/grpc-web by checking the character after the prefix. consider the same approach here: after the application/json prefix, accept only end-of-string, ;, or whitespace (per RFC 7231 media-type syntax). something like:

const absl::string_view content_type = headers.getContentTypeValue();
const auto& json_ct = Http::Headers::get().ContentTypeValues.Json;
bool is_json_content_type =
    absl::StartsWith(content_type, json_ct) &&
    (content_type.size() == json_ct.size() ||
     content_type[json_ct.size()] == ';' ||
     content_type[json_ct.size()] == ' ');

also consider adding negative test cases for media types that share the application/json prefix but aren't JSON (application/json-patch+json, application/jsonl), and a case with no space after the semicolon (application/json;charset=utf-8).

Signed-off-by: Boteng Yao <boteng@google.com>
@botengyao
Copy link
Member Author

@wdauchy, great feedback, I do think we need to update them in this PR, updating, and PTAL.

Signed-off-by: Boteng Yao <boteng@google.com>
@botengyao botengyao changed the title mcp: relax application/json requirement with start with mcp: relax application/json requirement Feb 17, 2026
Signed-off-by: Boteng Yao <boteng@google.com>
Copy link
Contributor

@wdauchy wdauchy left a comment

Choose a reason for hiding this comment

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

feedback addressed, looks good. thanks for the quick turnaround.

agrawroh
agrawroh previously approved these changes Feb 17, 2026
Signed-off-by: Boteng Yao <boteng@google.com>
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.

3 participants

Comments