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

fix(WrapResponseWriter): allow multiple informational statuses #961

Conversation

costela
Copy link
Contributor

@costela costela commented Dec 3, 2024

Informational status in the range 100-199 may be sent multiple times, unlike statuses in the range 200-599.

This follows usage in the stdlib:
https://github.com/golang/go/blob/485ed2fa5b5e0b7067ef72a0f4bdc9ca12b77ed7/src/net/http/server.go#L1216

See also docs on WriteHeader (emphasis added):

The provided code must be a valid HTTP 1xx-5xx status code.
Any number of 1xx headers may be written, followed by at most
one 2xx-5xx
header. 1xx headers are sent immediately, but 2xx-5xx
headers may be buffered. Use the Flusher interface to send
buffered data. The header map is cleared when 2xx-5xx headers are
sent, but not with 1xx headers.

@costela costela changed the title fix: allow multiple informational status fix: allow multiple informational statuses Dec 3, 2024
@costela costela changed the title fix: allow multiple informational statuses fix(WrapResponseWriter): allow multiple informational statuses Dec 3, 2024
Copy link
Contributor

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, thank you.

Minor suggestion.

@@ -81,6 +81,10 @@ type basicWriter struct {
}

func (b *basicWriter) WriteHeader(code int) {
if code >= 100 && code <= 199 && code != http.StatusSwitchingProtocols {
b.ResponseWriter.WriteHeader(code)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we're missing if !b.discard {, same as below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, you're right! Fixed. PTAL

Informational status in the range 100-199 may be sent multiple times, unlike statuses in the range 200-599.

This follows usage in the stdlib:
https://github.com/golang/go/blob/485ed2fa5b5e0b7067ef72a0f4bdc9ca12b77ed7/src/net/http/server.go#L1216
@costela costela force-pushed the fix-wrapresponsewriter-writing-multiple-informational-statuses branch from 3ea3720 to 45de05a Compare December 4, 2024 13:49
Copy link
Contributor

@VojtechVitek VojtechVitek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@VojtechVitek VojtechVitek merged commit d9d5e31 into go-chi:master Dec 14, 2024
18 checks passed
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 this pull request may close these issues.

2 participants