Skip to content

Commit

Permalink
httprouter: unexport HTTPContext.Writer
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
mvdan authored and p4u committed Feb 23, 2024
1 parent 23129ba commit 4c258e3
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 10 deletions.
2 changes: 1 addition & 1 deletion httprouter/httprouter.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
17 changes: 8 additions & 9 deletions httprouter/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -57,32 +56,32 @@ 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 {
return string(msg[:256]) + "..."
}
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
}

0 comments on commit 4c258e3

Please sign in to comment.