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

responseWriterDelegator incorrectly handles 100 Continue response #1772

Open
VirrageS opened this issue Mar 11, 2025 · 1 comment · May be fixed by #1773
Open

responseWriterDelegator incorrectly handles 100 Continue response #1772

VirrageS opened this issue Mar 11, 2025 · 1 comment · May be fixed by #1773
Labels

Comments

@VirrageS
Copy link

Description

100 Continue response can be issued many times before an actual response is return (eg. 200 OK). In case of promhttp these responses should be ignored.

When you look at the http/server.go: https://github.com/golang/go/blob/go1.24.1/src/net/http/server.go#L1212-L1228 you can notice that they only set wroteHeader variable or status when it is not informational header.
But here https://github.com/prometheus/client_golang/blob/v1.21.1/prometheus/promhttp/delegator.go#L55-L66 we don't filter 100 Continue response.

Therefore, when 100 Continue is set and then there is no explicit 200 OK reported, then final status code will be 100 and not 200 as it should be.

Steps To Reproduce

  1. Use following handler:
    	targetHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
    		w.WriteHeader(http.StatusContinue) // Explicitly inform client that we are ready for body.
    		
    		body, _ := io.ReadAll(r.Body)
    		w.Write(body)
    	})
    	http.Handle("/target", promhttp.InstrumentHandlerResponseSize(metric, targetHandler))
  2. Run the code.
  3. Handler will report 100 Continue header as final status code instead of 200 OK.

Expected behavior

200 OK status code to be reported even when there is explicit 100 Continue done in the handler.

Likely we need to do similar thing that they do in http/server.go which is to simply ignore informational headers if they are issued before a regular response.

Copy link

dosubot bot commented Mar 11, 2025

Hey @VirrageS! I'm here to help you with any bugs, questions, or contributions. Let's take a look at the issue you've encountered with the responseWriterDelegator.

The issue you're encountering is due to the responseWriterDelegator in the promhttp package not filtering out informational status codes like 100 Continue. This results in the final status code being incorrectly reported as 100 instead of the expected 200 OK.

To address this, you can modify the WriteHeader method in the responseWriterDelegator to ignore informational status codes when setting the final status. This approach is similar to how the Go HTTP server handles such responses, ensuring that only non-informational status codes are considered for the final status.

Here's a potential solution:

  1. Update the WriteHeader method in responseWriterDelegator to check if the status code is informational (1xx) and only set the status if it's not:
func (r *responseWriterDelegator) WriteHeader(code int) {
    if code < 200 || code >= 300 {
        // Ignore informational status codes (1xx)
        return
    }
    if r.observeWriteHeader != nil && !r.wroteHeader {
        r.observeWriteHeader(code)
    }
    r.status = code
    r.wroteHeader = true
    r.ResponseWriter.WriteHeader(code)
}

This change ensures that informational status codes like 100 Continue are ignored, and the final status code reflects the actual response, such as 200 OK.

To continue talking to Dosu, mention @dosu.


Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Bug Report | Other

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

Successfully merging a pull request may close this issue.

1 participant