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

net/http/httputil: RoundTripper leaving side effect status code 1XX in responseWriter #67534

Open
simonlsk opened this issue May 20, 2024 · 5 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@simonlsk
Copy link

Go version

go1.22.3 darwin/arm64

Output of go env in your module/workspace:

GO111MODULE='on'
GOARCH='arm64'
GOBIN=''
GOCACHE='/Users/gopher/Library/Caches/go-build'
GOENV='/Users/gopher/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMODCACHE='/Users/gopher/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/gopher/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/Users/gopher/sdk/go1.22.3'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/gopher/sdk/go1.22.3/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.22.3'
GCCGO='gccgo'
AR='ar'
CC='clang'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/gopher/workspace/dags/projects/dagshub/gogs/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/pp/1mc266xs6x57phbmgzl5js2w0000gn/T/go-build60180279=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

I upgraded go from 1.22.1 to 1.22.3, and part of my http proxy response modifier stopped working correctly.

What did you see happen?

I setup my ReverseProxy like so:

proxy := &httputil.ReverseProxy{
	Director:       ctx.Redirect,
	Transport:      ctx,
	ModifyResponse: ctx.MyModifyResponse,
	ErrorHandler:   stubErrorHandler,
}

proxy.ServeHTTP(ctx.Resp, ctx.Req.Request)

In my function ctx.MyModifyResponse I check the status code of my http.ResponseWriter to verify nothing was written previously.

if ctx.Resp.status != 0 {
    return errors.WithStack(&ContextWrittenError{})
}

But now since 972870d has been implemented, when a request returns intermediate 100 responses, the last 100 response stays in the status field of my ReponseWriter, which causes MyModifyResponse to fail.

What did you expect to see?

I expected the ResponseWriter to return to its original state once the transport.RoundTrip(outreq) is done, and the status returned is not 1XX.

@simonlsk simonlsk changed the title net/http/httputil: RoundTripper leaving side effect status code 1XX in responseWriter. net/http/httputil: RoundTripper leaving side effect status code 1XX in responseWriter May 20, 2024
@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label May 20, 2024
@cagedmantis cagedmantis added this to the Backlog milestone May 20, 2024
@cagedmantis
Copy link
Contributor

@neild @rsc

@neild
Copy link
Contributor

neild commented May 23, 2024

The http.ResponseWriter interface doesn't have a way to query what the last status written is. So when you say "I check the status code of my http.ResponseWriter", I'm guessing you're talking about a custom ResponseWriter implementation of some kind.

This seems to be an issue for that implementation.

Or perhaps I'm not understanding something.

@simonlsk
Copy link
Author

Yes correct, that affects this specific implementation.
The behavior that seems odd to me is that the status code is being written into the provided response writer, before the final response is actually handed over to http.ResponseWriter at the end of ServeHTTP.
My intuition is the http.ResponseWriter is not supposed to be called before the response has finished being manipulated.

@neild
Copy link
Contributor

neild commented May 23, 2024

1xx response codes are interim responses that occur before the final response is sent.

For example, a 100 Continue response is sent to tell a client that the server has accepted the request. See https://www.rfc-editor.org/rfc/rfc9110.html#section-15.2.

@simonlsk
Copy link
Author

simonlsk commented Jun 5, 2024

I understand.
This is changing the behavior of any code inspecting a custom ResponseWriter inside ModifyResponse. Additionally, if the underlying proxied server returns 1xx response codes does it mean the proxy server has to behave similarly?
I hope my issue is clear, if you need any more clarification, I'll be happy to give it. If you think this is expected behavior then I understand.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

3 participants