From 1dd3839a9f3279845f5993f1cb9b517b725f192e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Fri, 5 Jun 2026 06:45:50 +0800 Subject: [PATCH] fix(signer): address post-merge review comments (#3216) * fix: address PR #3150 review comments - Restore get_host_addr as best-effort wrapper (String return type) - Replace bare expect("err") with descriptive messages in unsigned trailer - Simplify aws-chunked header construction - 20 signer + 96 io-core tests pass * fix(signer): preserve host fallback * fix(signer): avoid signer fallback panics --------- Co-authored-by: houseme --- ...st_signature_streaming_unsigned_trailer.rs | 40 ++++++++++++++-- crates/signer/src/utils.rs | 48 +++++++++++++------ 2 files changed, 70 insertions(+), 18 deletions(-) diff --git a/crates/signer/src/request_signature_streaming_unsigned_trailer.rs b/crates/signer/src/request_signature_streaming_unsigned_trailer.rs index cf1c71e54..b90d5a91e 100644 --- a/crates/signer/src/request_signature_streaming_unsigned_trailer.rs +++ b/crates/signer/src/request_signature_streaming_unsigned_trailer.rs @@ -25,15 +25,47 @@ pub fn streaming_unsigned_v4( ) -> request::Request { let headers = req.headers_mut(); - let chunked_value = HeaderValue::from_str(&["aws-chunked"].join(",")).expect("err"); + let chunked_value = HeaderValue::from_static("aws-chunked"); headers.insert(http::header::TRANSFER_ENCODING, chunked_value); - if !session_token.is_empty() { - headers.insert("X-Amz-Security-Token", HeaderValue::from_str(session_token).expect("err")); + if !session_token.is_empty() + && let Ok(token_value) = HeaderValue::from_str(session_token) + { + headers.insert("X-Amz-Security-Token", token_value); } let format = format_description!("[year]-[month]-[day]T[hour]:[minute]:[second].[subsecond]Z"); - headers.insert("X-Amz-Date", HeaderValue::from_str(&req_time.format(&format).unwrap()).expect("err")); + if let Ok(date) = req_time.format(&format) + && let Ok(date_value) = HeaderValue::from_str(&date) + { + headers.insert("X-Amz-Date", date_value); + } //req.content_length = 100； req } + +#[cfg(test)] +mod tests { + use super::streaming_unsigned_v4; + use http::request; + use s3s::Body; + use time::OffsetDateTime; + + #[test] + fn streaming_unsigned_v4_skips_invalid_session_token_header() { + let req = request::Request::builder() + .method(http::Method::GET) + .uri("https://bucket.example.com/object") + .body(Body::empty()) + .expect("request should build"); + + let req = streaming_unsigned_v4(req, "invalid\ntoken", 0, OffsetDateTime::UNIX_EPOCH); + + assert!(req.headers().get("X-Amz-Security-Token").is_none()); + assert_eq!( + req.headers().get(http::header::TRANSFER_ENCODING), + Some(&http::HeaderValue::from_static("aws-chunked")) + ); + assert!(req.headers().get("X-Amz-Date").is_some()); + } +} diff --git a/crates/signer/src/utils.rs b/crates/signer/src/utils.rs index 0141403e1..f877b3a51 100644 --- a/crates/signer/src/utils.rs +++ b/crates/signer/src/utils.rs @@ -26,14 +26,7 @@ pub enum HostAddrError { pub fn try_get_host_addr(req: &request::Request) -> Result { let host = req.headers().get("host"); - let uri = req.uri(); - let uri_host = uri.host().ok_or(HostAddrError::MissingUriHost)?; - - let req_host = if let Some(port) = uri.port() { - format!("{uri_host}:{port}") - } else { - uri_host.to_string() - }; + let req_host = uri_host_addr(req).ok_or(HostAddrError::MissingUriHost)?; if let Some(host) = host { let host = host.to_str().map_err(|_| HostAddrError::InvalidHostHeader)?; @@ -48,12 +41,24 @@ pub fn try_get_host_addr(req: &request::Request) -> Result) -> String { match try_get_host_addr(req) { Ok(host) => host, - Err(HostAddrError::MissingUriHost) => match req.headers().get("host").map(|host| host.to_str()) { - Some(Ok(host)) => host.to_string(), - Some(Err(_)) => panic!("failed to resolve request host: invalid UTF-8 header value for `host`"), - None => panic!("failed to resolve request host: request uri has no host"), - }, - Err(err) => panic!("failed to resolve request host: {err}"), + Err(HostAddrError::MissingUriHost) => req + .headers() + .get("host") + .and_then(|host| host.to_str().ok()) + .unwrap_or_default() + .to_string(), + Err(HostAddrError::InvalidHostHeader) => uri_host_addr(req).unwrap_or_default(), + } +} + +fn uri_host_addr(req: &request::Request) -> Option { + let uri = req.uri(); + let uri_host = uri.host()?; + + if let Some(port) = uri.port() { + Some(format!("{uri_host}:{port}")) + } else { + Some(uri_host.to_string()) } } @@ -132,6 +137,21 @@ mod tests { assert!(matches!(err, HostAddrError::InvalidHostHeader)); } + #[test] + fn get_host_addr_uses_uri_host_when_host_header_is_non_utf8() { + let mut req = request::Request::builder() + .method(http::Method::GET) + .uri("https://bucket.example.com:9443/object") + .body(Body::empty()) + .expect("request should build"); + req.headers_mut().insert( + "host", + HeaderValue::from_bytes(&[0xFF]).expect("invalid utf8 bytes should be accepted by HeaderValue"), + ); + + assert_eq!(get_host_addr(&req), "bucket.example.com:9443"); + } + #[test] fn try_get_host_addr_rejects_relative_uri_without_host() { let req = request::Request::builder()