Skip to content

extproc: do not sign content-length for AWS#694

Merged
mathetake merged 3 commits intomainfrom
thisisatest
Jun 10, 2025
Merged

extproc: do not sign content-length for AWS#694
mathetake merged 3 commits intomainfrom
thisisatest

Conversation

@mathetake
Copy link
Copy Markdown
Member

@mathetake mathetake commented Jun 9, 2025

Commit Message

Previously, AWS signing has included the "content-length" header. However, Envoy's extproc filter strips it from the request as we are using CONTINUE_AND_REPLACE option to reduce the memory overhead. While we have still no clue as to why AWS doesn't complain when the request body is small, excluding content-length from the signing target headers will make the tests with both small and large bodies pass.

Related Issues/PRs (if applicable)

CONTINUE_AND_REPLACE was introduced in #636 to avoid sending a request body twice between Envoy and the ExtProc.

mathetake added 2 commits June 9, 2025 14:23
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake mathetake marked this pull request as ready for review June 9, 2025 21:34
@mathetake mathetake requested a review from a team as a code owner June 9, 2025 21:34
// which should be acceptable for AWS Bedrock or any modern HTTP service.
//
// https://github.com/envoyproxy/envoy/blob/60b2b5187cf99db79ecfc54675354997af4765ea/source/extensions/filters/http/ext_proc/processor_state.cc#L180-L183
req.ContentLength = -1
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

still no idea why AWS doesn't complain when the body is small but at least this makes all tests happy

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

my best guess is that if there's only one chunk in the request (== small body), then AWS will fill in the content-length with the length of the only chunk and matches the signature. But we'll never know...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ye I think there is some special handling on the AWS side

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake mathetake merged commit d3c58bb into main Jun 10, 2025
17 checks passed
@mathetake mathetake deleted the thisisatest branch June 10, 2025 02:41
yuzisun pushed a commit to yuzisun/ai-gateway that referenced this pull request Jun 10, 2025
**Commit Message**

Previously, AWS signing has included the "content-length" header.
However, Envoy's extproc filter strips it from the request as we are
using CONTINUE_AND_REPLACE option to reduce the memory overhead. While
we have still no clue as to why AWS doesn't complain when the request
body is small, excluding content-length from the signing target headers
will make the tests with both small and large bodies pass.

**Related Issues/PRs (if applicable)**

CONTINUE_AND_REPLACE was introduced in envoyproxy#636 to avoid sending a request
body twice between Envoy and the ExtProc.

---------

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
yuzisun added a commit that referenced this pull request Jun 10, 2025
**Commit Message**

Previously, AWS signing has included the "content-length" header.
However, Envoy's extproc filter strips it from the request as we are
using CONTINUE_AND_REPLACE option to reduce the memory overhead. While
we have still no clue as to why AWS doesn't complain when the request
body is small, excluding content-length from the signing target headers
will make the tests with both small and large bodies pass.

**Related Issues/PRs (if applicable)**

CONTINUE_AND_REPLACE was introduced in #636 to avoid sending a request
body twice between Envoy and the ExtProc.

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Co-authored-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
yuzisun added a commit to yuzisun/ai-gateway that referenced this pull request Jul 2, 2025
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.

3 participants