From 0f2ba4832bfdd5619b68bd88fcdd7d154c877c32 Mon Sep 17 00:00:00 2001 From: aton-of-data Date: Sun, 11 Oct 2026 13:22:04 +0000 Subject: [PATCH] fix(transport): keep 401/403 status when the body is a JSON-RPC error A 401 or 403 without a WWW-Authenticate header falls through to the non-success branch of the reqwest client's post_message, where a JSON-RPC error body was returned as an ordinary JSON response. Callers then saw a plain JSON-RPC error and could not tell that the server had rejected their credentials. Skip the JSON-RPC error branch for 401 and 403 so these responses end as UnexpectedServerResponse("HTTP 401 Unauthorized: ..."), the same as a plain-text body, and as legacy_discover_response already does. Fixes #1339 --- .../common/reqwest/streamable_http_client.rs | 14 +++++-- .../test_streamable_http_4xx_error_body.rs | 42 +++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/crates/rmcp/src/transport/common/reqwest/streamable_http_client.rs b/crates/rmcp/src/transport/common/reqwest/streamable_http_client.rs index 5ebe8fa68..0ec23acb1 100644 --- a/crates/rmcp/src/transport/common/reqwest/streamable_http_client.rs +++ b/crates/rmcp/src/transport/common/reqwest/streamable_http_client.rs @@ -287,9 +287,17 @@ impl StreamableHttpClient for reqwest::Client { { return Ok(response); } - if content_type - .as_deref() - .is_some_and(|ct| ct.as_bytes().starts_with(JSON_MIME_TYPE.as_bytes())) + // A 401/403 without a challenge (those with one returned above) + // must keep its status rather than pass as an ordinary JSON-RPC + // error, matching `legacy_discover_response`. + let is_auth_failure = matches!( + status, + reqwest::StatusCode::UNAUTHORIZED | reqwest::StatusCode::FORBIDDEN + ); + if !is_auth_failure + && content_type + .as_deref() + .is_some_and(|ct| ct.as_bytes().starts_with(JSON_MIME_TYPE.as_bytes())) { match parse_json_rpc_error(&body) { Some(message) => { diff --git a/crates/rmcp/tests/test_streamable_http_4xx_error_body.rs b/crates/rmcp/tests/test_streamable_http_4xx_error_body.rs index 13deeeb60..9f2df76c4 100644 --- a/crates/rmcp/tests/test_streamable_http_4xx_error_body.rs +++ b/crates/rmcp/tests/test_streamable_http_4xx_error_body.rs @@ -152,3 +152,45 @@ async fn sessionful_discover_4xx_does_not_trigger_legacy_fallback() { other => panic!("expected UnexpectedServerResponse, got: {other:?}"), } } + +const JSON_RPC_ERROR_BODY: &str = + r#"{"jsonrpc":"2.0","id":1,"error":{"code":-32001,"message":"Unauthorized"}}"#; + +async fn post_ping_expecting_status_error(status: u16, expected_prefix: &str) { + let url = spawn_mock_server(status, "application/json", JSON_RPC_ERROR_BODY).await; + + let client = reqwest::Client::new(); + let result = client + .post_message( + Arc::from(url.as_str()), + ping_message(), + None, + None, + HashMap::new(), + ) + .await; + + match result { + Err(StreamableHttpError::UnexpectedServerResponse(message)) => { + assert!( + message.starts_with(expected_prefix), + "expected message starting with {expected_prefix:?}, got: {message}" + ); + } + other => panic!("expected UnexpectedServerResponse, got: {other:?}"), + } +} + +/// A 401 without `WWW-Authenticate` must not be swallowed into a JSON-RPC +/// error just because the body happens to be one; the HTTP status has to +/// reach the caller, as it does for a plain-text 401. +#[tokio::test] +async fn http_401_json_rpc_error_body_without_challenge_keeps_status() { + post_ping_expecting_status_error(401, "HTTP 401 Unauthorized").await; +} + +/// Same as above for 403 without `WWW-Authenticate`. +#[tokio::test] +async fn http_403_json_rpc_error_body_without_challenge_keeps_status() { + post_ping_expecting_status_error(403, "HTTP 403 Forbidden").await; +}