Skip to content

Commit 22ef52a

Browse files
authored
fix(rmcp): reject duplicate sep-2243 headers (#1274)
1 parent 26f3b2e commit 22ef52a

1 file changed

Lines changed: 74 additions & 8 deletions

File tree

‎crates/rmcp/src/transport/common/mcp_headers.rs‎

Lines changed: 74 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -268,8 +268,7 @@ pub(crate) fn validate_request_headers(
268268
};
269269
let params = request.get("params");
270270

271-
let header_method = header_str(headers, HEADER_MCP_METHOD);
272-
match header_method {
271+
match header_str(headers, HEADER_MCP_METHOD)? {
273272
None => return Err("missing required Mcp-Method header".to_owned()),
274273
Some(value) if value != method => {
275274
return Err(format!(
@@ -280,7 +279,7 @@ pub(crate) fn validate_request_headers(
280279
}
281280

282281
if let Some(expected) = extract_name(method, params) {
283-
match header_str(headers, HEADER_MCP_NAME) {
282+
match header_str(headers, HEADER_MCP_NAME)? {
284283
None => return Err(format!("missing required Mcp-Name header for `{method}`")),
285284
Some(raw) => {
286285
let decoded = decode_header_value(raw)
@@ -300,7 +299,7 @@ pub(crate) fn validate_request_headers(
300299
let arguments = params.and_then(|p| p.get("arguments"));
301300
for (prop, header) in param_header_annotations(schema) {
302301
let full = format!("{HEADER_MCP_PARAM_PREFIX}{header}");
303-
let header_value = header_str(headers, &full);
302+
let header_value = header_str(headers, &full)?;
304303
let arg = arguments.and_then(|a| a.get(&prop));
305304
let body_value = arg.filter(|v| !v.is_null()).and_then(primitive_to_string);
306305

@@ -329,10 +328,21 @@ pub(crate) fn validate_request_headers(
329328
Ok(())
330329
}
331330

332-
/// Case-insensitive header lookup returning the value as `&str`, if present and valid UTF-8.
331+
/// The sole value for `name` as `&str`, if present and valid UTF-8.
332+
///
333+
/// Errors when `name` appears more than once: these headers are singletons, and
334+
/// letting one through would let an intermediary that resolves duplicates
335+
/// differently route on a value this request never dispatches.
333336
#[cfg(feature = "server-side-http")]
334-
fn header_str<'a>(headers: &'a http::HeaderMap, name: &str) -> Option<&'a str> {
335-
headers.get(name).and_then(|value| value.to_str().ok())
337+
fn header_str<'a>(headers: &'a http::HeaderMap, name: &str) -> Result<Option<&'a str>, String> {
338+
let mut values = headers.get_all(name).iter();
339+
let Some(value) = values.next() else {
340+
return Ok(None);
341+
};
342+
if values.next().is_some() {
343+
return Err(format!("duplicate {name} header"));
344+
}
345+
Ok(value.to_str().ok())
336346
}
337347

338348
#[cfg(all(test, feature = "client-side-sse", feature = "server-side-http"))]
@@ -354,7 +364,8 @@ mod tests {
354364
fn header_map(pairs: &[(&str, &str)]) -> HeaderMap {
355365
let mut map = HeaderMap::new();
356366
for (name, value) in pairs {
357-
map.insert(
367+
// Append, not insert, so repeated names survive as duplicates.
368+
map.append(
358369
HeaderName::from_bytes(name.as_bytes()).unwrap(),
359370
HeaderValue::from_str(value).unwrap(),
360371
);
@@ -621,6 +632,26 @@ mod tests {
621632
assert!(validate_request_headers(&headers, &tools_call_request(), None).is_err());
622633
}
623634

635+
#[test]
636+
fn rejects_duplicate_method() {
637+
let headers = header_map(&[
638+
("Mcp-Method", "tools/call"),
639+
("Mcp-Method", "tools/list"),
640+
("Mcp-Name", "deploy"),
641+
]);
642+
assert!(validate_request_headers(&headers, &tools_call_request(), None).is_err());
643+
}
644+
645+
#[test]
646+
fn rejects_duplicate_name() {
647+
let headers = header_map(&[
648+
("Mcp-Method", "tools/call"),
649+
("Mcp-Name", "deploy"),
650+
("Mcp-Name", "other"),
651+
]);
652+
assert!(validate_request_headers(&headers, &tools_call_request(), None).is_err());
653+
}
654+
624655
#[test]
625656
fn accepts_matching_param() {
626657
let schema = schema_with(json!({
@@ -654,5 +685,40 @@ mod tests {
654685
]);
655686
assert!(validate_request_headers(&headers, &request, Some(&schema)).is_err());
656687
}
688+
689+
#[test]
690+
fn rejects_duplicate_param() {
691+
let schema = schema_with(json!({
692+
"region": { "type": "string", "x-mcp-header": "Region" },
693+
}));
694+
let request = json!({
695+
"jsonrpc": "2.0", "id": 1, "method": "tools/call",
696+
"params": { "name": "deploy", "arguments": { "region": "us-west1" } }
697+
});
698+
let headers = header_map(&[
699+
("Mcp-Method", "tools/call"),
700+
("Mcp-Name", "deploy"),
701+
("Mcp-Param-Region", "us-west1"),
702+
("Mcp-Param-Region", "eu-central1"),
703+
]);
704+
assert!(validate_request_headers(&headers, &request, Some(&schema)).is_err());
705+
}
706+
707+
#[test]
708+
fn rejects_duplicate_param_for_absent_argument() {
709+
let schema = schema_with(json!({
710+
"region": { "type": "string", "x-mcp-header": "Region" },
711+
}));
712+
let headers = header_map(&[
713+
("Mcp-Method", "tools/call"),
714+
("Mcp-Name", "deploy"),
715+
("Mcp-Param-Region", "us-west1"),
716+
("Mcp-Param-Region", "eu-central1"),
717+
]);
718+
assert_eq!(
719+
validate_request_headers(&headers, &tools_call_request(), Some(&schema)),
720+
Err("duplicate Mcp-Param-Region header".to_owned())
721+
);
722+
}
657723
}
658724
}

0 commit comments

Comments
 (0)