http: prevent DumpRequest from adding implicit headers

Fixes #2272

R=rsc
CC=golang-dev
https://golang.org/cl/5043051

3 files changed, 162 insertions, 104 deletions

diff --git a/src/pkg/http/dump.go b/src/pkg/http/dump.go
index 358980f7ca..f78df57710 100644
--- a/src/pkg/http/dump.go
+++ b/src/pkg/http/dump.go
@@ -44,7 +44,7 @@ func DumpRequest(req *Request, body bool) (dump []byte, err os.Error) {
 			return
 		}
 	}
-	err = req.Write(&b)
+	err = req.dumpWrite(&b)
 	req.Body = save
 	if err != nil {
 		return
diff --git a/src/pkg/http/request.go b/src/pkg/http/request.go
index ed4114b549..dc344ca005 100644
--- a/src/pkg/http/request.go
+++ b/src/pkg/http/request.go
@@ -64,13 +64,20 @@ func (e *badStringError) String() string { return fmt.Sprintf("%s %q", e.what, e
 
 // Headers that Request.Write handles itself and should be skipped.
 var reqWriteExcludeHeader = map[string]bool{
-	"Host":              true,
+	"Host":              true, // not in Header map anyway
 	"User-Agent":        true,
 	"Content-Length":    true,
 	"Transfer-Encoding": true,
 	"Trailer":           true,
 }
 
+var reqWriteExcludeHeaderDump = map[string]bool{
+	"Host":              true, // not in Header map anyway
+	"Content-Length":    true,
+	"Transfer-Encoding": true,
+	"Trailer":           true,
+}
+
 // A Request represents a parsed HTTP request header.
 type Request struct {
 	Method string   // GET, POST, PUT, etc.
@@ -283,6 +290,53 @@ func (req *Request) WriteProxy(w io.Writer) os.Error {
 	return req.write(w, true)
 }
 
+func (req *Request) dumpWrite(w io.Writer) os.Error {
+	urlStr := req.RawURL
+	if urlStr == "" {
+		urlStr = valueOrDefault(req.URL.EncodedPath(), "/")
+		if req.URL.RawQuery != "" {
+			urlStr += "?" + req.URL.RawQuery
+		}
+	}
+
+	bw := bufio.NewWriter(w)
+	fmt.Fprintf(bw, "%s %s HTTP/%d.%d\r\n", valueOrDefault(req.Method, "GET"), urlStr,
+		req.ProtoMajor, req.ProtoMinor)
+
+	host := req.Host
+	if host == "" && req.URL != nil {
+		host = req.URL.Host
+	}
+	if host != "" {
+		fmt.Fprintf(bw, "Host: %s\r\n", host)
+	}
+
+	// Process Body,ContentLength,Close,Trailer
+	tw, err := newTransferWriter(req)
+	if err != nil {
+		return err
+	}
+	err = tw.WriteHeader(bw)
+	if err != nil {
+		return err
+	}
+
+	err = req.Header.WriteSubset(bw, reqWriteExcludeHeaderDump)
+	if err != nil {
+		return err
+	}
+
+	io.WriteString(bw, "\r\n")
+
+	// Write body and trailer
+	err = tw.WriteBody(bw)
+	if err != nil {
+		return err
+	}
+	bw.Flush()
+	return nil
+}
+
 func (req *Request) write(w io.Writer, usingProxy bool) os.Error {
 	host := req.Host
 	if host == "" {
diff --git a/src/pkg/http/requestwrite_test.go b/src/pkg/http/requestwrite_test.go
index 128ef776b8..a8cb75a597 100644
--- a/src/pkg/http/requestwrite_test.go
+++ b/src/pkg/http/requestwrite_test.go
@@ -16,17 +16,21 @@ import (
 )
 
 type reqWriteTest struct {
-	Req       Request
-	Body      interface{} // optional []byte or func() io.ReadCloser to populate Req.Body
-	Raw       string
-	RawProxy  string
-	WantError os.Error
+	Req  Request
+	Body interface{} // optional []byte or func() io.ReadCloser to populate Req.Body
+
+	// Any of these three may be empty to skip that test.
+	WantWrite string // Request.Write
+	WantProxy string // Request.WriteProxy
+	WantDump  string // DumpRequest
+
+	WantError os.Error // wanted error from Request.Write
 }
 
 var reqWriteTests = []reqWriteTest{
 	// HTTP/1.1 => chunked coding; no body; no trailer
 	{
-		Request{
+		Req: Request{
 			Method: "GET",
 			RawURL: "http://www.techcrunch.com/",
 			URL: &url.URL{
@@ -58,9 +62,7 @@ var reqWriteTests = []reqWriteTest{
 			Form:  map[string][]string{},
 		},
 
-		nil,
-
-		"GET http://www.techcrunch.com/ HTTP/1.1\r\n" +
+		WantWrite: "GET http://www.techcrunch.com/ HTTP/1.1\r\n" +
 			"Host: www.techcrunch.com\r\n" +
 			"User-Agent: Fake\r\n" +
 			"Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8\r\n" +
@@ -70,7 +72,7 @@ var reqWriteTests = []reqWriteTest{
 			"Keep-Alive: 300\r\n" +
 			"Proxy-Connection: keep-alive\r\n\r\n",
 
-		"GET http://www.techcrunch.com/ HTTP/1.1\r\n" +
+		WantProxy: "GET http://www.techcrunch.com/ HTTP/1.1\r\n" +
 			"Host: www.techcrunch.com\r\n" +
 			"User-Agent: Fake\r\n" +
 			"Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8\r\n" +
@@ -79,12 +81,10 @@ var reqWriteTests = []reqWriteTest{
 			"Accept-Language: en-us,en;q=0.5\r\n" +
 			"Keep-Alive: 300\r\n" +
 			"Proxy-Connection: keep-alive\r\n\r\n",
-
-		nil,
 	},
 	// HTTP/1.1 => chunked coding; body; empty trailer
 	{
-		Request{
+		Req: Request{
 			Method: "GET",
 			URL: &url.URL{
 				Scheme: "http",
@@ -97,25 +97,28 @@ var reqWriteTests = []reqWriteTest{
 			TransferEncoding: []string{"chunked"},
 		},
 
-		[]byte("abcdef"),
+		Body: []byte("abcdef"),
 
-		"GET /search HTTP/1.1\r\n" +
+		WantWrite: "GET /search HTTP/1.1\r\n" +
 			"Host: www.google.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Transfer-Encoding: chunked\r\n\r\n" +
 			chunk("abcdef") + chunk(""),
 
-		"GET http://www.google.com/search HTTP/1.1\r\n" +
+		WantProxy: "GET http://www.google.com/search HTTP/1.1\r\n" +
 			"Host: www.google.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Transfer-Encoding: chunked\r\n\r\n" +
 			chunk("abcdef") + chunk(""),
 
-		nil,
+		WantDump: "GET /search HTTP/1.1\r\n" +
+			"Host: www.google.com\r\n" +
+			"Transfer-Encoding: chunked\r\n\r\n" +
+			chunk("abcdef") + chunk(""),
 	},
 	// HTTP/1.1 POST => chunked coding; body; empty trailer
 	{
-		Request{
+		Req: Request{
 			Method: "POST",
 			URL: &url.URL{
 				Scheme: "http",
@@ -129,28 +132,26 @@ var reqWriteTests = []reqWriteTest{
 			TransferEncoding: []string{"chunked"},
 		},
 
-		[]byte("abcdef"),
+		Body: []byte("abcdef"),
 
-		"POST /search HTTP/1.1\r\n" +
+		WantWrite: "POST /search HTTP/1.1\r\n" +
 			"Host: www.google.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Connection: close\r\n" +
 			"Transfer-Encoding: chunked\r\n\r\n" +
 			chunk("abcdef") + chunk(""),
 
-		"POST http://www.google.com/search HTTP/1.1\r\n" +
+		WantProxy: "POST http://www.google.com/search HTTP/1.1\r\n" +
 			"Host: www.google.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Connection: close\r\n" +
 			"Transfer-Encoding: chunked\r\n\r\n" +
 			chunk("abcdef") + chunk(""),
-
-		nil,
 	},
 
 	// HTTP/1.1 POST with Content-Length, no chunking
 	{
-		Request{
+		Req: Request{
 			Method: "POST",
 			URL: &url.URL{
 				Scheme: "http",
@@ -164,9 +165,9 @@ var reqWriteTests = []reqWriteTest{
 			ContentLength: 6,
 		},
 
-		[]byte("abcdef"),
+		Body: []byte("abcdef"),
 
-		"POST /search HTTP/1.1\r\n" +
+		WantWrite: "POST /search HTTP/1.1\r\n" +
 			"Host: www.google.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Connection: close\r\n" +
@@ -174,20 +175,18 @@ var reqWriteTests = []reqWriteTest{
 			"\r\n" +
 			"abcdef",
 
-		"POST http://www.google.com/search HTTP/1.1\r\n" +
+		WantProxy: "POST http://www.google.com/search HTTP/1.1\r\n" +
 			"Host: www.google.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Connection: close\r\n" +
 			"Content-Length: 6\r\n" +
 			"\r\n" +
 			"abcdef",
-
-		nil,
 	},
 
 	// HTTP/1.1 POST with Content-Length in headers
 	{
-		Request{
+		Req: Request{
 			Method: "POST",
 			RawURL: "http://example.com/",
 			Host:   "example.com",
@@ -197,52 +196,46 @@ var reqWriteTests = []reqWriteTest{
 			ContentLength: 6,
 		},
 
-		[]byte("abcdef"),
+		Body: []byte("abcdef"),
 
-		"POST http://example.com/ HTTP/1.1\r\n" +
+		WantWrite: "POST http://example.com/ HTTP/1.1\r\n" +
 			"Host: example.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Content-Length: 6\r\n" +
 			"\r\n" +
 			"abcdef",
 
-		"POST http://example.com/ HTTP/1.1\r\n" +
+		WantProxy: "POST http://example.com/ HTTP/1.1\r\n" +
 			"Host: example.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Content-Length: 6\r\n" +
 			"\r\n" +
 			"abcdef",
-
-		nil,
 	},
 
 	// default to HTTP/1.1
 	{
-		Request{
+		Req: Request{
 			Method: "GET",
 			RawURL: "/search",
 			Host:   "www.google.com",
 		},
 
-		nil,
-
-		"GET /search HTTP/1.1\r\n" +
+		WantWrite: "GET /search HTTP/1.1\r\n" +
 			"Host: www.google.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"\r\n",
 
 		// Looks weird but RawURL overrides what WriteProxy would choose.
-		"GET /search HTTP/1.1\r\n" +
+		WantProxy: "GET /search HTTP/1.1\r\n" +
 			"Host: www.google.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"\r\n",
-
-		nil,
 	},
 
 	// Request with a 0 ContentLength and a 0 byte body.
 	{
-		Request{
+		Req: Request{
 			Method:        "POST",
 			RawURL:        "/",
 			Host:          "example.com",
@@ -251,24 +244,22 @@ var reqWriteTests = []reqWriteTest{
 			ContentLength: 0, // as if unset by user
 		},
 
-		func() io.ReadCloser { return ioutil.NopCloser(io.LimitReader(strings.NewReader("xx"), 0)) },
+		Body: func() io.ReadCloser { return ioutil.NopCloser(io.LimitReader(strings.NewReader("xx"), 0)) },
 
-		"POST / HTTP/1.1\r\n" +
+		WantWrite: "POST / HTTP/1.1\r\n" +
 			"Host: example.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"\r\n",
 
-		"POST / HTTP/1.1\r\n" +
+		WantProxy: "POST / HTTP/1.1\r\n" +
 			"Host: example.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"\r\n",
-
-		nil,
 	},
 
 	// Request with a 0 ContentLength and a 1 byte body.
 	{
-		Request{
+		Req: Request{
 			Method:        "POST",
 			RawURL:        "/",
 			Host:          "example.com",
@@ -277,26 +268,24 @@ var reqWriteTests = []reqWriteTest{
 			ContentLength: 0, // as if unset by user
 		},
 
-		func() io.ReadCloser { return ioutil.NopCloser(io.LimitReader(strings.NewReader("xx"), 1)) },
+		Body: func() io.ReadCloser { return ioutil.NopCloser(io.LimitReader(strings.NewReader("xx"), 1)) },
 
-		"POST / HTTP/1.1\r\n" +
+		WantWrite: "POST / HTTP/1.1\r\n" +
 			"Host: example.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Transfer-Encoding: chunked\r\n\r\n" +
 			chunk("x") + chunk(""),
 
-		"POST / HTTP/1.1\r\n" +
+		WantProxy: "POST / HTTP/1.1\r\n" +
 			"Host: example.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Transfer-Encoding: chunked\r\n\r\n" +
 			chunk("x") + chunk(""),
-
-		nil,
 	},
 
 	// Request with a ContentLength of 10 but a 5 byte body.
 	{
-		Request{
+		Req: Request{
 			Method:        "POST",
 			RawURL:        "/",
 			Host:          "example.com",
@@ -304,18 +293,13 @@ var reqWriteTests = []reqWriteTest{
 			ProtoMinor:    1,
 			ContentLength: 10, // but we're going to send only 5 bytes
 		},
-
-		[]byte("12345"),
-
-		"", // ignored
-		"", // ignored
-
-		os.NewError("http: Request.ContentLength=10 with Body length 5"),
+		Body:      []byte("12345"),
+		WantError: os.NewError("http: Request.ContentLength=10 with Body length 5"),
 	},
 
 	// Request with a ContentLength of 4 but an 8 byte body.
 	{
-		Request{
+		Req: Request{
 			Method:        "POST",
 			RawURL:        "/",
 			Host:          "example.com",
@@ -323,18 +307,13 @@ var reqWriteTests = []reqWriteTest{
 			ProtoMinor:    1,
 			ContentLength: 4, // but we're going to try to send 8 bytes
 		},
-
-		[]byte("12345678"),
-
-		"", // ignored
-		"", // ignored
-
-		os.NewError("http: Request.ContentLength=4 with Body length 8"),
+		Body:      []byte("12345678"),
+		WantError: os.NewError("http: Request.ContentLength=4 with Body length 8"),
 	},
 
 	// Request with a 5 ContentLength and nil body.
 	{
-		Request{
+		Req: Request{
 			Method:        "POST",
 			RawURL:        "/",
 			Host:          "example.com",
@@ -342,22 +321,30 @@ var reqWriteTests = []reqWriteTest{
 			ProtoMinor:    1,
 			ContentLength: 5, // but we'll omit the body
 		},
+		WantError: os.NewError("http: Request.ContentLength=5 with nil Body"),
+	},
 
-		nil, // missing body
-
-		"POST / HTTP/1.1\r\n" +
-			"Host: example.com\r\n" +
-			"User-Agent: Go http package\r\n" +
-			"Content-Length: 5\r\n\r\n" +
-			"",
+	// Verify that DumpRequest preserves the HTTP version number, doesn't add a Host,
+	// and doesn't add a User-Agent.
+	{
+		Req: Request{
+			Method:     "GET",
+			RawURL:     "/foo",
+			ProtoMajor: 1,
+			ProtoMinor: 0,
+			Header: Header{
+				"X-Foo": []string{"X-Bar"},
+			},
+		},
 
-		"POST / HTTP/1.1\r\n" +
-			"Host: example.com\r\n" +
-			"User-Agent: Go http package\r\n" +
-			"Content-Length: 5\r\n\r\n" +
-			"",
+		// We can dump it:
+		WantDump: "GET /foo HTTP/1.0\r\n" +
+			"X-Foo: X-Bar\r\n\r\n",
 
-		os.NewError("http: Request.ContentLength=5 with nil Body"),
+		// .. but we can't call Request.Write on it, due to its lack of Host header.
+		// TODO(bradfitz): there might be an argument to allow this, but for now I'd
+		// rather let HTTP/1.0 continue to die.
+		WantError: os.NewError("http: Request.Write on Request with no Host or URL set"),
 	},
 }
 
@@ -366,6 +353,9 @@ func TestRequestWrite(t *testing.T) {
 		tt := &reqWriteTests[i]
 
 		setBody := func() {
+			if tt.Body == nil {
+				return
+			}
 			switch b := tt.Body.(type) {
 			case []byte:
 				tt.Req.Body = ioutil.NopCloser(bytes.NewBuffer(b))
@@ -373,12 +363,11 @@ func TestRequestWrite(t *testing.T) {
 				tt.Req.Body = b()
 			}
 		}
-		if tt.Body != nil {
-			setBody()
-		}
+		setBody()
 		if tt.Req.Header == nil {
 			tt.Req.Header = make(Header)
 		}
+
 		var braw bytes.Buffer
 		err := tt.Req.Write(&braw)
 		if g, e := fmt.Sprintf("%v", err), fmt.Sprintf("%v", tt.WantError); g != e {
@@ -389,25 +378,40 @@ func TestRequestWrite(t *testing.T) {
 			continue
 		}
 
-		sraw := braw.String()
-		if sraw != tt.Raw {
-			t.Errorf("Test %d, expecting:\n%s\nGot:\n%s\n", i, tt.Raw, sraw)
-			continue
+		if tt.WantWrite != "" {
+			sraw := braw.String()
+			if sraw != tt.WantWrite {
+				t.Errorf("Test %d, expecting:\n%s\nGot:\n%s\n", i, tt.WantWrite, sraw)
+				continue
+			}
 		}
 
-		if tt.Body != nil {
+		if tt.WantProxy != "" {
 			setBody()
+			var praw bytes.Buffer
+			err = tt.Req.WriteProxy(&praw)
+			if err != nil {
+				t.Errorf("WriteProxy #%d: %s", i, err)
+				continue
+			}
+			sraw := praw.String()
+			if sraw != tt.WantProxy {
+				t.Errorf("Test Proxy %d, expecting:\n%s\nGot:\n%s\n", i, tt.WantProxy, sraw)
+				continue
+			}
 		}
-		var praw bytes.Buffer
-		err = tt.Req.WriteProxy(&praw)
-		if err != nil {
-			t.Errorf("error writing #%d: %s", i, err)
-			continue
-		}
-		sraw = praw.String()
-		if sraw != tt.RawProxy {
-			t.Errorf("Test Proxy %d, expecting:\n%s\nGot:\n%s\n", i, tt.RawProxy, sraw)
-			continue
+
+		if tt.WantDump != "" {
+			setBody()
+			dump, err := DumpRequest(&tt.Req, true)
+			if err != nil {
+				t.Errorf("DumpRequest #%d: %s", i, err)
+				continue
+			}
+			if string(dump) != tt.WantDump {
+				t.Errorf("DumpRequest %d, expecting:\n%s\nGot:\n%s\n", i, tt.WantDump, string(dump))
+				continue
+			}
 		}
 	}
 }