mcp: relax application/json requirement#43500
mcp: relax application/json requirement#43500botengyao wants to merge 4 commits intoenvoyproxy:mainfrom
Conversation
Signed-off-by: Boteng Yao <boteng@google.com>
There was a problem hiding this comment.
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>
|
@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>
Relaxed the MCP filter POST Content-Type check from an exact match on
application/jsonto a prefix match, so thatapplication/json; charset=utf-8and 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:]