Skip to content

extproc: reverts the use of REPLACE_AND_CONTINUE#730

Merged
mathetake merged 10 commits intomainfrom
revertcontinue
Jun 27, 2025
Merged

extproc: reverts the use of REPLACE_AND_CONTINUE#730
mathetake merged 10 commits intomainfrom
revertcontinue

Conversation

@mathetake
Copy link
Member

@mathetake mathetake commented Jun 20, 2025

Description

This reverts 65ca02a. It was introduced to avoid sending request bodies twice to the extrpoc. However, the current implementation of extproc in Envoy side will ALWAYS remove content-length header hence result in forcing the chunked transfer-encoding. That causes some issue with some AI providers as described in #721 for example.

Related Issues/PRs (if applicable)

Reverts #636
Fixes #721

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@yuzisun yuzisun self-assigned this Jun 21, 2025
@mathetake mathetake marked this pull request as ready for review June 27, 2025 03:37
@mathetake mathetake requested a review from a team as a code owner June 27, 2025 03:37
Copy link
Contributor

@wengyao04 wengyao04 left a comment

Choose a reason for hiding this comment

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

lgtm, but some tests fail

@mathetake
Copy link
Member Author

oh yeah wierd...

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
@mathetake
Copy link
Member Author

ok passing

@wengyao04
Copy link
Contributor

wengyao04 commented Jun 27, 2025

Do you think we can revert this change back ? TO allow aws sign with content-lengh ?

https://github.com/envoyproxy/ai-gateway/blob/main/internal/extproc/backendauth/aws.go#L116C2-L116C24

@mathetake mathetake merged commit 1bed302 into main Jun 27, 2025
26 checks passed
@mathetake mathetake deleted the revertcontinue branch June 27, 2025 17:38
mathetake added a commit that referenced this pull request Jun 27, 2025
**Description**

Previously, we did not sign the content-length header due to a
quirkiness introduced in #721. Now that #721 was reverted in #730, the
content-length is properly set, so we can sign the content-length.

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
yuzisun added a commit to yuzisun/ai-gateway that referenced this pull request Jul 2, 2025
yuzisun added a commit to yuzisun/ai-gateway that referenced this pull request Jul 2, 2025
)"

This reverts commit 1bed302.

Signed-off-by: Dan Sun <dsun20@bloomberg.net>
mathetake added a commit that referenced this pull request Jul 2, 2025
**Description**

We use REPLACE_AND_CONTINUE in the request header phase on the upstream
extproc filter. However, the current extproc implementation removes the
content-length header unconditionally when body mutation happens with
REPLACE_AND_CONTINUE flag.

This adds a workaround of that limitation by inserting the header
mutation filter after the upstream extproc filter where we retrieve the
content-length from a dynamic metadata.

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

Fixes the issue that the #730 resolves in different way

---------

Signed-off-by: Takeshi Yoneda <t.y.mathetake@gmail.com>
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.

Content-length header being removed, force-switching the request to chunked transfer endcoding

3 participants