http.Redirect should not clean the query params of the redirect URL

[gopherbot]

by jgennis:

What steps will reproduce the problem?
1. Set up a basic Go AppEngine app using the dev server
2. Create a URL handler that uses http.Redirect to redirect a GET to the
AppEngine-provided login page (the string returned from user.LoginURL).

AppEngine is likely not be required to reproduce this, but I haven't done so using a
standalone HTTP server.

What is the expected output?

A redirect to http://localhost:8080/_ah/login?continue=http%3A//localhost%3A8080/

What do you see instead?

A redirect to http://localhost:8080/_ah/login?continue=http%3A/localhost%3A8080/

Note that the 'http%3A//' in the "continue" query param was collapsed to
'http%3A/'.

Which compiler are you using (5g, 6g, 8g, gccgo)?

The 6g compiler in the Go AppEngine 1.5.0 dev server.

Which operating system are you using?

Linux

Which revision are you using?  (hg identify)

r57.1

Please provide any additional information below.

The problem appears to be (though I haven't verified this) the http.Clean call being
done in http.Redirect at the following line:

http://golang.org/src/pkg/http/server.go?s=17120:17145#L586

The Clean call is cleaning the query parameters when it should just be cleaning the path
portion of the URL.

This issue can be worked around by manually setting the Location header and redirect
code rather than calling http.Redirect.

[rsc]

Comment 1:

no idea if this is true

Owner changed to @bradfitz.

Status changed to Accepted.

[bradfitz]

Comment 2:

Labels changed: added pkg-http.

[bradfitz]

Comment 3:

This was fixed May 5th, and submitted May 11th:
http://code.google.com/p/go/source/detail?r=3556fc4657a3
r57 was cut on May 3rd.  Considering that a work-around was available (setting the
Location header yourself), we didn't cherry-pick that fix into the stable branch at the
time.
The next stable release will include this.

Status changed to Invalid.