Fix: Set content-length on ext_proc body mutations #821
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| package extproc | ||
|
|
||
| import ( | ||
| "context" | ||
| "net/http" | ||
| "strconv" | ||
| "testing" | ||
|
|
||
| "github.com/rossoctl/cortex/authbridge/authlib/pipeline" | ||
| ) | ||
|
|
||
| // Envoy's ext_proc contract for the mode this listener runs (BUFFERED body, | ||
| // SEND headers), from api/envoy/extensions/filters/http/ext_proc/v3/ | ||
| // processing_mode.proto, BodySendMode (v1.37.1): | ||
| // | ||
| // In BUFFERED mode with SEND header mode, content length header is | ||
| // allowed but it is external processor's responsibility to set the | ||
| // content length correctly matched to the length of mutated body. | ||
| // | ||
| // These tests pin our side of that contract: a body-mutation reply carries | ||
| // content-length equal to the mutated body. They do not exercise Envoy. | ||
| // Envoy's side is pinned by its own integration test at the same tag, | ||
| // test/extensions/filters/http/ext_proc/ext_proc_integration_test.cc | ||
| // MismatchedContentLengthAndBodyLength: BUFFERED + SEND, the processor | ||
| // replaces "Replace this!" with "Hello, World!" and sets content-length to | ||
| // the wrong value → the upstream is never reached, downstream gets 500. The | ||
| // original body below is the same; the replacement is deliberately longer, | ||
| // so a reply that merely echoed the original length would fail too. | ||
|
|
||
| func TestWithBodyMutation_SetsContentLength(t *testing.T) { | ||
| pctx := &pipeline.Context{Body: []byte("Replace this!")} | ||
| pctx.SetBody([]byte("Hello, World! (longer)")) | ||
|
|
||
| hm := withBodyMutation(passBodyResponse(), pctx).GetRequestBody().GetResponse().GetHeaderMutation() | ||
| if got, want := mutationHeaderValue(hm, "content-length"), strconv.Itoa(len(pctx.Body)); got != want { | ||
|
Member
There was a problem hiding this comment.
newBody := []byte("Hello, World! (longer)")
pctx := &pipeline.Context{Body: []byte("Replace this!")}
pctx.SetBody(newBody)
hm := withBodyMutation(passBodyResponse(), pctx).GetRequestBody().GetResponse().GetHeaderMutation()
if got, want := mutationHeaderValue(hm, "content-length"), strconv.Itoa(len(newBody)); got != want {Not blocking — the test still pins the regression it was written for (before this PR there was no |
||
| t.Fatalf("content-length = %q, want %q", got, want) | ||
| } | ||
| } | ||
|
|
||
| func TestHandleResponseBody_SetsContentLength(t *testing.T) { | ||
| newBody := []byte("Hello, World! (longer)") | ||
| p, err := pipeline.New([]pipeline.Plugin{&responseMutator{newBody}}) | ||
| if err != nil { | ||
| t.Fatal(err) | ||
| } | ||
| srv := &Server{OutboundPipeline: pipeline.NewHolder(p)} | ||
| pctx := &pipeline.Context{ResponseHeaders: http.Header{"Content-Encoding": {"gzip"}}} | ||
|
|
||
| resp := srv.handleResponseBody(context.Background(), []byte("Replace this!"), pctx, "") | ||
| hm := resp.GetResponseBody().GetResponse().GetHeaderMutation() | ||
| if got, want := mutationHeaderValue(hm, "content-length"), strconv.Itoa(len(newBody)); got != want { | ||
| t.Fatalf("content-length = %q, want %q", got, want) | ||
| } | ||
| if !mutationRemovesHeader(hm, "content-encoding") { | ||
| t.Fatalf("content-encoding not removed: %+v", hm) | ||
| } | ||
| } | ||
|
|
||
| type responseMutator struct{ newBody []byte } | ||
|
|
||
| func (*responseMutator) Name() string { return "response-mutator" } | ||
| func (*responseMutator) Capabilities() pipeline.PluginCapabilities { | ||
| return pipeline.PluginCapabilities{WritesBody: true} | ||
| } | ||
| func (*responseMutator) OnRequest(context.Context, *pipeline.Context) pipeline.Action { | ||
| return pipeline.Action{Type: pipeline.Continue} | ||
| } | ||
| func (m *responseMutator) OnResponse(_ context.Context, pctx *pipeline.Context) pipeline.Action { | ||
| pctx.SetResponseBody(m.newBody) | ||
| return pipeline.Action{Type: pipeline.Continue} | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: this is now the only site in the file that emits a response-phase
HeaderMutation, and it is deliberately self-contained —withHeaderMutationreturns early for the response phase (default: return resp, server.go:767), so plugin-made response header edits are still not propagated.A one-line comment saying so would help: the request path composes (
withHeaderMutationthenwithBodyMutationappend into oneHeaderMutation), and a future reader could reasonably assume the same symmetry holds here and that appending to this mutation would pick up plugin edits. It would not.