go-kit / kit

A standard library for microservices.
https://gokit.io
MIT License
26.53k stars 2.43k forks source link

http.EncodeJSONResponse body writing is not compatible with net/http #1291

Open kerma opened 4 months ago

kerma commented 4 months ago

What did you do?

  1. Implemented http.StatusCoder() which returns 304 (Not Modified)
  2. Observed an error when http.EncodeJSONResponse is called: http: request method or response status code does not allow body

It seems that body is not written only on 204: https://github.com/go-kit/kit/blob/master/transport/http/server.go#L177

However, stdlib defines "no body allowed" as 100-199, 204, 304: https://cs.opensource.google/go/go/+/refs/tags/go1.22.3:src/net/http/transfer.go;l=460

What did you expect?

No error.

What happened instead?

Got an error.

peterbourgon commented 4 months ago

Oops. Seems like a simple PR to fix.

kerma commented 4 months ago

Using this fix myself:

func EncodeJSONResponse(_ context.Context, w http.ResponseWriter, response any) error {
    w.Header().Set("Content-Type", "application/json; charset=utf-8")
    if headerer, ok := response.(http.Headerer); ok {
        for k, values := range headerer.Headers() {
            for _, v := range values {
                w.Header().Add(k, v)
            }
        }
    }
    code := http.StatusOK
    if sc, ok := response.(http.StatusCoder); ok {
        code = sc.StatusCode()
    }
    w.WriteHeader(code)
    if !bodyAllowedForStatus(code) {
        return nil
    }
    return json.NewEncoder(w).Encode(response)
}

// bodyAllowedForStatus reports whether a given response status code
// permits a body. See RFC 7230, section 3.3.
// forked from net/http/transfer.go
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
}