]> Cypherpunks repositories - gostls13.git/commitdiff
http: remove Request.RawURL
authorBrad Fitzpatrick <bradfitz@golang.org>
Wed, 12 Oct 2011 18:48:25 +0000 (11:48 -0700)
committerBrad Fitzpatrick <bradfitz@golang.org>
Wed, 12 Oct 2011 18:48:25 +0000 (11:48 -0700)
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

src/pkg/http/cgi/child.go
src/pkg/http/cgi/child_test.go
src/pkg/http/cgi/host.go
src/pkg/http/readrequest_test.go
src/pkg/http/request.go
src/pkg/http/requestwrite_test.go
src/pkg/http/transport.go
src/pkg/http/transport_test.go
src/pkg/websocket/hixie_test.go
src/pkg/websocket/hybi_test.go

index 8d0eca8d55bfafa8e740fe11d62c515c4201b7c4..bf14c04a843b0d3865525d34736c8ad66bd3caf6 100644 (file)
@@ -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
        }
index eee043bc90df4a9b9c024f299f9d68a48249b8a3..ec53ab851baaeeab8bc871baad2bc31e02eb8dba 100644 (file)
@@ -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")
        }
index 1d63821416ddd0e125248f460d50571321e55ff6..9ea4c9d8bf2ededd9393d1ad4372100b13678e05 100644 (file)
@@ -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,
index f6dc99e2e085d290417fe826863694b3bfcc11ba..6d9042aceb629d671d0404b3e8d9dfb7f977e0a9 100644 (file)
@@ -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:       "",
index dc344ca0057803b4f3eb4f338e42f01a8ddf94cc..4f555ff5753fcba7df42dc6c3f9a8d3622f90fa4 100644 (file)
@@ -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
        }
 
index 8c29c44f49820a48eaf2383c53b68931b25dbbe9..194f6dd213cf2157af66d76e58ca668e5693c25b 100644 (file)
@@ -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
+}
index 8ac78324a3824539791c7e76fa74bc95d6a43595..a580e1f7cb40a90e68a3a8139055c6628ad32a3d 100644 (file)
@@ -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),
                }
index b9ae7a36857caa6fd9f742abdc039939c1ccc1ee..a5dfe5ee3caf931a6db7ff80dae1f72dc582548a 100644 (file)
@@ -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)
                }
index a480b6608c5c36f1bdcf8fb96668a073fc9eea38..98a0de4d6f42cf4d5c151ce8cfa99aadf3655fa0 100644 (file)
@@ -72,13 +72,13 @@ Sec-WebSocket-Protocol: sample
        }
        req, err := http.ReadRequest(bufio.NewReader(b))
        if err != nil {
-               t.Errorf("read request: %v", err)
+               t.Fatalf("read request: %v", err)
        }
        if req.Method != "GET" {
                t.Errorf("request method expected GET, but got %q", req.Method)
        }
-       if req.RawURL != "/demo" {
-               t.Errorf("request path expected /demo, but got %q", req.RawURL)
+       if req.URL.Path != "/demo" {
+               t.Errorf("request path expected /demo, but got %q", req.URL.Path)
        }
        if req.Proto != "HTTP/1.1" {
                t.Errorf("request proto expected HTTP/1.1, but got %q", req.Proto)
index 0814c08015f21c7e2ee2df7d76ce11832eb3488d..71d1893b30172a96a38fd10432ff7d888aee7571 100644 (file)
@@ -63,13 +63,13 @@ Sec-WebSocket-Protocol: chat
        }
        req, err := http.ReadRequest(bufio.NewReader(b))
        if err != nil {
-               t.Errorf("read request: %v", err)
+               t.Fatalf("read request: %v", err)
        }
        if req.Method != "GET" {
                t.Errorf("request method expected GET, but got %q", req.Method)
        }
-       if req.RawURL != "/chat" {
-               t.Errorf("request path expected /chat, but got %q", req.RawURL)
+       if req.URL.Path != "/chat" {
+               t.Errorf("request path expected /chat, but got %q", req.URL.Path)
        }
        if req.Proto != "HTTP/1.1" {
                t.Errorf("request proto expected HTTP/1.1, but got %q", req.Proto)
@@ -125,13 +125,13 @@ Sec-WebSocket-Protocol: chat
        }
        req, err := http.ReadRequest(bufio.NewReader(b))
        if err != nil {
-               t.Errorf("read request: %v", err)
+               t.Fatalf("read request: %v", err)
        }
        if req.Method != "GET" {
                t.Errorf("request method expected GET, but got %q", req.Method)
        }
-       if req.RawURL != "/chat" {
-               t.Errorf("request path expected /demo, but got %q", req.RawURL)
+       if req.URL.Path != "/chat" {
+               t.Errorf("request path expected /demo, but got %q", req.URL.Path)
        }
        if req.Proto != "HTTP/1.1" {
                t.Errorf("request proto expected HTTP/1.1, but got %q", req.Proto)