matryer / respond

Idiomatic API responses for Go.
MIT License
88 stars 10 forks source link

Responding 204 leads to panic #5

Open haukened opened 2 years ago

haukened commented 2 years ago

Using HTTP status code 204 "No Content" to indicate that a request was successfully processed but no data is being returned leads to a panic.



 -> gopkg.in/matryer/respond%2ev1.with
 ->   /home/david/go/pkg/mod/gopkg.in/matryer/respond.v1@v1.0.1/respond.go:33

    gopkg.in/matryer/respond%2ev1.WithStatus
      /home/david/go/pkg/mod/gopkg.in/matryer/respond.v1@v1.0.1/respond.go:76
    main.UserListOne
      /home/david/dev/wfi_api/user.go:123
    net/http.HandlerFunc.ServeHTTP
      /usr/local/go/src/net/http/server.go:2047
    github.com/go-chi/chi/v5.(*Mux).routeHTTP
      /home/david/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.7/mux.go:442
    net/http.HandlerFunc.ServeHTTP
      /usr/local/go/src/net/http/server.go:2047
    github.com/go-chi/chi/v5/middleware.Timeout.func1.1
      /home/david/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.7/middleware/timeout.go:45
    net/http.HandlerFunc.ServeHTTP
      /usr/local/go/src/net/http/server.go:2047
    github.com/go-chi/chi/v5/middleware.Recoverer.func1
      /home/david/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.7/middleware/recoverer.go:38
    net/http.HandlerFunc.ServeHTTP
      /usr/local/go/src/net/http/server.go:2047
    github.com/go-chi/chi/v5/middleware.RequestID.func1
      /home/david/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.7/middleware/request_id.go:76
    net/http.HandlerFunc.ServeHTTP
      /usr/local/go/src/net/http/server.go:2047
    github.com/go-chi/chi/v5/middleware.RealIP.func1
      /home/david/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.7/middleware/realip.go:35
    net/http.HandlerFunc.ServeHTTP
      /usr/local/go/src/net/http/server.go:2047
    github.com/go-chi/chi/v5/middleware.RequestLogger.func1.1
      /home/david/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.7/middleware/logger.go:57
    net/http.HandlerFunc.ServeHTTP
      /usr/local/go/src/net/http/server.go:2047
    github.com/go-chi/chi/v5.(*Mux).ServeHTTP
      /home/david/go/pkg/mod/github.com/go-chi/chi/v5@v5.0.7/mux.go:88
    net/http.serverHandler.ServeHTTP
      /usr/local/go/src/net/http/server.go:2879
    net/http.(*conn).serve
      /usr/local/go/src/net/http/server.go:1930
    created by net/http.(*Server).Serve
      /usr/local/go/src/net/http/server.go:3034```
haukened commented 2 years ago

it appears this is coming from /net/http/server.go:41 where they define a BodyNotAllowed message:

// Errors used by the HTTP server.
var (
    // ErrBodyNotAllowed is returned by ResponseWriter.Write calls
    // when the HTTP method or response code does not permit a
    // body.
    ErrBodyNotAllowed = errors.New("http: request method or response status code does not allow body")

    // ErrHijacked is returned by ResponseWriter.Write calls when
    // the underlying connection has been hijacked using the
    // Hijacker interface. A zero-byte write on a hijacked
    // connection will return ErrHijacked without any other side
    // effects.
    ErrHijacked = errors.New("http: connection has been hijacked")

    // ErrContentLength is returned by ResponseWriter.Write calls
    // when a Handler set a Content-Length response header with a
    // declared size and then attempted to write more bytes than
    // declared.
    ErrContentLength = errors.New("http: wrote more than the declared Content-Length")

    // Deprecated: ErrWriteAfterFlush is no longer returned by
    // anything in the net/http package. Callers should not
    // compare errors against this variable.
    ErrWriteAfterFlush = errors.New("unused")
)

This is implemented further down at line 1541:

    if !w.bodyAllowed() {
        return 0, ErrBodyNotAllowed
    }

Looking at the source there, it appears there are several status codes where body is not allowed, including all 100 codes, 204, and 304:

// bodyAllowedForStatus reports whether a given response status code
// permits a body. See RFC 7230, section 3.3.
func bodyAllowedForStatus(status int) bool {
    switch {
    case status >= 100 && status <= 199:
        return false
    case status == 204:
        return false
    case status == 304:
        return false
    }
    return true
}
haukened commented 2 years ago

This is resolved in PR #6