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: Client should scope cookie to Request.Host before Request.URL #38988

Open
colinclerk opened this issue May 10, 2020 · 3 comments
Open

net/http: Client should scope cookie to Request.Host before Request.URL #38988

colinclerk opened this issue May 10, 2020 · 3 comments
Milestone

Comments

@colinclerk
Copy link

@colinclerk colinclerk commented May 10, 2020

What version of Go are you using (go version)?

$ go version
go version go1.14.2 linux/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/root/.cache/go-build"
GOENV="/root/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build866802505=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Created a httptest.Server with a handler that sets a cookie, then issued a request from a http.Client with a custom Host field.

https://play.golang.org/p/78mh9ZwYI_t

What did you expect to see?

I expected to see the cookie scoped to the Host field.

What did you see instead?

The cookie was scoped to httptest.Server.URL


I was seeing unexpected cookie behavior while testing and I believe this is the root of it:

go/src/net/http/client.go

Lines 170 to 186 in 57e32c4

func (c *Client) send(req *Request, deadline time.Time) (resp *Response, didTimeout func() bool, err error) {
if c.Jar != nil {
for _, cookie := range c.Jar.Cookies(req.URL) {
req.AddCookie(cookie)
}
}
resp, didTimeout, err = send(req, c.transport(), deadline)
if err != nil {
return nil, didTimeout, err
}
if c.Jar != nil {
if rc := resp.Cookies(); len(rc) > 0 {
c.Jar.SetCookies(req.URL, rc)
}
}
return resp, nil, nil
}

Instead of the client always using req.URL for the cookie jar, I believe it should first look at req.Host, then req.Header.Get("Host"), and then finally req.URL.

I think this would be consistent with the behavior of Request.Write described on the Request struct:

go/src/net/http/request.go

Lines 231 to 235 in 57e32c4

// For client requests, Host optionally overrides the Host
// header to send. If empty, the Request.Write method uses
// the value of URL.Host. Host may contain an international
// domain name.
Host string

@toothrot toothrot changed the title http.Client should scope cookie to Request.Host before Request.URL net/http: Client should scope cookie to Request.Host before Request.URL May 11, 2020
@toothrot toothrot added this to the Backlog milestone May 11, 2020
@toothrot
Copy link
Contributor

@toothrot toothrot commented May 11, 2020

@vdobler
Copy link
Contributor

@vdobler vdobler commented May 11, 2020

@colinclerk Thanks for this bug report. There might be a real issue here.

From https://tools.ietf.org/html/rfc6265#section-2.3

The request-host is the name of the host, as known by the user agent,
to which the user agent is sending an HTTP request or from which it
is receiving an HTTP response (i.e., the name of the host to which it
sent the corresponding HTTP request).

The term request-uri is defined in Section 5.1.2 of [RFC2616].

According to https://tools.ietf.org/html/rfc6265#section-5.4 domain matching works on the request-host (with the unclear definition above).
The request-uri consists of the Host header and the abs_path, and the request-host sounds more like the r.URL.Host.

On the other hand: curl seems to use the Host header....

@colinclerk
Copy link
Author

@colinclerk colinclerk commented May 12, 2020

@vdobler What is the intended purpose of allowing outbound requests where r.Host != r.URL.Host?

When I issue outbound requests where r.Host != r.URL.Host, my intent is to mimic DNS resolution:

  • r.Host is what my user has in their address bar
  • r.URL.Host is where the DNS for r.Host resolves

Having this separation allows me to mimic a server that has multiple hosts pointed at it, and generate different responses depending on the Host.

Since I'm imagining r.Host is what my user has in their address bar, it's also where I'm expecting cookies to be set. But maybe I'm thinking about this all wrong?

Another exercise that might be helpful is to think about things from the server's perspective. r.URL.Host doesn't exist on the server, so when it issues SetCookie it expects the cookie to be set on the incoming request's r.Host.

That's not what happens when using Client and setting r.Host manually:
https://play.golang.org/p/NJIuiMhPP55

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

Successfully merging a pull request may close this issue.

None yet
3 participants
You can’t perform that action at this time.