Repository navigation
fix(transport): match reqwest response media types exactly - #1390
Draft
theamodhshetty wants to merge 1 commit into
Draft
theamodhshetty wants to merge 1 commit into
theamodhshetty wants to merge 1 commit into
Conversation
Contributor
|
@DaleSeo (assignment failed) is on triage rotation this week and is the reviewer for this one. Labels come from the PR labeler. Please review the change. Automated triage. Labels are a first pass and the assigned maintainer has the final say. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation and Context
The reqwest Streamable HTTP client rejects valid
Application/JSONandText/Event-Streamresponses, while admitting different types such asapplication/jsonxandtext/event-streaming. On a JSON 503 response, the case mismatch also loses the original JSON-RPC error.Fixes #1389. This is separate from #1382, which changes server request validation.
Use one private matcher for the complete response media-type essence across GET admission, POST JSON/SSE dispatch and JSON error classification. Compare type/subtype case-insensitively, preserve parameter tolerance, and reject suffixes and comma-joined essences. Parameter bytes are left opaque. The existing accepted media types and body-free 429 policy are unchanged. No API or dependency changes.
How Has This Been Tested?
On macOS arm64 with Rust 1.96.1:
2a401c4, with 10 passing controls. All pass with this fix.The wire tests exercise GET, POST JSON/SSE, parameters, absent headers, case variants, suffix/list rejection, JSON 503 preservation and 429 status handling. No live external service or credentials are used.
Breaking Changes
Previously misclassified response types such as
application/jsonxnow return the existing unexpected-content-type error on success responses. They are not JSON/SSE media types. Valid response types remain accepted, with added case-insensitive support.Types of changes
Additional context
Submitted as a draft under the one-open-ready-PR limit while #1387 is in review. Issue #1389 records the proposal; maintainer agreement on the scope is still pending. This patch is scoped to the reqwest backend; Unix-socket response handling is unchanged. Developed with Codex assistance. Local checks and hosted CI are reported separately.