http: RoundTripper mutating *Request

[bradfitz]

The http Transport RoundTripper probably shouldn't mutate *Request at all.

And we should document RoundTripper to say that Request shouldn't be touched either.

[rsc]

Comment 1:

(or document that it can modify the request.)

[bradfitz]

Comment 2:

It's already documented that it may modify the request:
    // RoundTrip may modify the request. The request Headers field is
    // guaranteed to be initialized.
    RoundTrip(req *Request) (resp *Response, err os.Error)
The only reason it does so, though, is to guard against Request.URL being nil, and
instead populating it based on parsing Request.RawURL.
I'd first prefer to have it documented what the precedence and relationship between URL
and RawURL was.  We've documented the rest of Request's fields well, but not those two.
Alternatively, the Transport code could clone the *Request and only mutate its private
copy but I worry about people assuming pointer equality of requests with chained
transports (e.g. people using http://gorilla-web.appspot.com/pkg/gorilla/context/ ).
Or I could just change Transport to pass around a Request wrapper type with extra state.
But before I do that, let's discuss URL vs RawURL.

Status changed to Thinking.

[rsc]

Comment 3:

RawURL should take precedence.  Let's make it not modify.
It should be possible to not modify and not make a copy either.

Status changed to HelpWanted.

[bradfitz]

Comment 5:

http://golang.org/cl/5211043
http: prevent RoundTrip from modifying *Request
WIP on issue #2146
But now fails with:
FAIL: http_test.TestTransportNilURL (0.00 seconds)
unexpected RoundTrip error: http: Request.Write on Request with no Host or URL
set
I strongly dislike Request.RawURL vs Request.URL.
RawURL has different meanings in server vs. client
context, sometimes is set vs sometimes not.
I'd rather kill RawURL entirely (or rationalize its
existence) before proceeding further with fixing this.
Could RawURL's purpose(s) be folded into *URL somehow?

[bradfitz]

Comment 6:

This issue was closed by revision b9ad278.

Status changed to Fixed.