Fix handling of repeated headers in AWS request canonicalization (#2261)

CHANGELOG.next.toml

@@ -124,3 +124,11 @@ The rationale behind this change is that the previous design was tailored toward
 references = ["smithy-rs#2276"]
 meta = { "breaking" = true, "tada" = false, "bug" = true, "target" = "server"}
 author = "hlbarber"
+
+[[aws-sdk-rust]]
+message = """
+Fix request canonicalization for HTTP requests with repeated headers (for example S3's `GetObjectAttributes`). Previously requests with repeated headers would fail with a 403 signature mismatch due to this bug.
+"""
+references = ["smithy-rs#2261", "aws-sdk-rust#720"]
+meta = { "breaking" = false, "tada" = false, "bug" = true }
+author = "nipunn1313"

aws/rust-runtime/aws-sigv4/src/http_request/canonical_request.rs

@@ -13,7 +13,7 @@ use crate::http_request::url_escape::percent_encode_path;
 use crate::http_request::PercentEncodingMode;
 use crate::http_request::{PayloadChecksumKind, SignableBody, SignatureLocation, SigningParams};
 use crate::sign::sha256_hex_string;
-use http::header::{HeaderName, HOST};
+use http::header::{AsHeaderName, HeaderName, HOST};
 use http::{HeaderMap, HeaderValue, Method, Uri};
 use std::borrow::Cow;
 use std::cmp::Ordering;
@@ -225,7 +225,7 @@ impl<'a> CanonicalRequest<'a> {
  }
 
  let mut signed_headers = Vec::with_capacity(canonical_headers.len());
- for (name, _) in &canonical_headers {
+ for name in canonical_headers.keys() {
  if let Some(excluded_headers) = params.settings.excluded_headers.as_ref() {
  if excluded_headers.contains(name) {
  continue;
@@ -328,6 +328,19 @@ impl<'a> CanonicalRequest<'a> {
  canonical_headers.insert(x_amz_date, date_header.clone());
  date_header
  }
+
+ fn header_values_for(&self, key: impl AsHeaderName) -> String {
+ let values: Vec<&str> = self
+ .headers
+ .get_all(key)
+ .into_iter()
+ .map(|value| {
+ std::str::from_utf8(value.as_bytes())
+ .expect("SDK request header values are valid UTF-8")
+ })
+ .collect();
+ values.join(",")
+ }
 }
 
 impl<'a> fmt::Display for CanonicalRequest<'a> {
@@ -337,15 +350,8 @@ impl<'a> fmt::Display for CanonicalRequest<'a> {
  writeln!(f, "{}", self.params.as_deref().unwrap_or(""))?;
  // write out _all_ the headers
  for header in &self.values.signed_headers().headers {
- // a missing header is a bug, so we should panic.
- let value = &self.headers[&header.0];
  write!(f, "{}:", header.0.as_str())?;
- writeln!(
- f,
- "{}",
- std::str::from_utf8(value.as_bytes())
- .expect("SDK request header values are valid UTF-8")
- )?;
+ writeln!(f, "{}", self.header_values_for(&header.0))?;
  }
  writeln!(f)?;
  // write out the signed headers
@@ -538,6 +544,35 @@ mod tests {
  }
  }
 
+ #[test]
+ fn test_repeated_header() {
+ let mut req = test_request("get-vanilla-query-order-key-case");
+ req.headers_mut().append(
+ "x-amz-object-attributes",
+ HeaderValue::from_static("Checksum"),
+ );
+ req.headers_mut().append(
+ "x-amz-object-attributes",
+ HeaderValue::from_static("ObjectSize"),
+ );
+ let req = SignableRequest::from(&req);
+ let settings = SigningSettings {
+ payload_checksum_kind: PayloadChecksumKind::XAmzSha256,
+ ..Default::default()
+ };
+ let signing_params = signing_params(settings);
+ let creq = CanonicalRequest::from(&req, &signing_params).unwrap();
+
+ assert_eq!(
+ creq.values.signed_headers().to_string(),
+ "host;x-amz-content-sha256;x-amz-date;x-amz-object-attributes"
+ );
+ assert_eq!(
+ creq.header_values_for("x-amz-object-attributes"),
+ "Checksum,ObjectSize",
+ );
+ }
+
  #[test]
  fn test_set_xamz_sha_256() {
  let req = test_request("get-vanilla-query-order-key-case");