Skip to content

Commit

Permalink
contrib/envoyproxy: fix empty request (#3089)
Browse files Browse the repository at this point in the history
  • Loading branch information
e-n-0 authored Jan 15, 2025
1 parent 39f9e5d commit 56c1875
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 0 deletions.
8 changes: 8 additions & 0 deletions contrib/envoyproxy/go-control-plane/envoy.go
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,14 @@ func propagationRequestHeaderMutation(span ddtrace.Span) (*envoyextproc.Processi
func processResponseHeaders(res *envoyextproc.ProcessingRequest_ResponseHeaders, currentRequest *currentRequest) (*envoyextproc.ProcessingResponse, error) {
log.Debug("external_processing: received response headers: %v\n", res.ResponseHeaders)

if currentRequest == nil {
// Can happen when a malformed request is sent to Envoy (with no header), the request is never sent to the External Processor and directly passed to the server
// However the response of the server (which is valid) is sent to the External Processor and fail to be processed
log.Warn("external_processing: can't process the response: envoy never sent the beginning of the request, this is a known issue" +
" and can happen when a malformed request is sent to Envoy where the header Host is missing. See link to issue https://github.com/envoyproxy/envoy/issues/38022")
return nil, status.Errorf(codes.InvalidArgument, "Error processing response headers from ext_proc: can't process the response")
}

if err := createFakeResponseWriter(currentRequest.wrappedResponseWriter, res); err != nil {
return nil, status.Errorf(codes.InvalidArgument, "Error processing response headers from ext_proc: %v", err)
}
Expand Down
47 changes: 47 additions & 0 deletions contrib/envoyproxy/go-control-plane/envoy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,53 @@ func TestXForwardedForHeaderClientIp(t *testing.T) {
})
}

func TestMalformedEnvoyProcessing(t *testing.T) {
appsec.Start()
defer appsec.Stop()
if !appsec.Enabled() {
t.Skip("appsec disabled")
}

setup := func() (envoyextproc.ExternalProcessorClient, mocktracer.Tracer, func()) {
rig, err := newEnvoyAppsecRig(t, false)
require.NoError(t, err)

mt := mocktracer.Start()

return rig.client, mt, func() {
rig.Close()
mt.Stop()
}
}

t.Run("response-received-without-request", func(t *testing.T) {
client, mt, cleanup := setup()
defer cleanup()

ctx := context.Background()
stream, err := client.Process(ctx)
require.NoError(t, err)

err = stream.Send(&envoyextproc.ProcessingRequest{
Request: &envoyextproc.ProcessingRequest_ResponseHeaders{
ResponseHeaders: &envoyextproc.HttpHeaders{
Headers: makeResponseHeaders(t, map[string]string{}, "400"),
},
},
})
require.NoError(t, err)

_, err = stream.Recv()
require.Error(t, err)
stream.Recv()

// No span created, the request is invalid.
// Span couldn't be created without request data
finished := mt.FinishedSpans()
require.Len(t, finished, 0)
})
}

func newEnvoyAppsecRig(t *testing.T, traceClient bool, interceptorOpts ...ddgrpc.Option) (*envoyAppsecRig, error) {
t.Helper()

Expand Down

0 comments on commit 56c1875

Please sign in to comment.