Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 68 additions & 14 deletions crates/rmcp/src/transport/streamable_http_server/tower.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,59 @@ use crate::{
pub(crate) const DEFAULT_MAX_REQUEST_BODY_BYTES: usize = 4 * 1024 * 1024;
const STATELESS_STREAM_CHANNEL_CAPACITY: usize = 16;

// HTTP lists and parameter lists may contain delimiters in quoted strings.
// A quoted-pair keeps an escaped quote from ending the string.
fn split_unquoted(value: &str, delimiter: char) -> impl Iterator<Item = &str> {
let mut quoted = false;
let mut escaped = false;
value.split(move |character| {
if escaped {
escaped = false;
} else if quoted && character == '\\' {
escaped = true;
} else if character == '"' {
quoted = !quoted;
} else if !quoted && character == delimiter {
return true;
}
false
})
}

// Yield quality-filtered essences; callers apply MCP's explicit media-type policy.
fn accepted_media_types(headers: &HeaderMap) -> impl Iterator<Item = &str> {
headers
.get_all(http::header::ACCEPT)
.iter()
.filter_map(|value| value.to_str().ok())
.flat_map(|value| split_unquoted(value, ','))
.filter_map(|entry| {
let mut parts = split_unquoted(entry, ';');
let essence = parts.next()?.trim();
parts
.all(|parameter| {
let Some((name, value)) = parameter.split_once('=') else {
return true;
};
!name.trim().eq_ignore_ascii_case("q") || quality_is_acceptable(value.trim())
})
.then_some(essence)
})
}

// RFC 9110 section 12.4.2 permits up to three fractional digits.
fn quality_is_acceptable(quality: &str) -> bool {
let (whole, fraction) = quality.split_once('.').unwrap_or((quality, ""));
if fraction.len() > 3 || !fraction.bytes().all(|digit| digit.is_ascii_digit()) {
return false;
}
match whole {
"0" => fraction.bytes().any(|digit| digit != b'0'),
"1" => fraction.bytes().all(|digit| digit == b'0'),
_ => false,
}
}

struct ErrorResponse(Box<BoxResponse>);

impl ErrorResponse {
Expand Down Expand Up @@ -1847,11 +1900,8 @@ where
B::Error: Display,
{
// check accept header
if !request
.headers()
.get(http::header::ACCEPT)
.and_then(|header| header.to_str().ok())
.is_some_and(|header| header.contains(EVENT_STREAM_MIME_TYPE))
if !accepted_media_types(request.headers())
.any(|essence| essence.eq_ignore_ascii_case(EVENT_STREAM_MIME_TYPE))
{
return Ok(Response::builder()
.status(http::StatusCode::NOT_ACCEPTABLE)
Expand Down Expand Up @@ -1986,14 +2036,13 @@ where
B::Error: Display,
{
// check accept header
if !request
.headers()
.get(http::header::ACCEPT)
.and_then(|header| header.to_str().ok())
.is_some_and(|header| {
header.contains(JSON_MIME_TYPE) && header.contains(EVENT_STREAM_MIME_TYPE)
})
{
let mut accepts_json = false;
let mut accepts_sse = false;
for essence in accepted_media_types(request.headers()) {
accepts_json |= essence.eq_ignore_ascii_case(JSON_MIME_TYPE);
accepts_sse |= essence.eq_ignore_ascii_case(EVENT_STREAM_MIME_TYPE);
}
if !accepts_json || !accepts_sse {
return Ok(Response::builder()
.status(http::StatusCode::NOT_ACCEPTABLE)
.body(Full::new(Bytes::from("Not Acceptable: Client must accept both application/json and text/event-stream")).boxed())
Expand All @@ -2005,7 +2054,12 @@ where
.headers()
.get(http::header::CONTENT_TYPE)
.and_then(|header| header.to_str().ok())
.is_some_and(|header| header.starts_with(JSON_MIME_TYPE))
.is_some_and(|header| {
header
.split(';')
.next()
.is_some_and(|essence| essence.trim().eq_ignore_ascii_case(JSON_MIME_TYPE))
})
{
return Ok(Response::builder()
.status(http::StatusCode::UNSUPPORTED_MEDIA_TYPE)
Expand Down
228 changes: 227 additions & 1 deletion crates/rmcp/tests/test_streamable_http_standard_headers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,21 @@ async fn spawn_server() -> (reqwest::Client, String, CancellationToken) {
async fn spawn_server_with<S>(
service_factory: impl Fn() -> Result<S, std::io::Error> + Send + Sync + 'static,
) -> (reqwest::Client, String, CancellationToken)
where
S: ServerHandler + Send + 'static,
{
spawn_server_with_mode(service_factory, false).await
}

async fn spawn_server_with_mode<S>(
service_factory: impl Fn() -> Result<S, std::io::Error> + Send + Sync + 'static,
legacy_session_mode: bool,
) -> (reqwest::Client, String, CancellationToken)
where
S: ServerHandler + Send + 'static,
{
let config = StreamableHttpServerConfig::default()
.with_legacy_session_mode(false)
.with_legacy_session_mode(legacy_session_mode)
.with_json_response(true)
.with_sse_keep_alive(None)
.with_cancellation_token(CancellationToken::new());
Expand Down Expand Up @@ -495,3 +505,219 @@ async fn accepts_initialize_with_matching_mcp_method() -> anyhow::Result<()> {
ct.cancel();
Ok(())
}

#[tokio::test]
async fn media_type_headers_follow_http_semantics() {
let (client, url, ct) = spawn_server().await;
let cases: &[(&str, &[&str], &str, u16)] = &[
(
"ordinary",
&["application/json, text/event-stream"],
"application/json",
202,
),
(
"content type case",
&["application/json, text/event-stream"],
"Application/JSON",
202,
),
(
"accept case",
&["APPLICATION/JSON, TEXT/EVENT-STREAM"],
"application/json",
202,
),
(
"split accept",
&["application/json", "text/event-stream"],
"application/json",
202,
),
(
"content type suffix",
&["application/json, text/event-stream"],
"application/jsonx",
415,
),
(
"accept suffix",
&["application/json-seq, text/event-stream"],
"application/json",
406,
),
(
"zero quality",
&["application/json;q=0, text/event-stream"],
"application/json",
406,
),
(
"zero quality decimal",
&["application/json;Q=0.000, text/event-stream"],
"application/json",
406,
),
(
"positive quality",
&["application/json;q=0.001, text/event-stream"],
"application/json",
202,
),
Comment on lines +562 to +566

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The table covers the zero and positive boundary, but not the upper bound or malformed weights. So the "1" case and rejection paths in quality_is_acceptable are still untested. Could we add tests for those too, based on whatever behavior we settle on for malformed weights?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agreed—the current matrix misses the 1 branch and malformed-weight rejection. I’ll add the upper-bound and malformed cases here, including 1, 1., 1.000, over-precision, values above one, missing leading zero, and negative weights. Malformed weights will be tested against this implementation’s admission policy, without claiming full HTTP negotiation.

Thanks also to theamodhshetty for the supplemental matrix and independent macOS run. That covers this gap well; it has not been folded into the branch yet, and I’ll rerun the tests on the resulting commit.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Added in e0eb843.

The matrix now covers the upper bound and the rejection paths in quality_is_acceptable. Accepted: q=1, q=1., q=1.000, and text/event-stream;q=0.01. Rejected with 406: q=0.0001 (more than three fractional digits), q=1.001 (a fraction on a weight of one), q=.5 (no whole part), q=-0.1 (negative), q=abc (not a number), and text/event-stream;q=0 (zero weight on a required type).

To be explicit about the behaviour you asked to settle on: a qvalue that is not 0 or 1 with at most three fractional digits is treated as not acceptable, rather than being coerced to 0 or 1. That is this implementation's existing admission policy, not a claim about how a general HTTP stack should negotiate.

The GET-side cases got the same treatment: text/event-stream;q=1, q=1. and q=1.000 still reach the legacy session check (400), while q=1.001, q=0.0001, q=.5, q=-0.1 and q=abc are rejected (406).

Validation on this revision: all 67 tests across the six HTTP integration targets and 378 library tests passed, plus targeted Clippy with warnings denied and nightly formatting. Those are local checks; the upstream workflows on this head are waiting on maintainer approval.

(
"parameters",
&["application/json;profile=\"a,b;c\", text/event-stream"],
"application/json; charset=utf-8",
202,
),
(
"parameter is not media",
&["text/plain;profile=\"application/json, text/event-stream\""],
"application/json",
406,
),
(
"escaped quoted parameter",
&[r#"application/json;profile="a\",b;c", text/event-stream"#],
"application/json",
202,
),
(
"quoted quality is not a weight",
&[r#"application/json;profile="a;q=0", text/event-stream"#],
"application/json",
202,
),
(
"escaped parameter is not media",
&[r#"text/plain;profile="a\",application/json, text/event-stream""#],
"application/json",
406,
),
("wildcard alone", &["*/*"], "application/json", 406),
// Invalid weights follow this implementation's admission policy.
(
"one",
&["application/json;q=1, text/event-stream"],
"application/json",
202,
),
(
"one decimal",
&["application/json;q=1., text/event-stream"],
"application/json",
202,
),
(
"one three digits",
&["application/json;q=1.000, text/event-stream"],
"application/json",
202,
),
(
"over precision",
&["application/json;q=0.0001, text/event-stream"],
"application/json",
406,
),
(
"above one",
&["application/json;q=1.001, text/event-stream"],
"application/json",
406,
),
(
"missing leading zero",
&["application/json;q=.5, text/event-stream"],
"application/json",
406,
),
(
"negative quality",
&["application/json;q=-0.1, text/event-stream"],
"application/json",
406,
),
(
"non digit quality",
&["application/json;q=abc, text/event-stream"],
"application/json",
406,
),
(
"zero SSE",
&["application/json, text/event-stream;q=0"],
"application/json",
406,
),
(
"positive SSE",
&["application/json, text/event-stream;q=0.01"],
"application/json",
202,
),
(
"weighted split reversed",
&["text/event-stream;q=0.5", "application/json;q=0.5"],
"application/json",
202,
),
(
"same field reversed",
&["text/event-stream, application/json"],
"application/json",
202,
),
(
"SSE near match",
&["application/json, text/event-streaming"],
"application/json",
406,
),
];
let mut failures = Vec::new();
for (name, accept, content_type, expected) in cases {
let mut request = client
.post(&url)
.header("Content-Type", *content_type)
.body(r#"{"jsonrpc":"2.0","method":"notifications/initialized"}"#);
for value in *accept {
request = request.header("Accept", *value);
}
let actual = request.send().await.unwrap().status().as_u16();
if actual != *expected {
failures.push(format!("{name}: expected {expected}, got {actual}"));
}
}
ct.cancel();
// Legacy mode dispatches GET through media validation before requiring a session ID.
let (client, url, ct) = spawn_server_with_mode(|| Ok(HeaderValidationServer), true).await;
for (accept, expected) in [
("text/event-stream", 400),
("TEXT/EVENT-STREAM", 400),
("text/event-streamx", 406),
("text/event-stream;q=0.0", 406),
("text/event-stream;q=1", 400),
("text/event-stream;q=1.", 400),
("text/event-stream;q=1.000", 400),
("text/event-stream;q=1.001", 406),
("text/event-stream;q=0.0001", 406),
("text/event-stream;q=.5", 406),
("text/event-stream;q=-0.1", 406),
("text/event-stream;q=abc", 406),
] {
let actual = client
.get(&url)
.header("Accept", accept)
.send()
.await
.unwrap()
.status()
.as_u16();
if actual != expected {
failures.push(format!("GET {accept}: expected {expected}, got {actual}"));
}
}
ct.cancel();
assert!(failures.is_empty(), "{}", failures.join("\n"));
}
Loading