Skip to content

Commit e685a31

Browse files
authored
Aligning http-1x checksum updates with checksum changes from #4200 (#4204)
## Motivation and Context <!--- Why is this change required? What problem does it solve? --> <!--- If it fixes an open issue, please link to the issue here --> There were some conflicts around checksum changes made in #4200 while merging main back into the `feature/http-1.x` branch. This PR reconciles those changes ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
1 parent 92fb5d8 commit e685a31

File tree

5 files changed

+73
-57
lines changed

5 files changed

+73
-57
lines changed

aws/rust-runtime/aws-runtime/src/content_encoding.rs

Lines changed: 46 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,17 @@ where
382382
tracing::trace!(
383383
"No more chunk data, writing CRLF + CHUNK_TERMINATOR to end the data, and the first trailer frame"
384384
);
385+
386+
// We exhausted the body data, now check if the length is correct
387+
if let Err(poll_stream_len_err) =
388+
http_1x_utils::check_for_stream_length_mismatch(
389+
*this.inner_body_bytes_read_so_far as u64,
390+
this.options.stream_length,
391+
)
392+
{
393+
return poll_stream_len_err;
394+
}
395+
385396
*this.state = AwsChunkedBodyState::WritingTrailers;
386397
let trailers = frame.trailers_ref();
387398

@@ -436,15 +447,19 @@ where
436447
Poll::Ready(None) => {
437448
let trailers = match *this.state {
438449
AwsChunkedBodyState::WritingChunk => {
439-
let actual_stream_length = *this.inner_body_bytes_read_so_far as u64;
440-
let expected_stream_length = this.options.stream_length;
441-
if actual_stream_length != expected_stream_length {
442-
let err = Box::new(AwsChunkedBodyError::StreamLengthMismatch {
443-
actual: actual_stream_length,
444-
expected: expected_stream_length,
445-
});
446-
return Poll::Ready(Some(Err(err)));
447-
};
450+
// We exhausted the body data, now check if the length is correct
451+
if let Err(poll_stream_len_err) =
452+
http_1x_utils::check_for_stream_length_mismatch(
453+
*this.inner_body_bytes_read_so_far as u64,
454+
this.options.stream_length,
455+
)
456+
{
457+
return poll_stream_len_err;
458+
}
459+
460+
// Since we exhausted the body data, but are still in the WritingChunk state we did
461+
// not poll any trailer frames and we write the CRLF + Chunk terminator to begin the
462+
// trailer section plus a single final CRLF to end the (empty) trailer section
448463
let mut trailers = BytesMut::with_capacity(7);
449464
trailers.extend_from_slice(
450465
&[CRLF_RAW, CHUNK_TERMINATOR_RAW, CRLF_RAW].concat(),
@@ -473,8 +488,10 @@ where
473488
}
474489
/// Utility functions to help with the [http_body_1x::Body] trait implementation
475490
mod http_1x_utils {
491+
use std::task::Poll;
492+
476493
use super::{CRLF_RAW, TRAILER_SEPARATOR};
477-
use bytes::BytesMut;
494+
use bytes::{Bytes, BytesMut};
478495
use http_1x::{HeaderMap, HeaderName};
479496

480497
/// Writes trailers out into a `string` and then converts that `String` to a `Bytes` before
@@ -531,6 +548,25 @@ mod http_1x_utils {
531548
None => 0,
532549
}
533550
}
551+
552+
/// This is an ugly return type, but in practice it just returns `Ok(())` if the values match
553+
/// and `Err(Poll::Ready(Some(Err(AwsChunkedBodyError::StreamLengthMismatch))))` if they don't
554+
#[allow(clippy::type_complexity)]
555+
pub(super) fn check_for_stream_length_mismatch(
556+
actual_stream_length: u64,
557+
expected_stream_length: u64,
558+
) -> Result<(), Poll<Option<Result<http_body_1x::Frame<Bytes>, aws_smithy_types::body::Error>>>>
559+
{
560+
if actual_stream_length != expected_stream_length {
561+
let err = Box::new(super::AwsChunkedBodyError::StreamLengthMismatch {
562+
actual: actual_stream_length,
563+
expected: expected_stream_length,
564+
});
565+
return Err(Poll::Ready(Some(Err(err))));
566+
};
567+
568+
Ok(())
569+
}
534570
}
535571

536572
/// Errors related to `AwsChunkedBody`

aws/sdk-codegen/src/main/kotlin/software/amazon/smithy/rustsdk/IntegrationTestDependencies.kt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Compani
2121
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.Hound
2222
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.Http1x
2323
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.HttpBody1x
24-
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.HttpBodyUtil
24+
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.HttpBodyUtil01x
2525
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.SerdeJson
2626
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.Smol
2727
import software.amazon.smithy.rust.codegen.core.rustlang.CargoDependency.Companion.TempFile
@@ -187,7 +187,7 @@ class S3TestDependencies(private val runtimeConfig: RuntimeConfig) : LibRsCustom
187187
addDependency(FuturesUtil.toDevDependency())
188188
addDependency(HdrHistogram)
189189
addDependency(HttpBody1x.toDevDependency().copy(optional = false))
190-
addDependency(HttpBodyUtil.toDevDependency().copy(optional = false))
190+
addDependency(HttpBodyUtil01x.toDevDependency().copy(optional = false))
191191
addDependency(Smol)
192192
addDependency(TempFile)
193193
addDependency(TracingAppender)

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/rustlang/CargoDependency.kt

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -370,8 +370,6 @@ data class CargoDependency(
370370
val HttpBodyUtil01x: CargoDependency =
371371
CargoDependency("http-body-util", CratesIo("0.1.3"))
372372

373-
val HttpBodyUtil: CargoDependency = CargoDependency("http-body-util", CratesIo("0.1.3"))
374-
375373
fun smithyAsync(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-async")
376374

377375
fun smithyCbor(runtimeConfig: RuntimeConfig) = runtimeConfig.smithyRuntimeCrate("smithy-cbor")

rust-runtime/aws-smithy-checksums/src/body/cache.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55

66
//! Checksum caching functionality.
77
8-
use http::HeaderMap;
8+
use http_1x::HeaderMap;
99
use std::sync::{Arc, Mutex};
1010

1111
/// A cache for storing previously calculated checksums.

rust-runtime/aws-smithy-checksums/src/body/calculate.rs

Lines changed: 24 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,28 @@ impl ChecksumBody<SdkBody> {
4949
cache: Some(cache),
5050
}
5151
}
52+
53+
// It would be nicer if this could take &self, but I couldn't make that
54+
// work out with the Pin/Projection types, so its a static method for now
55+
fn extract_or_set_cached_headers(
56+
maybe_cache: &Option<ChecksumCache>,
57+
checksum: Box<dyn HttpChecksum>,
58+
) -> http_1x::HeaderMap {
59+
let calculated_headers = checksum.headers();
60+
if let Some(cache) = maybe_cache {
61+
if let Some(cached_headers) = cache.get() {
62+
if cached_headers != calculated_headers {
63+
warn!(cached = ?cached_headers, calculated = ?calculated_headers, "calculated checksum differs from cached checksum!");
64+
}
65+
cached_headers
66+
} else {
67+
cache.set(calculated_headers.clone());
68+
calculated_headers
69+
}
70+
} else {
71+
calculated_headers
72+
}
73+
}
5274
}
5375

5476
impl http_body_1x::Body for ChecksumBody<SdkBody> {
@@ -72,7 +94,7 @@ impl http_body_1x::Body for ChecksumBody<SdkBody> {
7294
} else {
7395
// Add checksum trailer to other trailers if necessary
7496
let checksum_headers = if let Some(checksum) = this.checksum.take() {
75-
checksum.headers()
97+
ChecksumBody::extract_or_set_cached_headers(this.cache, checksum)
7698
} else {
7799
return Poll::Ready(None);
78100
};
@@ -91,7 +113,7 @@ impl http_body_1x::Body for ChecksumBody<SdkBody> {
91113
// trailers on the body) we write them here
92114
if !*this.written_trailers {
93115
let checksum_headers = if let Some(checksum) = this.checksum.take() {
94-
checksum.headers()
116+
ChecksumBody::extract_or_set_cached_headers(this.cache, checksum)
95117
} else {
96118
return Poll::Ready(None);
97119
};
@@ -105,46 +127,6 @@ impl http_body_1x::Body for ChecksumBody<SdkBody> {
105127
};
106128
poll_res
107129
}
108-
109-
fn poll_trailers(
110-
self: Pin<&mut Self>,
111-
cx: &mut Context<'_>,
112-
) -> Poll<Result<Option<HeaderMap>, Self::Error>> {
113-
let this = self.project();
114-
let poll_res = this.body.poll_trailers(cx);
115-
116-
if let Poll::Ready(Ok(maybe_inner_trailers)) = poll_res {
117-
let checksum_headers = if let Some(checksum) = this.checksum.take() {
118-
let calculated_headers = checksum.headers();
119-
120-
if let Some(cache) = this.cache {
121-
if let Some(cached_headers) = cache.get() {
122-
if cached_headers != calculated_headers {
123-
warn!(cached = ?cached_headers, calculated = ?calculated_headers, "calculated checksum differs from cached checksum!");
124-
}
125-
cached_headers
126-
} else {
127-
cache.set(calculated_headers.clone());
128-
calculated_headers
129-
}
130-
} else {
131-
calculated_headers
132-
}
133-
} else {
134-
return Poll::Ready(Ok(None));
135-
};
136-
137-
return match maybe_inner_trailers {
138-
Some(inner_trailers) => Poll::Ready(Ok(Some(append_merge_header_maps(
139-
inner_trailers,
140-
checksum_headers,
141-
)))),
142-
None => Poll::Ready(Ok(Some(checksum_headers))),
143-
};
144-
}
145-
146-
poll_res
147-
}
148130
}
149131

150132
#[cfg(test)]

0 commit comments

Comments
 (0)