I don't even want to spend the mental energy reviewing this while we're in
code freeze unless you tell me it's an important regression from Go 1.2.
Can this wait?
On Mon, May 19, 2014 at 11:35 AM, <daniel.morsing@gmail.com> wrote:
> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> net/http: don't change host header when following redirects
>
> If a client used the feature of having a different host from the value
> in req.URL.Host, the Client would change the host when following
> redirects.
>
> If we're following a local redirect, keep sending the same Host header.
>
> Fixes issue 8027.
>
> Please review this at https://codereview.appspot.com/98320044/
>
> Affected files (+66, -1 lines):
> M src/pkg/net/http/client.go
> M src/pkg/net/http/client_test.go
>
>
> Index: src/pkg/net/http/client.go
> ===================================================================
> --- a/src/pkg/net/http/client.go
> +++ b/src/pkg/net/http/client.go
> @@ -317,10 +317,17 @@
> nreq.Method = "GET"
> }
> nreq.Header = make(Header)
> - nreq.URL, err = base.Parse(urlStr)
> + var relurl *url.URL
> + relurl, err = url.Parse(urlStr)
> if err != nil {
> break
> }
> + // if someone used the trick of setting a
> different Host and URL.Host
> + // we need to make sure that we don't change the
> Host
> + if !relurl.IsAbs() {
> + nreq.Host = req.Host
> + }
> + nreq.URL = base.ResolveReference(relurl)
> if len(via) > 0 {
> // Add the Referer header.
> lastReq := via[len(via)-1]
> Index: src/pkg/net/http/client_test.go
> ===================================================================
> --- a/src/pkg/net/http/client_test.go
> +++ b/src/pkg/net/http/client_test.go
> @@ -1036,3 +1036,61 @@
> t.Errorf("Response trailers = %#v; want %#v", res.Trailer,
> want)
> }
> }
> +
> +func TestClientChangesHost(t *testing.T) {
> + defer afterTest(t)
> + saw := make(chan string, 3)
> + var ts *httptest.Server
> + ts = httptest.NewServer(HandlerFunc(func(w ResponseWriter, r
> *Request) {
> + saw <- r.Host
> + switch r.URL.Path {
> + case "/":
> + Redirect(w, r, "/foo", StatusFound)
> + case "/foo":
> + Redirect(w, r, ts.URL+"/bar", StatusFound)
> + }
> + }))
> + defer ts.Close()
> +
> + req, err := NewRequest("GET", ts.URL, nil)
> + if err != nil {
> + t.Fatalf("expected successful request build, got %q", err)
> + }
> + req.Host = "example.com"
> +
> + c := &Client{}
> + _, err = c.Do(req)
> + if err != nil {
> + t.Fatalf("expected successful request, got %q", err)
> + }
> + select {
> + case host := <-saw:
> + if host != "example.com" {
> + t.Fatalf("expected host to be 'example.com', got
> %q", host)
> + }
> + default:
> + t.Fatal("server didn't see a request")
> + }
> +
> + select {
> + case host := <-saw:
> + if host != "example.com" {
> + t.Fatalf("expected host to be 'example.com', got
> %q", host)
> + }
> + default:
> + t.Fatal("server didn't see a second request")
> + }
> +
> + select {
> + case host := <-saw:
> + tsurl, err := url.Parse(ts.URL)
> + if err != nil {
> + t.Fatalf("expected successful url parse, got %q",
> err)
> + }
> + if host != tsurl.Host {
> + t.Fatalf("expected host to be %q, got %q",
> tsurl.Host, host)
> + }
> + default:
> + t.Fatal("server didn't see the third request")
> + }
> +}
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>