Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: expand use of chunked content encoding #1018

Closed
wants to merge 10 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changes/0b435166-944d-4836-8683-964034b823cc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"id": "0b435166-944d-4836-8683-964034b823cc",
"type": "feature",
"description": "Enable `aws-chunked` content encoding for streaming requests without a content length"
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ internal fun SdkBuffer.writeTrailerSignature(signature: String) {
*/
@InternalApi
public val HttpBody.isEligibleForAwsChunkedStreaming: Boolean
get() = (this is HttpBody.SourceContent || this is HttpBody.ChannelContent) && contentLength != null &&
(isOneShot || contentLength!! > AWS_CHUNKED_THRESHOLD)
get() = (this is HttpBody.SourceContent || this is HttpBody.ChannelContent) &&
(isOneShot || (contentLength?.compareTo(AWS_CHUNKED_THRESHOLD) ?: 0) > 0)
Comment on lines +46 to +47
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style: ?.compareTo works but is pretty verbose and still requires null-coalescing the value. Shorter would just be:

(contentLength ?: 0 > AWS_CHUNKED_THRESHOLD)

Same comment applies below.


/**
* @return a boolean representing if the signing configuration is configured (via [HashSpecification]) for aws-chunked content encoding
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -193,9 +193,6 @@ internal class DefaultCanonicalizer(private val sha256Supplier: HashSupplier = :
}
}

/** The number of bytes to read at a time during SHA256 calculation on streaming bodies. */
private const val STREAM_CHUNK_BYTES = 16384 // 16KB

/**
* Canonicalizes a path from this [Url.Builder].
* @param config The signing configuration to use
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ public class FlexibleChecksumsRequestInterceptor<I>(

// FIXME this duplicates the logic from aws-signing-common, but can't import from there due to circular import.
private val HttpBody.isEligibleForAwsChunkedStreaming: Boolean
get() = (this is HttpBody.SourceContent || this is HttpBody.ChannelContent) && contentLength != null &&
(isOneShot || contentLength!! > 65536 * 16)
get() = (this is HttpBody.SourceContent || this is HttpBody.ChannelContent) &&
(isOneShot || (contentLength?.compareTo(65_536 * 16) ?: 0) > 0)

/**
* @return if the [HashFunction] is supported by flexible checksums
Expand Down
Loading