northwesternmutual / kanali

A Kubernetes Native API Management Solution
Apache License 2.0
186 stars 21 forks source link

http: multiple response.WriteHeader calls #83

Closed kh34 closed 7 years ago

kh34 commented 7 years ago

Seeing this message in the kanali logs during periods of high volume proxy traffic. Is this an error? How does this impact the client?

frankgreco commented 7 years ago

This is happening because a bug in whichw.WriteHeader(resp.StatusCode) is potentially called twice. It will be called twice if there is an error during the io.Copy operation when data is being written to the http response.

Based on the the stack overflow issue before, this fix is to not attempt to further handle an io.Copy error in the context of an HTTP response but instead, simply log the error.

below is pulled from https://stackoverflow.com/questions/26096944/when-responding-using-io-copy-who-should-be-responsible-for-the-error

io.Copy calls Write on the targetio.Writer. http.ResponseWriter's documentation on the Write method specifies this behavior:

// Write writes the data to the connection as part of an HTTP reply.
// If WriteHeader has not yet been called, Write calls WriteHeader(http.StatusOK)
// before writing the data.  If the Header does not contain a
// Content-Type line, Write adds a Content-Type set to the result of passing
// the initial 512 bytes of written data to DetectContentType.
Write([]byte) (int, error)

That means it will first call WriteHeader:

// WriteHeader sends an HTTP response header with status code.
// If WriteHeader is not called explicitly, the first call to Write
// will trigger an implicit WriteHeader(http.StatusOK).
// Thus explicit calls to WriteHeader are mainly used to
// send error codes.
WriteHeader(int)

So yes, if the io.Copy was to fail somewhen during a Write operation you'd already have written a 200 OK response, however, if your response specifies a Content-Length the client will know something is wrong when your response's length doesn't match.

frankgreco commented 7 years ago

Concerning how this affects the client, according to how the WriteHeader method is implemented (here), it will not be fatal. The response code will not represent the value from the initial call to WriteHeader.