Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 22 additions & 1 deletion gateway/webhook/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,28 @@ func (webhook *HandleT) RequestHandler(w http.ResponseWriter, r *http.Request) {
webhook.Register(sourceDefName)
}
webhook.requestQMu.RLock()
requestQ := webhook.requestQ[sourceDefName]
requestQ, ok := webhook.requestQ[sourceDefName]
if !ok {
// The source type was not registered — either because
// `webhookV2HandlerEnabled` is false and the lazy registration in
// `processBackendConfig` has not yet run for this source, or because
// the source type is not one this instance handles. Sending on the
// nil channel returned by the map lookup would block this HTTP handler
// goroutine forever, leaving the client hanging and leaking a
// goroutine per unregistered request. Fail fast with 404 instead.
webhook.requestQMu.RUnlock()
stat := webhook.statReporterCreator(arctx, reqType)
stat.RequestFailed("invalidWebhookSource")
stat.Report(webhook.stats)
webhook.failRequest(
w,
r,
response.GetStatus(response.InvalidWebhookSource),
response.GetErrorStatusCode(response.InvalidWebhookSource),
)
webhook.ackCount.Add(1)
return
}
requestQ <- &req
webhook.requestQMu.RUnlock()

Expand Down
60 changes: 60 additions & 0 deletions gateway/webhook/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"sync"
"testing"
"testing/iotest"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -718,3 +719,62 @@ func newMockTransformerServer(successAfter int, successRespBody, failureRespBody
mockServer.Server = handler
return mockServer
}

// Regression test for #6999: sending a webhook request for a source type that
// has not been Register()ed used to send on a nil channel, blocking the HTTP
// handler goroutine forever. The fix returns 404 (InvalidWebhookSource) fast
// instead. This test intentionally does NOT call Register(sourceDefName) and
// asserts that the handler returns quickly with the correct status.
func TestWebhookRequestHandlerReturns404WhenSourceNotRegistered(t *testing.T) {
initWebhook()

ctrl := gomock.NewController(t)
mockGW := mockWebhook.NewMockGateway(ctrl)
mockTransformerFeaturesService := mock_features.NewMockFeaturesService(ctrl)

webhookHandler := Setup(
mockGW,
mockTransformerFeaturesService,
stats.NOP,
config.Default,
newSourceStatReporter,
func(bt *batchWebhookTransformerT) {
bt.sourceTransformAdapter = func(ctx context.Context) (sourceTransformAdapter, error) {
return &mockSourceTransformAdapter{}, nil
}
},
)
t.Cleanup(func() {
_ = webhookHandler.Shutdown()
})

// The nil-channel path is only reachable when webhookV2 is disabled, so
// the handler does not fall back to lazy Register() during RequestHandler.
webhookHandler.config.webhookV2HandlerEnabled = false

// NOTE: intentionally no webhookHandler.Register(...) — this is exactly
// the pre-condition that used to hang the handler goroutine.
req := httptest.NewRequest(http.MethodPost, "/v1/webhook", bytes.NewBufferString(sampleJson))
w := httptest.NewRecorder()
ctx := context.WithValue(req.Context(), gwtypes.CtxParamCallType, "webhook")
ctx = context.WithValue(ctx, gwtypes.CtxParamAuthRequestContext, &gwtypes.AuthRequestContext{
SourceDefName: "some-unregistered-source-type",
WriteKey: sampleWriteKey,
})
req = req.WithContext(ctx)

done := make(chan struct{})
go func() {
webhookHandler.RequestHandler(w, req)
close(done)
}()

select {
case <-done:
case <-time.After(2 * time.Second):
t.Fatal("RequestHandler blocked instead of returning 404 for unregistered source (#6999)")
}

assert.Equal(t, http.StatusNotFound, w.Result().StatusCode)
assert.Contains(t, strings.TrimSpace(w.Body.String()), response.InvalidWebhookSource)
}