mirror of
https://github.com/rustfs/rustfs.git
synced 2026-08-30 16:59:52 +00:00
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 <housemecn@gmail.com>
This commit is contained in:
@@ -25,15 +25,47 @@ pub fn streaming_unsigned_v4(
|
|||||||
) -> request::Request<Body> {
|
) -> request::Request<Body> {
|
||||||
let headers = req.headers_mut();
|
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);
|
headers.insert(http::header::TRANSFER_ENCODING, chunked_value);
|
||||||
if !session_token.is_empty() {
|
if !session_token.is_empty()
|
||||||
headers.insert("X-Amz-Security-Token", HeaderValue::from_str(session_token).expect("err"));
|
&& 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");
|
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.content_length = 100;
|
||||||
|
|
||||||
req
|
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());
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
+34
-14
@@ -26,14 +26,7 @@ pub enum HostAddrError {
|
|||||||
|
|
||||||
pub fn try_get_host_addr(req: &request::Request<Body>) -> Result<String, HostAddrError> {
|
pub fn try_get_host_addr(req: &request::Request<Body>) -> Result<String, HostAddrError> {
|
||||||
let host = req.headers().get("host");
|
let host = req.headers().get("host");
|
||||||
let uri = req.uri();
|
let req_host = uri_host_addr(req).ok_or(HostAddrError::MissingUriHost)?;
|
||||||
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()
|
|
||||||
};
|
|
||||||
|
|
||||||
if let Some(host) = host {
|
if let Some(host) = host {
|
||||||
let host = host.to_str().map_err(|_| HostAddrError::InvalidHostHeader)?;
|
let host = host.to_str().map_err(|_| HostAddrError::InvalidHostHeader)?;
|
||||||
@@ -48,12 +41,24 @@ pub fn try_get_host_addr(req: &request::Request<Body>) -> Result<String, HostAdd
|
|||||||
pub fn get_host_addr(req: &request::Request<Body>) -> String {
|
pub fn get_host_addr(req: &request::Request<Body>) -> String {
|
||||||
match try_get_host_addr(req) {
|
match try_get_host_addr(req) {
|
||||||
Ok(host) => host,
|
Ok(host) => host,
|
||||||
Err(HostAddrError::MissingUriHost) => match req.headers().get("host").map(|host| host.to_str()) {
|
Err(HostAddrError::MissingUriHost) => req
|
||||||
Some(Ok(host)) => host.to_string(),
|
.headers()
|
||||||
Some(Err(_)) => panic!("failed to resolve request host: invalid UTF-8 header value for `host`"),
|
.get("host")
|
||||||
None => panic!("failed to resolve request host: request uri has no host"),
|
.and_then(|host| host.to_str().ok())
|
||||||
},
|
.unwrap_or_default()
|
||||||
Err(err) => panic!("failed to resolve request host: {err}"),
|
.to_string(),
|
||||||
|
Err(HostAddrError::InvalidHostHeader) => uri_host_addr(req).unwrap_or_default(),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
fn uri_host_addr(req: &request::Request<Body>) -> Option<String> {
|
||||||
|
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));
|
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]
|
#[test]
|
||||||
fn try_get_host_addr_rejects_relative_uri_without_host() {
|
fn try_get_host_addr_rejects_relative_uri_without_host() {
|
||||||
let req = request::Request::builder()
|
let req = request::Request::builder()
|
||||||
|
|||||||
Reference in New Issue
Block a user