Unverified Commit 93fd642d authored by Russell Cohen's avatar Russell Cohen Committed by GitHub
Browse files

[bugfix] header trimming truncated length-1 headers (#845)

* [bugfix] header trimming truncated length-1 headers

* Update changelog
parent 758fc313
Loading
Loading
Loading
Loading
+1 −0
Original line number Diff line number Diff line
@@ -10,6 +10,7 @@ vNext (Month Day, Year)
- Fix epoch seconds date-time parsing bug in `aws-smithy-types` (smithy-rs#834)
- Omit trailing zeros from fraction when formatting HTTP dates in `aws-smithy-types` (smithy-rs#834)
- Model structs now have accessor methods for their members (smithy-rs#842)
- :bug: Fix bug that caused signing to fail for requests where the body length was <=9. (smithy-rs#845)

v0.0.23-alpha (November 3rd, 2021)
==================================
+14 −35
Original line number Diff line number Diff line
@@ -363,41 +363,10 @@ fn trim_all(text: &[u8]) -> Cow<'_, [u8]> {
/// Removes excess spaces before and after a given byte string by returning a subset of those bytes.
/// Will return an empty slice if a string is composed entirely of whitespace.
fn trim_spaces_from_byte_string(bytes: &[u8]) -> &[u8] {
    if bytes.is_empty() {
        return bytes;
    }

    let mut starting_index = 0;

    for i in 0..bytes.len() {
        // If we get to the end of the array without hitting a non-whitespace char, return empty slice
        if i == bytes.len() - 1 {
            // This range equates to an empty slice
            return &bytes[0..0];
        // otherwise, skip over each instance of whitespace
        } else if bytes[i] == b' ' {
            continue;
        }

        // return the index of the first non-whitespace character
        starting_index = i;
        break;
    }

    // Now we do the same but in reverse
    let mut ending_index = 0;
    for i in (0..bytes.len()).rev() {
        // skip over each instance of whitespace
        if bytes[i] == b' ' {
            continue;
        }

        // return the index of the first non-whitespace character
        ending_index = i;
        break;
    }

    &bytes[starting_index..=ending_index]
    let starting_index = bytes.iter().position(|b| *b != b' ').unwrap_or(0);
    let ending_offset = bytes.iter().rev().position(|b| *b != b' ').unwrap_or(0);
    let ending_index = bytes.len() - ending_offset;
    &bytes[starting_index..ending_index]
}

/// Works just like [trim_all] but acts on HeaderValues instead of bytes
@@ -757,6 +726,11 @@ mod tests {
        assert_eq!(expected, actual);
    }

    #[test]
    fn trim_spaces_works_on_single_characters() {
        assert_eq!(trim_all(b"2").as_ref(), b"2");
    }

    proptest! {
        #[test]
        fn test_trim_all_doesnt_elongate_strings(s in ".*") {
@@ -771,5 +745,10 @@ mod tests {
        fn test_normalize_header_value_doesnt_panic(v in (".*").prop_filter_map("Must be a valid HeaderValue", |v| http::HeaderValue::from_maybe_shared(v).ok())) {
            let _ = normalize_header_value(&v);
        }

        #[test]
        fn test_trim_all_does_nothing_when_there_are_no_spaces(s in "[^ ]*") {
            assert_eq!(trim_all(s.as_bytes()).as_ref(), s.as_bytes());
        }
    }
}
+1 −0
Original line number Diff line number Diff line
@@ -153,6 +153,7 @@ pub fn sign<'a>(
    request: SignableRequest<'a>,
    params: &'a SigningParams<'a>,
) -> Result<SigningOutput<SigningInstructions>, Error> {
    tracing::trace!(request = ?request, params = ?params, "signing request");
    match params.settings.signature_location {
        SignatureLocation::Headers => {
            let (signing_headers, signature) =
+2 −1
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ const NAUGHTY_STRINGS: &str = include_str!("blns/blns.txt");
// // A useful way to find leaks in the signing system that requires an actual S3 bucket to test with
// // If you want to use this, update the credentials to be your credentials and change the bucket name
// // to your bucket
// // NOTE: this won't actually succeed, you'll get a 400 back from S3 because the headers are too long.
// #[tokio::test]
// async fn test_metadata_field_against_naughty_strings_list() -> Result<(), aws_sdk_s3::Error> {
//     // re-add `aws-config = { path = "../../build/aws-sdk/aws-config" }` to this project's Cargo.toml
@@ -95,7 +96,7 @@ async fn test_s3_signer_with_naughty_string_metadata() -> Result<(), aws_sdk_s3:

    // This is a snapshot test taken from a known working test result
    let snapshot_signature =
        "Signature=849f8737d8e8239a349d74af5b2c1d24be43a199e591bd2fc9db7d8a62f49d71";
        "Signature=8dfa41f2db599a9fba53393b0ae5da646e5e452fa3685f7a1487d6eade5ec5c8";
    assert!(
        auth_header
            .to_str()