Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
kerma opened this issue May 10, 2024 · 2 comments · May be fixed by #1294
Open

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

kerma opened this issue May 10, 2024 · 2 comments · May be fixed by #1294
Labels

Comments

@kerma
Copy link

kerma commented May 10, 2024

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.

@kerma kerma added the bug label May 10, 2024
@peterbourgon
Copy link
Member

Oops. Seems like a simple PR to fix.

@kerma
Copy link
Author

kerma commented May 11, 2024

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
}

kerma added a commit to kerma/kit that referenced this issue Jul 10, 2024
kerma added a commit to kerma/kit that referenced this issue Jul 10, 2024
@kerma kerma linked a pull request Jul 10, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants