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 overwrites valid Authorization headers when user included in URL #11399

Closed
sinbad opened this issue Jun 25, 2015 · 1 comment
Milestone

Comments

@sinbad
Copy link
Contributor

sinbad commented Jun 25, 2015

In function send() in net/http/client.go:

    if u := req.URL.User; u != nil {
        username := u.Username()
        password, _ := u.Password()
        req.Header.Set("Authorization", "Basic "+basicAuth(username, password))
    }

This means if someone builds a Request which includes the username and password already encoded in the Authorization header, either set directly or via SetBasicAuth(), and the URL happens to include a username (but no password) - which is somewhat common in Git clone URLs for example - the request has its Authorization header re-written and authentication will fail, rather inexplicably for the developer who believes they constructed their request correctly (indeed sending the same thing via curl works). I only figured out why this was by using Wireshark then looking at the Go source!

I think Go should not overwrite the Authorization header if it is already present. I suggest this code should instead be:

    if u := req.URL.User; u != nil && req.Header.Get("Authorization") == "" {
        username := u.Username()
        password, _ := u.Password()
        req.Header.Set("Authorization", "Basic "+basicAuth(username, password))
    }

While you might suggest not using a username and no password in the URL if Authorization is present, these URLs are out there on systems already, and stripping them because of this unintuitive behaviour is the wrong approach.

I would have just submitted a PR if you were using them :)

@sinbad sinbad changed the title net/http overwrites valid Authorisation headers when user included in URL net/http overwrites valid Authorization headers when user included in URL Jun 25, 2015
sinbad added a commit to sinbad/go that referenced this issue Jun 25, 2015
sinbad added a commit to sinbad/go that referenced this issue Jun 25, 2015
… username is in the URL

See golang#11399

Change-Id: I3be7fbc86c5f62761f47122632f3e11b56cb6be6
sinbad added a commit to sinbad/go that referenced this issue Jun 26, 2015
Fixes golang#11399

Change-Id: I3be7fbc86c5f62761f47122632f3e11b56cb6be6
@gopherbot
Copy link

CL https://golang.org/cl/11510 mentions this issue.

@rsc rsc closed this as completed in 379d832 Jun 26, 2015
@mikioh mikioh changed the title net/http overwrites valid Authorization headers when user included in URL net/http: client overwrites valid Authorization headers when user included in URL Jun 27, 2015
@mikioh mikioh added this to the Go1.5 milestone Jun 27, 2015
@golang golang locked and limited conversation to collaborators Jun 27, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants