diff --git a/core/action/action.go b/core/action/action.go index fcb9c9bc6..deaf7fb3a 100644 --- a/core/action/action.go +++ b/core/action/action.go @@ -18,6 +18,12 @@ import ( log "github.com/sirupsen/logrus" ) +// RemoteActionTimeout caps the total time spent on a remote post-serve action +// HTTP call (dial, TLS handshake, request write, response read). Without it a +// non-responsive endpoint would leak the calling goroutine forever +// (GHSA-42j2-w334-qxw7). Exported so tests can lower it. +var RemoteActionTimeout = 30 * time.Second + type Action struct { Binary string Script *os.File @@ -140,7 +146,8 @@ func (action *Action) Execute(pair *models.RequestResponsePair, journalIDChannel req.Header.Add("Content-Type", "application/json") req.Header.Add("X-CORRELATION-ID", correlationID) - resp, err := http.DefaultClient.Do(req) + client := &http.Client{Timeout: RemoteActionTimeout} + resp, err := client.Do(req) completionTime := time.Now() journalID, received := receiveJournalIdWithTimeout(journalIDChannel, time.Second) log.Info("Journal ID received ", journalID) diff --git a/core/action/action_test.go b/core/action/action_test.go index b03d86463..f3b52a841 100644 --- a/core/action/action_test.go +++ b/core/action/action_test.go @@ -4,6 +4,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/SpectoLabs/hoverfly/core/journal" "github.com/gorilla/mux" @@ -166,3 +167,44 @@ func Test_ExecuteRemotePostServeAction_WithUnReachableHost(t *testing.T) { func processHandlerOkay(w http.ResponseWriter, r *http.Request) { w.WriteHeader(200) } + +// Regression test for GHSA-42j2-w334-qxw7: a remote post-serve action pointed +// at a non-responsive endpoint must not block forever. Without the client +// timeout the goroutine would leak indefinitely. +func Test_ExecuteRemotePostServeAction_TimesOutOnSlowEndpoint(t *testing.T) { + RegisterTestingT(t) + + block := make(chan struct{}) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + <-block + })) + defer server.Close() // runs last; needs handler to have exited + defer close(block) // runs first; unblocks handler so server can close + + originalTimeout := action.RemoteActionTimeout + action.RemoteActionTimeout = 100 * time.Millisecond + defer func() { action.RemoteActionTimeout = originalTimeout }() + + newAction, err := action.NewRemoteAction(server.URL+"/slow", 0) + Expect(err).To(BeNil()) + + originalPair := models.RequestResponsePair{ + Response: models.ResponseDetails{Body: "Normal body"}, + } + journalIDChannel := make(chan string, 1) + journalIDChannel <- "1" + newJournal := journal.NewJournal() + + done := make(chan error, 1) + go func() { + done <- newAction.Execute(&originalPair, journalIDChannel, newJournal) + }() + + select { + case err := <-done: + Expect(err).NotTo(BeNil()) + case <-time.After(5 * time.Second): + t.Fatal("Execute did not return within 5s — timeout not applied") + } + close(journalIDChannel) +}