Skip to content

Set Content-Length when the request body is set instead of via middleware - #694

Open
Tongs2000 wants to merge 2 commits into
mainfrom
feat-inline-content-length
Open

Set Content-Length when the request body is set instead of via middleware#694
Tongs2000 wants to merge 2 commits into
mainfrom
feat-inline-content-length

Conversation

@Tongs2000

@Tongs2000 Tongs2000 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Request.SetStream now sets ContentLength when the stream length can be determined, so the SDK no longer needs the standalone ComputeContentLength middleware. That middleware is kept but deprecated.

The length is set when the body is a known-length reader such as a bytes or strings reader or a seekable stream. When the length cannot be determined, for example a non-seekable stream, ContentLength is left unchanged so a user-provided value is preserved. Request compression updates the length after rewriting the body.

This is paired with the aws-sdk-go-v2 change (aws/aws-sdk-go-v2#3513) that stops registering ComputeContentLength and regenerates clients.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

…ware

Add Request.SetStreamWithLength, which sets ContentLength when the stream
length is known. The protocol serializers and request compression now use it,
so the SDK no longer needs the standalone ComputeContentLength middleware. That
middleware is kept but deprecated.
@lucix-aws

Copy link
Copy Markdown
Collaborator

Why the new API, why not just augment SetStream to do this? There already seems to be some logic that checks the stream length for something else there, even.

@Tongs2000

Copy link
Copy Markdown
Contributor Author

Why the new API, why not just augment SetStream to do this? There already seems to be some logic that checks the stream length for something else there, even.

Some middlewares in the Deserialize step also uses SetStream to restore the body after reading it. They don't compute/set ContentLength.

@lucix-aws

Copy link
Copy Markdown
Collaborator

Do you have an example?

@Tongs2000

Copy link
Copy Markdown
Contributor Author

There already seems to be some logic that checks the stream length for something else there, even.

Yes. Like the request/response logging middleware, in its HandleDeserialize:

smithyRequest, err = smithyRequest.SetStream(rc.Body)

@lucix-aws

Copy link
Copy Markdown
Collaborator

I'm not entirely sure why that extra SetStream call is there, but regardless, why would it be a problem to re-set content length there?

@Tongs2000

Tongs2000 commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

I'm not entirely sure why that extra SetStream call is there, but regardless, why would it be a problem to re-set content length there?

Double checked, you are right.

Besides RequestCompression, only two middlewares call SetStreamRequestResponseLogger in the Deserialize step and InitializeStreamWriter (eventstream) in the Finalize step.

For RequestResponseLogger it's restoring the body after DumpRequestOut reads it. For InitializeStreamWriter (eventstream), it swaps the body to an in-memory io.Pipe() so event stream messages can be written after the request is sent.

Both pass a body whose length can't be determined. So, moving the length logic into SetStream won't affect them as the length can't be determined we leave ContentLength unchanged.

Per review, SetStream now sets ContentLength when the stream length can be
determined, instead of adding a separate SetStreamWithLength method.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants