Use httpsnoop to wrap ResponseWriter. #193
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Wrapping http.ResponseWriter is fraught with danger. Our compress handler made sure to implement all the optional ResponseWriter interfaces, but that made it implement them even if the underlying writer did not. For example, if the underlying ResponseWriter was _not_ an http.Hijacker, the compress writer nonetheless appeared to implement http.Hijacker, but would panic if you called Hijack(). On the other hand, the logging handler checked for certain combinations of optional interfaces and only implemented them as appropriate. However, it didn't check for all optional interfaces or all combinations, so most optional interfaces would still get lost. Fix both problems by using httpsnoop to do the wrapping. It uses code generation to ensure correctness, and it handles std lib changes like the http.Pusher addition in Go 1.8. Fixes gorilla#169.
elithrar
approved these changes
Aug 20, 2020
| @@ -220,3 +215,32 @@ func TestCompressHandlerPreserveInterfaces(t *testing.T) { | |||
| r.Header.Set(acceptEncoding, "deflate") | |||
| h.ServeHTTP(rw, r) | |||
| } | |||
|
|
|||
| type paltryResponseWriter struct{} | |||
|
Thanks very much, @muirdm! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Wrapping http.ResponseWriter is fraught with danger. Our compress
handler made sure to implement all the optional ResponseWriter
interfaces, but that made it implement them even if the underlying
writer did not. For example, if the underlying ResponseWriter was
not an http.Hijacker, the compress writer nonetheless appeared to
implement http.Hijacker, but would panic if you called Hijack().
On the other hand, the logging handler checked for certain
combinations of optional interfaces and only implemented them as
appropriate. However, it didn't check for all optional interfaces or
all combinations, so most optional interfaces would still get lost.
Fix both problems by using httpsnoop to do the wrapping. It uses code
generation to ensure correctness, and it handles std lib changes like
the http.Pusher addition in Go 1.8.
Fixes #169.