From 4c258e38d24649b4eb1f3b05ae110ed3159023a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Thu, 22 Feb 2024 22:05:03 +0000 Subject: [PATCH] httprouter: unexport HTTPContext.Writer Since it's too easy to use it directly instead of the Send method, which can then cause not closing the send message and not returning from the ServeHTTP handler goroutine at all. This caused a nasty goroutine leak and hang bug in another repo. While here, skip closing the request body, as net/http.Server does this. --- httprouter/httprouter.go | 2 +- httprouter/message.go | 17 ++++++++--------- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/httprouter/httprouter.go b/httprouter/httprouter.go index 57fe6f398..aa0fd2c48 100644 --- a/httprouter/httprouter.go +++ b/httprouter/httprouter.go @@ -275,7 +275,7 @@ func (r *HTTProuter) routerHandler(namespaceID string, accessType AuthAccessType return } - hc := &HTTPContext{Request: req, Writer: w, sent: make(chan struct{})} + hc := &HTTPContext{Request: req, writer: w, sent: make(chan struct{})} msg := Message{ Data: data, TimeStamp: time.Now(), diff --git a/httprouter/message.go b/httprouter/message.go index 8eaefabfc..3a08515d0 100644 --- a/httprouter/message.go +++ b/httprouter/message.go @@ -21,7 +21,7 @@ type Message struct { // HTTPContext is the Context for an HTTP request. type HTTPContext struct { - Writer http.ResponseWriter + writer http.ResponseWriter Request *http.Request contentType string @@ -46,7 +46,6 @@ func (h *HTTPContext) Send(msg []byte, httpStatusCode int) error { } }() defer close(h.sent) - defer h.Request.Body.Close() if httpStatusCode < 100 || httpStatusCode >= 600 { return fmt.Errorf("http status code %d not supported", httpStatusCode) @@ -57,21 +56,21 @@ func (h *HTTPContext) Send(msg []byte, httpStatusCode int) error { } // Set the content type if not set to default application/json if h.contentType == "" { - h.Writer.Header().Set("Content-Type", DefaultContentType) + h.writer.Header().Set("Content-Type", DefaultContentType) } else { - h.Writer.Header().Set("Content-Type", h.contentType) + h.writer.Header().Set("Content-Type", h.contentType) } if httpStatusCode == http.StatusNoContent { // For 204 status, don't set Content-Length, don't try to write a body. - h.Writer.WriteHeader(httpStatusCode) + h.writer.WriteHeader(httpStatusCode) log.Debugw("http response", "status", httpStatusCode) return nil } // Content length will be message length plus newline character - h.Writer.Header().Set("Content-Length", fmt.Sprintf("%d", len(msg)+1)) - h.Writer.WriteHeader(httpStatusCode) + h.writer.Header().Set("Content-Length", fmt.Sprintf("%d", len(msg)+1)) + h.writer.WriteHeader(httpStatusCode) log.Debugw("http response", "status", httpStatusCode, "data", func() string { if len(msg) > 256 { @@ -79,10 +78,10 @@ func (h *HTTPContext) Send(msg []byte, httpStatusCode int) error { } return string(msg) }()) - if _, err := h.Writer.Write(msg); err != nil { + if _, err := h.writer.Write(msg); err != nil { return err } // Ensure we end the response with a newline, to be nice. - _, err := h.Writer.Write([]byte("\n")) + _, err := h.writer.Write([]byte("\n")) return err }