Fix: streamable-http server: a supplied Mcp-Method contradicting an initialize body is silently accepted - #1275
Conversation
|
Fixed the Code Formatting check failure: ran nightly |
|
AI-assisted source review of The new initialize check still has two edge cases in
Suggested additions inside this PR's #[test]
fn initialize_rejects_conflicting_duplicates_in_both_orders() {
for (first, second) in [("initialize", "tools/list"), ("tools/list", "initialize")] {
let mut headers = headers_with(Some(first));
headers.append(HEADER_MCP_METHOD, http::HeaderValue::from_str(second).unwrap());
assert!(validate_standard_headers(&headers, &initialize_message(), no_tool_schema).is_err());
}
}
#[test]
fn initialize_rejects_present_non_text_method() {
let mut headers = headers_with(None);
let value = http::HeaderValue::from_bytes(&[0xff]).unwrap();
assert!(value.to_str().is_err());
headers.insert(HEADER_MCP_METHOD, value);
assert!(validate_standard_headers(&headers, &initialize_message(), no_tool_schema).is_err());
}The original issue's approach provides a small fix: inspect References: Prepared and posted by an authorized AI assistant for the contributing account; not a claim of the account owner's unaided Rust implementation or testing. |
13c3fd5 to
2efb0b5
Compare
|
Addressed both edge cases in 2efb0b5:
No Rust toolchain in this environment, so the new tests were not executed locally; they need to run in CI. |
jarrettdustinqq
left a comment
There was a problem hiding this comment.
Native validation follow-up to #issuecomment-5733844405 and the author's correction in 2efb0b5a004ba9360df682432bd14ba9b1c3211c.
I compiled the unmodified pinned head on x86_64 Linux using the project's Rust 1.96.0 toolchain. No source fix or competing implementation was needed.
cargo +1.96.0 test -p rmcp --lib --features transport-streamable-http-server standard_header_init_tests -- --nocapturecompleted successfully: 5 passed, 0 failed, 0 ignored, 218 filtered out. This includes both suggested duplicate-order and non-text-header tests, plus the existing missing/matching/contradicting-header controls.- I then executed that exact Cargo-built native test binary without the filter (
--nocapture --test-threads=1): 223 passed, 0 failed, 0 ignored, 0 filtered out. The five targeted tests are included in those 223, not additional unique tests.
Build settings: CARGO_BUILD_JOBS=1, CARGO_PROFILE_DEV_DEBUG=0, CARGO_PROFILE_TEST_DEBUG=0, CARGO_INCREMENTAL=0; rustc 1.96.0 (ac68faa20 2026-05-25), cargo 1.96.0 (30a34c682 2026-05-25). The build emitted three dead_code warnings in existing feature-gated code.
Scope and preserved failures: these are the library tests enabled by default features plus transport-streamable-http-server, not all-feature, integration, conformance, or upstream CI results. A subsequent offline Cargo invocation failed after I removed temporary downloaded crate archives to recover disk space; the already-built native binary remained intact and supplied the full 223-test result above. A separate attempt to execute the pre-review function as a negative control was rejected by the remote command runner before execution, so no red/green replay is claimed. The checked-out source remained byte-for-byte unchanged. No deployed service was probed.
This verifies that the current native tests execute and pass on the corrected head; it is not a maintainer approval, merge recommendation based on full CI, or claim that the account owner independently implemented/tested Rust. Work was performed by an authorized AI assistant; original reporting and the patch remain credited to their respective contributors.
| let mut method_values = headers.get_all(HEADER_MCP_METHOD).iter(); | ||
| let Some(first) = method_values.next() else { | ||
| return Ok(()); | ||
| }; |
There was a problem hiding this comment.
Inside validate_standard_headers, handle_post bypasses the new check when legacy sessions are enabled and no session ID is provided. For example, an initialize body and protocol header that both specify 2026-07-28, along with Mcp-Method: tools/list, still return HTTP 200 and create a session. We should run the validator before that path creates the session. Can you also add an HTTP-level regression test?
2efb0b5 to
6710a3b
Compare
|
Addressed your review feedback: |
Signed-off-by: SIDDARTHA REDDY <75976672+SIDDARTHAREDDY8@users.noreply.github.com> Signed-off-by: SIDDARTHA REDDY <75976672+SIDDARTHAREDDY8@users.noreply.github.com>
6710a3b to
3197be7
Compare
|
Applied |
Fixes #1271.
Implemented the fix for modelcontextprotocol/rust-sdk issue #1271 in crates/rmcp/src/transport/streamable_http_server/tower.rs. In validate_standard_headers, the InitializeRequest arm no longer returns Ok(()) blindly: a supplied Mcp-Method header is now checked against the body method (initialize), tolerating absence but rejecting contradiction with a header_mismatch JSON-RPC error (HTTP 400), mirroring the existing validate_header_matches_init_body pattern used for MCP-Protocol-Version. Added HEADER_MCP_METHOD to the http_header imports, updated the function docstring, and added 3 unit tests in a new standard_header_init_tests module: header-contradicting (rejected), header-absent (accepted), header-matching (accepted). Pushed to SIDDARTHAREDDY8/rust-sdk branch oss-bhai-2026-09-15 (commit 508d989) against upstream base main, matching the repo CONTRIBUTING-required base. No Rust toolchain in this environment, so tests were not executed locally and must run in CI. PR-body note for the publisher: this change was developed with AI assistance; disclose that honestly (e.g. an Assisted-by trailer or PR-body statement) since maintainers here actively check AI-use disclosure.
Assisted-by: AI agent (Bhai), reviewed by maintainer of the contributing account.
Tests could not be run in this environment: No Rust toolchain exists in this environment, so cargo test could not be run; the 3 new unit tests must run in CI. Static verification was done instead (API signatures confirmed against upstream source, HEADER_MCP_METHOD import confirmed, brace/paren balance checked, pushed fork blob re-fetched and confirmed to contain the new initialize arm and all 3 tests).