http: remove Request.RawURL

Its purpose is not only undocumented, it's also unknown (to me
and Russ, at least) and leads to complexity, bugs and
confusion.

R=golang-dev, adg, rsc
CC=golang-dev
https://golang.org/cl/5213043

8 files changed, 52 insertions, 85 deletions

diff --git a/src/pkg/http/cgi/child.go b/src/pkg/http/cgi/child.go
index 8d0eca8d55..bf14c04a84 100644
--- a/src/pkg/http/cgi/child.go
+++ b/src/pkg/http/cgi/child.go
@@ -93,20 +93,20 @@ func RequestFromMap(params map[string]string) (*http.Request, os.Error) {
 	if r.Host != "" {
 		// Hostname is provided, so we can reasonably construct a URL,
 		// even if we have to assume 'http' for the scheme.
-		r.RawURL = "http://" + r.Host + params["REQUEST_URI"]
-		url, err := url.Parse(r.RawURL)
+		rawurl := "http://" + r.Host + params["REQUEST_URI"]
+		url, err := url.Parse(rawurl)
 		if err != nil {
-			return nil, os.NewError("cgi: failed to parse host and REQUEST_URI into a URL: " + r.RawURL)
+			return nil, os.NewError("cgi: failed to parse host and REQUEST_URI into a URL: " + rawurl)
 		}
 		r.URL = url
 	}
 	// Fallback logic if we don't have a Host header or the URL
 	// failed to parse
 	if r.URL == nil {
-		r.RawURL = params["REQUEST_URI"]
-		url, err := url.Parse(r.RawURL)
+		uriStr := params["REQUEST_URI"]
+		url, err := url.Parse(uriStr)
 		if err != nil {
-			return nil, os.NewError("cgi: failed to parse REQUEST_URI into a URL: " + r.RawURL)
+			return nil, os.NewError("cgi: failed to parse REQUEST_URI into a URL: " + uriStr)
 		}
 		r.URL = url
 	}
diff --git a/src/pkg/http/cgi/child_test.go b/src/pkg/http/cgi/child_test.go
index eee043bc90..ec53ab851b 100644
--- a/src/pkg/http/cgi/child_test.go
+++ b/src/pkg/http/cgi/child_test.go
@@ -49,9 +49,6 @@ func TestRequest(t *testing.T) {
 	if g, e := req.Header.Get("Foo-Bar"), "baz"; e != g {
 		t.Errorf("expected Foo-Bar %q; got %q", e, g)
 	}
-	if g, e := req.RawURL, "http://example.com/path?a=b"; e != g {
-		t.Errorf("expected RawURL %q; got %q", e, g)
-	}
 	if g, e := req.URL.String(), "http://example.com/path?a=b"; e != g {
 		t.Errorf("expected URL %q; got %q", e, g)
 	}
@@ -81,9 +78,6 @@ func TestRequestWithoutHost(t *testing.T) {
 	if err != nil {
 		t.Fatalf("RequestFromMap: %v", err)
 	}
-	if g, e := req.RawURL, "/path?a=b"; e != g {
-		t.Errorf("expected RawURL %q; got %q", e, g)
-	}
 	if req.URL == nil {
 		t.Fatalf("unexpected nil URL")
 	}
diff --git a/src/pkg/http/cgi/host.go b/src/pkg/http/cgi/host.go
index 1d63821416..9ea4c9d8bf 100644
--- a/src/pkg/http/cgi/host.go
+++ b/src/pkg/http/cgi/host.go
@@ -322,7 +322,6 @@ func (h *Handler) handleInternalRedirect(rw http.ResponseWriter, req *http.Reque
 	newReq := &http.Request{
 		Method:     "GET",
 		URL:        url,
-		RawURL:     path,
 		Proto:      "HTTP/1.1",
 		ProtoMajor: 1,
 		ProtoMinor: 1,
diff --git a/src/pkg/http/readrequest_test.go b/src/pkg/http/readrequest_test.go
index f6dc99e2e0..6d9042aceb 100644
--- a/src/pkg/http/readrequest_test.go
+++ b/src/pkg/http/readrequest_test.go
@@ -40,7 +40,6 @@ var reqTests = []reqTest{
 
 		&Request{
 			Method: "GET",
-			RawURL: "http://www.techcrunch.com/",
 			URL: &url.URL{
 				Raw:          "http://www.techcrunch.com/",
 				Scheme:       "http",
@@ -83,7 +82,6 @@ var reqTests = []reqTest{
 
 		&Request{
 			Method: "GET",
-			RawURL: "/",
 			URL: &url.URL{
 				Raw:     "/",
 				Path:    "/",
@@ -110,7 +108,6 @@ var reqTests = []reqTest{
 
 		&Request{
 			Method: "GET",
-			RawURL: "//user@host/is/actually/a/path/",
 			URL: &url.URL{
 				Raw:          "//user@host/is/actually/a/path/",
 				Scheme:       "",
diff --git a/src/pkg/http/request.go b/src/pkg/http/request.go
index dc344ca005..4f555ff575 100644
--- a/src/pkg/http/request.go
+++ b/src/pkg/http/request.go
@@ -80,9 +80,8 @@ var reqWriteExcludeHeaderDump = map[string]bool{
 
 // A Request represents a parsed HTTP request header.
 type Request struct {
-	Method string   // GET, POST, PUT, etc.
-	RawURL string   // The raw URL given in the request.
-	URL    *url.URL // Parsed URL.
+	Method string // GET, POST, PUT, etc.
+	URL    *url.URL
 
 	// The protocol version for incoming requests.
 	// Outgoing requests always use HTTP/1.1.
@@ -265,7 +264,7 @@ const defaultUserAgent = "Go http package"
 // Write writes an HTTP/1.1 request -- header and body -- in wire format.
 // This method consults the following fields of req:
 //	Host
-//	RawURL, if non-empty, or else URL
+//	URL
 //	Method (defaults to "GET")
 //	Header
 //	ContentLength
@@ -282,21 +281,18 @@ func (req *Request) Write(w io.Writer) os.Error {
 // WriteProxy is like Write but writes the request in the form
 // expected by an HTTP proxy.  In particular, WriteProxy writes the
 // initial Request-URI line of the request with an absolute URI, per
-// section 5.1.2 of RFC 2616, including the scheme and host.  If
-// req.RawURL is non-empty, WriteProxy uses it unchanged.  In either
-// case, WriteProxy also writes a Host header, using either req.Host
-// or req.URL.Host.
+// section 5.1.2 of RFC 2616, including the scheme and host. In
+// either case, WriteProxy also writes a Host header, using either
+// req.Host or req.URL.Host.
 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
-		}
+	// TODO(bradfitz): RawPath here?
+	urlStr := valueOrDefault(req.URL.EncodedPath(), "/")
+	if req.URL.RawQuery != "" {
+		urlStr += "?" + req.URL.RawQuery
 	}
 
 	bw := bufio.NewWriter(w)
@@ -346,9 +342,12 @@ func (req *Request) write(w io.Writer, usingProxy bool) os.Error {
 		host = req.URL.Host
 	}
 
-	urlStr := req.RawURL
+	urlStr := req.URL.RawPath
+	if strings.HasPrefix(urlStr, "?") {
+		urlStr = "/" + urlStr // Issue 2344
+	}
 	if urlStr == "" {
-		urlStr = valueOrDefault(req.URL.EncodedPath(), "/")
+		urlStr = valueOrDefault(req.URL.RawPath, valueOrDefault(req.URL.EncodedPath(), "/"))
 		if req.URL.RawQuery != "" {
 			urlStr += "?" + req.URL.RawQuery
 		}
@@ -359,6 +358,7 @@ func (req *Request) write(w io.Writer, usingProxy bool) os.Error {
 			urlStr = req.URL.Scheme + "://" + host + urlStr
 		}
 	}
+	// TODO(bradfitz): escape at least newlines in urlStr?
 
 	bw := bufio.NewWriter(w)
 	fmt.Fprintf(bw, "%s %s HTTP/1.1\r\n", valueOrDefault(req.Method, "GET"), urlStr)
@@ -598,13 +598,14 @@ func ReadRequest(b *bufio.Reader) (req *Request, err os.Error) {
 	if f = strings.SplitN(s, " ", 3); len(f) < 3 {
 		return nil, &badStringError{"malformed HTTP request", s}
 	}
-	req.Method, req.RawURL, req.Proto = f[0], f[1], f[2]
+	var rawurl string
+	req.Method, rawurl, req.Proto = f[0], f[1], f[2]
 	var ok bool
 	if req.ProtoMajor, req.ProtoMinor, ok = ParseHTTPVersion(req.Proto); !ok {
 		return nil, &badStringError{"malformed HTTP version", req.Proto}
 	}
 
-	if req.URL, err = url.ParseRequest(req.RawURL); err != nil {
+	if req.URL, err = url.ParseRequest(rawurl); err != nil {
 		return nil, err
 	}
 
diff --git a/src/pkg/http/requestwrite_test.go b/src/pkg/http/requestwrite_test.go
index 8c29c44f49..194f6dd213 100644
--- a/src/pkg/http/requestwrite_test.go
+++ b/src/pkg/http/requestwrite_test.go
@@ -32,7 +32,6 @@ var reqWriteTests = []reqWriteTest{
 	{
 		Req: Request{
 			Method: "GET",
-			RawURL: "http://www.techcrunch.com/",
 			URL: &url.URL{
 				Raw:          "http://www.techcrunch.com/",
 				Scheme:       "http",
@@ -188,7 +187,7 @@ var reqWriteTests = []reqWriteTest{
 	{
 		Req: Request{
 			Method: "POST",
-			RawURL: "http://example.com/",
+			URL:    mustParseURL("http://example.com/"),
 			Host:   "example.com",
 			Header: Header{
 				"Content-Length": []string{"10"}, // ignored
@@ -198,14 +197,14 @@ var reqWriteTests = []reqWriteTest{
 
 		Body: []byte("abcdef"),
 
-		WantWrite: "POST http://example.com/ HTTP/1.1\r\n" +
+		WantWrite: "POST / 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",
 
-		WantProxy: "POST http://example.com/ HTTP/1.1\r\n" +
+		WantProxy: "POST / HTTP/1.1\r\n" +
 			"Host: example.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"Content-Length: 6\r\n" +
@@ -217,7 +216,7 @@ var reqWriteTests = []reqWriteTest{
 	{
 		Req: Request{
 			Method: "GET",
-			RawURL: "/search",
+			URL:    mustParseURL("/search"),
 			Host:   "www.google.com",
 		},
 
@@ -225,19 +224,13 @@ var reqWriteTests = []reqWriteTest{
 			"Host: www.google.com\r\n" +
 			"User-Agent: Go http package\r\n" +
 			"\r\n",
-
-		// Looks weird but RawURL overrides what WriteProxy would choose.
-		WantProxy: "GET /search HTTP/1.1\r\n" +
-			"Host: www.google.com\r\n" +
-			"User-Agent: Go http package\r\n" +
-			"\r\n",
 	},
 
 	// Request with a 0 ContentLength and a 0 byte body.
 	{
 		Req: Request{
 			Method:        "POST",
-			RawURL:        "/",
+			URL:           mustParseURL("/"),
 			Host:          "example.com",
 			ProtoMajor:    1,
 			ProtoMinor:    1,
@@ -266,7 +259,7 @@ var reqWriteTests = []reqWriteTest{
 	{
 		Req: Request{
 			Method:        "POST",
-			RawURL:        "/",
+			URL:           mustParseURL("/"),
 			Host:          "example.com",
 			ProtoMajor:    1,
 			ProtoMinor:    1,
@@ -292,7 +285,7 @@ var reqWriteTests = []reqWriteTest{
 	{
 		Req: Request{
 			Method:        "POST",
-			RawURL:        "/",
+			URL:           mustParseURL("/"),
 			Host:          "example.com",
 			ProtoMajor:    1,
 			ProtoMinor:    1,
@@ -306,7 +299,7 @@ var reqWriteTests = []reqWriteTest{
 	{
 		Req: Request{
 			Method:        "POST",
-			RawURL:        "/",
+			URL:           mustParseURL("/"),
 			Host:          "example.com",
 			ProtoMajor:    1,
 			ProtoMinor:    1,
@@ -320,7 +313,7 @@ var reqWriteTests = []reqWriteTest{
 	{
 		Req: Request{
 			Method:        "POST",
-			RawURL:        "/",
+			URL:           mustParseURL("/"),
 			Host:          "example.com",
 			ProtoMajor:    1,
 			ProtoMinor:    1,
@@ -334,7 +327,7 @@ var reqWriteTests = []reqWriteTest{
 	{
 		Req: Request{
 			Method:     "GET",
-			RawURL:     "/foo",
+			URL:        mustParseURL("/foo"),
 			ProtoMajor: 1,
 			ProtoMinor: 0,
 			Header: Header{
@@ -349,7 +342,10 @@ var reqWriteTests = []reqWriteTest{
 		// .. 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"),
+		WantWrite: "GET /foo HTTP/1.1\r\n" +
+			"Host: \r\n" +
+			"User-Agent: Go http package\r\n" +
+			"X-Foo: X-Bar\r\n\r\n",
 	},
 }
 
@@ -464,3 +460,11 @@ func TestRequestWriteClosesBody(t *testing.T) {
 func chunk(s string) string {
 	return fmt.Sprintf("%x\r\n%s\r\n", len(s), s)
 }
+
+func mustParseURL(s string) *url.URL {
+	u, err := url.Parse(s)
+	if err != nil {
+		panic(fmt.Sprintf("Error parsing URL %q: %v", s, err))
+	}
+	return u
+}
diff --git a/src/pkg/http/transport.go b/src/pkg/http/transport.go
index 8ac78324a3..a580e1f7cb 100644
--- a/src/pkg/http/transport.go
+++ b/src/pkg/http/transport.go
@@ -103,9 +103,7 @@ func ProxyURL(fixedURL *url.URL) func(*Request) (*url.URL, os.Error) {
 // RoundTrip implements the RoundTripper interface.
 func (t *Transport) RoundTrip(req *Request) (resp *Response, err os.Error) {
 	if req.URL == nil {
-		if req.URL, err = url.Parse(req.RawURL); err != nil {
-			return
-		}
+		return nil, os.NewError("http: nil Request.URL")
 	}
 	if req.URL.Scheme != "http" && req.URL.Scheme != "https" {
 		t.lk.Lock()
@@ -315,7 +313,7 @@ func (t *Transport) getConn(cm *connectMethod) (*persistConn, os.Error) {
 	case cm.targetScheme == "https":
 		connectReq := &Request{
 			Method: "CONNECT",
-			RawURL: cm.targetAddr,
+			URL:    &url.URL{RawPath: cm.targetAddr},
 			Host:   cm.targetAddr,
 			Header: make(Header),
 		}
diff --git a/src/pkg/http/transport_test.go b/src/pkg/http/transport_test.go
index b9ae7a3685..a5dfe5ee3c 100644
--- a/src/pkg/http/transport_test.go
+++ b/src/pkg/http/transport_test.go
@@ -78,7 +78,7 @@ func TestTransportConnectionCloseOnResponse(t *testing.T) {
 		fetch := func(n int) string {
 			req := new(Request)
 			var err os.Error
-			req.URL, err = url.Parse(ts.URL + fmt.Sprintf("?close=%v", connectionClose))
+			req.URL, err = url.Parse(ts.URL + fmt.Sprintf("/?close=%v", connectionClose))
 			if err != nil {
 				t.Fatalf("URL parse error: %v", err)
 			}
@@ -362,32 +362,6 @@ func TestTransportHeadChunkedResponse(t *testing.T) {
 	}
 }
 
-func TestTransportNilURL(t *testing.T) {
-	ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) {
-		fmt.Fprintf(w, "Hi")
-	}))
-	defer ts.Close()
-
-	req := new(Request)
-	req.URL = nil // what we're actually testing
-	req.Method = "GET"
-	req.RawURL = ts.URL
-	req.Proto = "HTTP/1.1"
-	req.ProtoMajor = 1
-	req.ProtoMinor = 1
-	req.Header = make(Header)
-
-	tr := &Transport{}
-	res, err := tr.RoundTrip(req)
-	if err != nil {
-		t.Fatalf("unexpected RoundTrip error: %v", err)
-	}
-	body, err := ioutil.ReadAll(res.Body)
-	if g, e := string(body), "Hi"; g != e {
-		t.Fatalf("Expected response body of %q; got %q", e, g)
-	}
-}
-
 var roundTripTests = []struct {
 	accept       string
 	expectAccept string
@@ -484,7 +458,7 @@ func TestTransportGzip(t *testing.T) {
 		c := &Client{Transport: &Transport{}}
 
 		// First fetch something large, but only read some of it.
-		res, err := c.Get(ts.URL + "?body=large&chunked=" + chunked)
+		res, err := c.Get(ts.URL + "/?body=large&chunked=" + chunked)
 		if err != nil {
 			t.Fatalf("large get: %v", err)
 		}
@@ -504,7 +478,7 @@ func TestTransportGzip(t *testing.T) {
 		}
 
 		// Then something small.
-		res, err = c.Get(ts.URL + "?chunked=" + chunked)
+		res, err = c.Get(ts.URL + "/?chunked=" + chunked)
 		if err != nil {
 			t.Fatal(err)
 		}