http: false User-Agent in DumpRequest

[masiulaniec]

In documentation the function http.DumpRequest is defined to dump the request's wire
representation.  In reality it makes up some fields.  In below example notice how my
telnet is reported to be a "Go http package".


package main

import  "http"

func dump(w http.ResponseWriter, req *http.Request) {
        dump, _ := http.DumpRequest(req, false)
        w.Write(dump)
}

func main() {
        http.ListenAndServe(":6060", http.HandlerFunc(dump))
}


Connected to localhost.
Escape character is '^]'.
GET / HTTP/1.0

HTTP/1.0 200 OK
Date: Sun, 18 Sep 2011 18:39:54 GMT
Content-Type: text/plain; charset=utf-8

GET / HTTP/1.1
Host: 
User-Agent: Go http package

Connection closed by foreign host.

[masiulaniec]

Comment 1:

It's also making up HTTP protocol version.

[bradfitz]

Comment 2:

Accepted, but this is mostly a documentation problem more than a bug.
Request.Write is about what you'd write if you were sending this request to another host
as a client, not about preserving the exact bytes you got in.  (hence the HTTP/1.1
version, for example)  The blank host is actually required for HTTP/1.1, if I recall
correctly.  The User-Agent perhaps should be added by the http.Client, rather than
DumpRequest.  I'll think about that part.

Owner changed to @bradfitz.

Status changed to Accepted.

[rsc]

Comment 3:

I think DumpRequest should not add these lines if possible.
That might mean making it a little more than a wrapper
around Request.Write.

[bradfitz]

Comment 4:

Oh, right, this is about DumpRequest more than Request.Write.
I think I've used DumpRequest once ever, but looking at the docs now, and rsc's comment
#3, it's unclear what this should mean:
// DumpRequest returns the wire representation of req
Does that mean the incoming wire representation (what we just got in a Server Handler?)
or the outgoing representation (what a client would send?)
DumpRequest already takes one somewhat ugly bool parameter.  It'd be really gross to add
a second.
Arguably the biggest feature of DumpRequest is its Request.Body saving/cloning. 
Otherwise you could just use Request.Write.  So I assume that this function was intended
to show the *outgoing* wire representation.
Should we rename it to DumpClientRequest and add a DumpServerRequest?
Should we keep the "body bool" parameter or make the client do that?  Perhaps exposing
drainBody as an *http.DumpBodySaver type?  Seems like too much package noise and too
much abstraction for a debug function.
I'm happy to do whatever if people have opinions.

[rsc]

Comment 5:

I'd prefer to just leave the debug API as it is
and fix DumpRequest.  Because it is "dump",
it should not add or remove things, just print
what's there.
Russ

[bradfitz]

Comment 6:

http://golang.org/cl/5043051

[bradfitz]

Comment 7:

This issue was closed by revision 6b6cb72.

Status changed to Fixed.