From 96db3dd6a88b600e58828bc31b4264d334231a22 Mon Sep 17 00:00:00 2001 From: Hare Date: Wed, 30 Sep 2026 07:59:50 +0900 Subject: [PATCH] fix: honor Accept media parameters --- crates/wip-https/src/codec.rs | 61 ++++++++++++++++++++++++-------- crates/wip-https/tests/routes.rs | 41 +++++++++++++++------ 2 files changed, 77 insertions(+), 25 deletions(-) diff --git a/crates/wip-https/src/codec.rs b/crates/wip-https/src/codec.rs index d962fa7..bf216f2 100644 --- a/crates/wip-https/src/codec.rs +++ b/crates/wip-https/src/codec.rs @@ -53,6 +53,15 @@ fn require_response_content_type(response: &Response) -> Result<(), Client } } +fn is_utf8_charset(value: &str) -> bool { + let value = value.trim(); + value.eq_ignore_ascii_case("utf-8") + || value + .strip_prefix('"') + .and_then(|value| value.strip_suffix('"')) + .is_some_and(|value| value.eq_ignore_ascii_case("utf-8")) +} + /// Returns whether a Content-Type value denotes UTF-8 `application/json`. /// /// The charset parameter is optional because JSON exchanged by this binding is @@ -72,13 +81,7 @@ pub fn is_json_content_type(value: &str) -> bool { let Some((name, value)) = parameter.trim().split_once('=') else { return false; }; - let charset = value.trim(); - let charset_is_utf8 = charset.eq_ignore_ascii_case("utf-8") - || charset - .strip_prefix('"') - .and_then(|value| value.strip_suffix('"')) - .is_some_and(|value| value.eq_ignore_ascii_case("utf-8")); - if !name.trim().eq_ignore_ascii_case("charset") || !charset_is_utf8 || saw_charset { + if !name.trim().eq_ignore_ascii_case("charset") || !is_utf8_charset(value) || saw_charset { return false; } saw_charset = true; @@ -88,7 +91,7 @@ pub fn is_json_content_type(value: &str) -> bool { fn accepts_json(headers: &HeaderMap) -> bool { let mut present = false; - let mut best_match: Option<(u8, f32)> = None; + let mut best_match: Option<((u8, u8), f32)> = None; for value in headers.get_all(ACCEPT) { present = true; let Ok(value) = value.to_str() else { @@ -99,17 +102,30 @@ fn accepts_json(headers: &HeaderMap) -> bool { let media_range = parts.next().unwrap_or_default().trim(); let mut quality = 1.0_f32; let mut saw_quality = false; + let mut saw_charset = false; + let mut media_parameter_count = 0; let mut valid = true; for parameter in parts { - let Some((name, value)) = parameter.trim().split_once('=') else { + let parameter = parameter.trim(); + let (name, value) = parameter + .split_once('=') + .map_or((parameter, None), |(name, value)| { + (name.trim(), Some(value)) + }); + if name.is_empty() { valid = false; break; - }; - if name.trim().eq_ignore_ascii_case("q") { - if saw_quality { + } + if saw_quality { + // Parameters after q are Accept extensions and do not + // constrain the selected representation. + continue; + } + if name.eq_ignore_ascii_case("q") { + let Some(value) = value else { valid = false; break; - } + }; saw_quality = true; match value.trim().parse::() { Ok(value) if (0.0..=1.0).contains(&value) => quality = value, @@ -118,9 +134,23 @@ fn accepts_json(headers: &HeaderMap) -> bool { break; } } + } else { + let Some(value) = value else { + valid = false; + break; + }; + if !name.eq_ignore_ascii_case("charset") + || !is_utf8_charset(value) + || saw_charset + { + valid = false; + break; + } + saw_charset = true; + media_parameter_count += 1; } } - let specificity = if media_range.eq_ignore_ascii_case("application/json") { + let media_specificity = if media_range.eq_ignore_ascii_case("application/json") { Some(2) } else if media_range.eq_ignore_ascii_case("application/*") { Some(1) @@ -129,7 +159,8 @@ fn accepts_json(headers: &HeaderMap) -> bool { } else { None }; - if valid && let Some(specificity) = specificity { + if valid && let Some(media_specificity) = media_specificity { + let specificity = (media_specificity, media_parameter_count); match best_match { Some((best_specificity, _)) if best_specificity > specificity => {} Some((best_specificity, best_quality)) if best_specificity == specificity => { diff --git a/crates/wip-https/tests/routes.rs b/crates/wip-https/tests/routes.rs index aa2af1a..f13be47 100644 --- a/crates/wip-https/tests/routes.rs +++ b/crates/wip-https/tests/routes.rs @@ -73,16 +73,37 @@ fn inbound_http_metadata_enforces_post_route_and_json() { .unwrap(); assert!(validate_http_request(&endpoint, Route::Fetch, &wrong_route).is_err()); - let incompatible_accept = Request::builder() - .method(Method::POST) - .uri("/wip/v1/fetch") - .header("content-type", "application/json") - .header("accept", "text/plain, */*;q=1, application/json;q=0") - .body(Vec::::new()) - .unwrap(); - let error = validate_http_request(&endpoint, Route::Fetch, &incompatible_accept).unwrap_err(); - assert!(matches!(&error, RequestMetadataError::NotAcceptable)); - assert_eq!(error.status(), StatusCode::NOT_ACCEPTABLE); + for accept in [ + "text/plain, */*;q=1, application/json;q=0", + "application/json; charset=utf-16", + "application/json; profile=future", + ] { + let incompatible_accept = Request::builder() + .method(Method::POST) + .uri("/wip/v1/fetch") + .header("content-type", "application/json") + .header("accept", accept) + .body(Vec::::new()) + .unwrap(); + let error = + validate_http_request(&endpoint, Route::Fetch, &incompatible_accept).unwrap_err(); + assert!(matches!(&error, RequestMetadataError::NotAcceptable)); + assert_eq!(error.status(), StatusCode::NOT_ACCEPTABLE); + } + + for accept in [ + "application/json; charset=UTF-8", + "application/json; q=1; profile=accept-extension", + ] { + let compatible_accept = Request::builder() + .method(Method::POST) + .uri("/wip/v1/fetch") + .header("content-type", "application/json") + .header("accept", accept) + .body(Vec::::new()) + .unwrap(); + assert!(validate_http_request(&endpoint, Route::Fetch, &compatible_accept).is_ok()); + } let unsupported_media_type = Request::builder() .method(Method::POST)