net/http: Proto field empty for Requests passed to CheckRedirect after the first

[andymacau853]

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

$ go version
go version go1.11.4 windows/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
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Administrator\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Project\Go\maas
set GOPROXY=
set GORACE=
set GOROOT=C:\Go
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fno-caret-diagnostics -Qunused-arguments -fmessage-length=
0 -fdebug-prefix-map=C:\Users\Administrator\AppData\Local\Temp\go-build757481154=/tmp/go-
build -gno-record-gcc-switches

What did you do?

1. I have CheckRedirect functions for check http's redirect.
2. When first time jump into CheckRedirect for http redirect, the Proto can show HTTP/1.1
3. When second time jump into CheckRedirect for http redirect or later, the Proto always show empty string!!!!

func Work2() {
	req, _ := http.NewRequest(
		http.MethodGet,
		"http://www.google.com",
		nil,
	)
	req.Header.Set("Accept", "text/html, application/xhtml+xml, */*")
	req.Header.Set("Accept-Language", "zh-TW")
	req.Header.Set("User-Agent", "Mozilla/5.0 (Windows NT 6.1; WOW64; Trident/7.0; rv:11.0) like Gecko")

	seq := 0
	res, err := (&http.Client{
		CheckRedirect: func(r *http.Request, v []*http.Request) error {
			request := v[seq].Method + " " + v[seq].URL.RequestURI() + " " + v[seq].Proto
			response := r.Response.Proto + " " + r.Response.Status
			seq++
			log.Println(request)
			log.Println(response)
			return nil
		},
	}).Do(req)
	if err != nil {
		log.Println(err)
	}
	defer res.Body.Close()
	request := res.Request.Method + " " + res.Request.URL.RequestURI() + " " + res.Request.Proto
	response := res.Proto + " " + res.Status
	log.Println(request)
	log.Println(response)
}

What did you expect to see?

Every time get the variable: Proto, always return HTTP/1.1
2019/04/12 22:52:02 GET / HTTP/1.1
2019/04/12 22:52:02 HTTP/1.1 302 Found
2019/04/12 22:52:02 GET /intl/zh-TW/ HTTP/1.1
2019/04/12 22:52:02 HTTP/1.1 302 Found
2019/04/12 22:52:02 GET /?hl=zh-TW HTTP/1.1
2019/04/12 22:52:02 HTTP/1.1 302 Found
2019/04/12 22:52:02 GET /?hl=zh-TW&gws_rd=ssl HTTP/1.1
2019/04/12 22:52:02 HTTP/2.0 200 OK

What did you see instead?

2019/04/12 22:52:02 GET / HTTP/1.1
2019/04/12 22:52:02 HTTP/1.1 302 Found
2019/04/12 22:52:02 GET /intl/zh-TW/
2019/04/12 22:52:02 HTTP/1.1 302 Found
2019/04/12 22:52:02 GET /?hl=zh-TW
2019/04/12 22:52:02 HTTP/1.1 302 Found
2019/04/12 22:52:02 GET /?hl=zh-TW&gws_rd=ssl
2019/04/12 22:52:02 HTTP/2.0 200 OK

[bcmills]

It's not obvious to me what you mean by “second time jump into CheckRedirect”. Could you give a concrete code snippet (ideally as a https://play.golang.org link) that demonstrates the problem?

[andymacau853]

It's not obvious to me what you mean by “second time jump into CheckRedirect”. Could you give a concrete code snippet (ideally as a https://play.golang.org link) that demonstrates the problem?

CheckRedirect is to call every time when http redirect to another destination!!!

[bcmills]

Thanks, the concrete code example makes it much clearer for me.

[bcmills]

CC @bradfitz

[bradfitz]

Those aren't copied because they're not relevant. The docs on net/http.Request.Proto (https://golang.org/pkg/net/http/#Request.Proto) say:

For client requests, these fields are ignored.

So I'm not sure there's anything to do here.

I could copy them through, but that'd only be misleading.

[andymacau853]

So, the result above is correct? why?

[bradfitz]

NewRequest populates its returned Request's Proto* fields, but that's a historical mistake. I think that predates understanding & documentation of what the Proto* fields represented. It wouldn't be worth it to change NewRequest at this point, but no need to add to the confusion by populating them in more places.

[andymacau853]

So can fix this issue on next go release?