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: Proto field empty for Requests passed to CheckRedirect after the first #31441

Closed
andymacau853 opened this issue Apr 12, 2019 · 8 comments
Labels
FrozenDueToAge help wanted NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@andymacau853
Copy link

andymacau853 commented Apr 12, 2019

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 bcmills changed the title Proto returned empty string when jump into CheckRedirect functions net/http: Proto returned empty string when jump into CheckRedirect functions Apr 12, 2019
@bcmills
Copy link
Contributor

bcmills commented Apr 12, 2019

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?

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Apr 12, 2019
@andymacau853
Copy link
Author

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
Copy link
Contributor

bcmills commented Apr 12, 2019

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

@bcmills bcmills removed the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Apr 12, 2019
@bcmills bcmills changed the title net/http: Proto returned empty string when jump into CheckRedirect functions net/http: Proto field empty for Requests passed to CheckRedirect after the first Apr 12, 2019
@bcmills
Copy link
Contributor

bcmills commented Apr 12, 2019

CC @bradfitz

@bcmills bcmills added this to the Go1.13 milestone Apr 12, 2019
@bradfitz
Copy link
Contributor

bradfitz commented Apr 12, 2019

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
Copy link
Author

So, the result above is correct? why?

@bradfitz
Copy link
Contributor

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
Copy link
Author

So can fix this issue on next go release?

@golang golang locked and limited conversation to collaborators May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge help wanted 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

4 participants