httptransport: add client-close detection

If an HTTP handler gets into the error handler, it now checks if the
request context is still valid. If not, the handler does not attempt to
write any response header or bodies and reports the non-standard 499
status code.

Signed-off-by: Hank Donnay <[email protected]>

httptransport/error.go

@@ -12,25 +12,46 @@ import (
 	"github.com/quay/zlog"
 )
 
+// StatusClientClosedRequest is a nonstandard HTTP status code used when the
+// client has gone away.
+//
+// This convention is cribbed from Nginx.
+const statusClientClosedRequest = 499
+
 // ApiError writes an untyped (that is, "application/json") error with the
 // provided HTTP status code and message.
 //
 // ApiError does not return, but instead causes the goroutine to exit.
 func apiError(ctx context.Context, w http.ResponseWriter, code int, f string, v ...interface{}) {
 	const errheader = `Clair-Error`
-	h := w.Header()
-	h.Del("link")
-	h.Set("content-type", "application/json")
-	h.Set("x-content-type-options", "nosniff")
-	h.Set("trailer", errheader)
-	w.WriteHeader(code)
+	disconnect := false
+	select {
+	case <-ctx.Done():
+		disconnect = true
+	default:
+	}
 	if ev := zlog.Debug(ctx); ev.Enabled() {
 		ev.
+			Bool("disconnect", disconnect).
 			Int("code", code).
 			Str("error", fmt.Sprintf(f, v...)).
 			Msg("http error response")
+	} else {
+		ev.Send()
+	}
+	if disconnect {
+		// Exit immediately if there's no client to read the response, anyway.
+		w.WriteHeader(statusClientClosedRequest)
+		runtime.Goexit()
 	}
 
+	h := w.Header()
+	h.Del("link")
+	h.Set("content-type", "application/json")
+	h.Set("x-content-type-options", "nosniff")
+	h.Set("trailer", errheader)
+	w.WriteHeader(code)
+
 	var buf bytes.Buffer
 	buf.WriteString(`{"code":"`)
 	switch code {

httptransport/error_test.go

@@ -0,0 +1,58 @@
+package httptransport
+
+import (
+	"context"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/quay/zlog"
+
+	"github.com/quay/clair/v4/internal/httputil"
+)
+
+func TestClientDisconnect(t *testing.T) {
+	var status int
+	// Bunch of sequencing events:
+	reqStart := make(chan struct{})
+	reqDone := make(chan struct{})
+	handlerDone := make(chan struct{})
+
+	// Server side:
+	//	- Emit reqStart once the request is received.
+	//	- Emit handlerDone once the request is done and "status" should be populated.
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		defer func() { close(handlerDone) }()
+		w = httputil.ResponseRecorder(&status, nil, w)
+		ctx := zlog.Test(r.Context(), t) // The error handler emits logs.
+		close(reqStart)
+		<-ctx.Done()
+		apiError(ctx, w, http.StatusOK, "hello from the handler")
+	}))
+	t.Cleanup(srv.Close)
+
+	ctx, done := context.WithCancel(context.Background())
+	// Closing "done" will cancel the client connection.
+	req, err := http.NewRequestWithContext(ctx, "GET", srv.URL+"/", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	// Emit reqDone when the client connection is done.
+	go func() {
+		_, err = srv.Client().Do(req)
+		close(reqDone)
+	}()
+
+	<-reqStart
+	done()
+	<-reqDone
+	t.Logf("got request error: %v", err)
+	if err == nil {
+		t.Error("expected non-nil error")
+	}
+
+	<-handlerDone
+	if got, want := status, statusClientClosedRequest; got != want {
+		t.Errorf("bad status code recorded: got: %d, want: %d", got, want)
+	}
+}