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

Invalid HTTP status code 0 #897

Closed
vmihailenco opened this issue Jul 4, 2020 · 6 comments · Fixed by #908
Closed

Invalid HTTP status code 0 #897

vmihailenco opened this issue Jul 4, 2020 · 6 comments · Fixed by #908

Comments

@vmihailenco
Copy link
Contributor

http.Handler that does not explicitly sets HTTP status code gets following attributes:

status.code - Unknown
status.message - Invalid HTTP status code 0

At least the status message looks wrong because Go uses 200 OK status code when status code is not explicitly set. So the status code is known and is valid.

@Aneurysm9
Copy link
Member

Can you provide a minimal repro? I've tried to reproduce with the othttp.Handler and have been unable. Its responseWriterWrapper ensures that the status is set.

I think that the helper in api/standard is correct as 0 is not a valid HTTP status code. Instrumentations should ensure that it is invoked with a valid status code.

@vmihailenco
Copy link
Contributor Author

https://gist.github.com/vmihailenco/1028b37642cd04ed0222cfe8a3125dbf prints

"StatusCode":2,"StatusMessage":"Invalid HTTP status code 0"

Its responseWriterWrapper ensures that the status is set.

It just records the status code. I don't see how it does anything beyond that.

@Aneurysm9
Copy link
Member

It just records the status code. I don't see how it does anything beyond that.

The responseWriterWrapper will ensure that the status code is http.StatusOK if Write() is called before WriteHeader(), in conformance with the http.ResponseWriter interface. Your repro does neither. In that case the rww.statusCode value remains at its zero value since nothing has updated it.

Is there a valid use for an http.Handler that does not call either Write() or WriteHeader() on its provided http.ResponseWriter?

@vmihailenco
Copy link
Contributor Author

Is there a valid use for an http.Handler that does not call either Write() or WriteHeader() on its provided http.ResponseWriter?

No calls to WriteHeader means WriteHeader(200). What is wrong with that?

@Aneurysm9
Copy link
Member

No calls to WriteHeader means WriteHeader(200). What is wrong with that?

I don't think it does. No calls to WriteHeader() before a call to Write() means WriteHeader(200). See the comments on the interface. No calls to either WriteHeader() or Write() is undefined behavior.

I can't think of a valid reason for an HTTP handler to provide zero response to a request, but it was easy to make a change in #908 that may avoid unexpected behavior.

@vmihailenco
Copy link
Contributor Author

No calls to either WriteHeader() or Write() is undefined behavior.

In my universe it always sends 200 OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants