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
net/http: TimeoutHandler doesn't log superfluous WriteHeader calls #30803
Comments
Thank you for reporting this issue @LarsFox and welcome to the Go project! I am don't think that we have a guarantee that every handler provided by the Here is a standalone repro that can be run on the Go playground https://play.golang.org/p/B-LBC3SyB47 package main
import (
"log"
"net/http"
"net/http/httptest"
"net/http/httputil"
"reflect"
"time"
)
func h404() http.Handler {
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
log.Println(reflect.TypeOf(w))
w.WriteHeader(404)
w.WriteHeader(404)
})
}
func main() {
cst := httptest.NewServer(http.TimeoutHandler(h404(), time.Second*10, "no logs here"))
defer cst.Close()
for i := 0; i < 100; i++ {
res, err := cst.Client().Get(cst.URL)
if err != nil {
log.Printf("%d: error: %v", i, err)
continue
}
blob, _ := httputil.DumpResponse(res, true)
res.Body.Close()
log.Printf("#%d:\n%s\n\n", i, blob)
}
} which produces 009/11/10 23:00:00 *http.timeoutWriter
2009/11/10 23:00:00 #0:
HTTP/1.1 404 Not Found
Date: Tue, 10 Nov 2009 23:00:00 GMT
Content-Length: 0
2009/11/10 23:00:00 *http.timeoutWriter
2009/11/10 23:00:00 #1:
HTTP/1.1 404 Not Found
Date: Tue, 10 Nov 2009 23:00:00 GMT
Content-Length: 0
2009/11/10 23:00:00 *http.timeoutWriter
2009/11/10 23:00:00 #2:
HTTP/1.1 404 Not Found
Date: Tue, 10 Nov 2009 23:00:00 GMT
Content-Length: 0 but no log to superfluous WriteHeader. Kindly paging @bradfitz @tombergan |
@LarsFox |
@Gnouc, didn’t quite get the difference. func (w *response) WriteHeader(code int) {
if w.conn.hijacked() {
caller := relevantCaller()
w.conn.server.logf("http: response.WriteHeader on hijacked connection from %s (%s:%d)", caller.Function, path.Base(caller.File), caller.Line)
return
}
if w.wroteHeader {
caller := relevantCaller()
w.conn.server.logf("http: superfluous response.WriteHeader call from %s (%s:%d)", caller.Function, path.Base(caller.File), caller.Line)
return
}
checkWriteHeaderCode(code)
w.wroteHeader = true
w.status = code
// more stuff goes on with the Content-Length header func (tw *timeoutWriter) WriteHeader(code int) {
checkWriteHeaderCode(code)
tw.mu.Lock()
defer tw.mu.Unlock()
if tw.timedOut || tw.wroteHeader {
return
}
tw.writeHeader(code)
}
func (tw *timeoutWriter) writeHeader(code int) {
tw.wroteHeader = true
tw.code = code
} Also, I don’t know, but by looking through the source code I found out that if I try sending codes of 404 and then 1000 (which is very naughty), only |
@LarsFox normal writer also modify the content length header in WriteHeader, timeoutWriter doesn't.
|
Change https://golang.org/cl/200518 mentions this issue: |
@cuonglm @LarsFox thanks for the comment in #30803 (comment), would you mind filing an issue highlighting that other inconsistency, if an issue doesn't exist already? |
What version of Go are you using (
go version
)?What did you do?
Here is the program that listens and serves 2 ports with a buggy handler that calls
WriteHeader
twice. The second port, however, is also wrapped in ahttp.TimeoutHandler
.Then I test the handlers with
curl localhost:8500 && curl localhost:8501
.What did you expect to see?
Two warnings.
What did you see instead?
A single warning.
Am I doing something terribly wrong?
System details
The text was updated successfully, but these errors were encountered: