qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-25 01:08:02 -07:00
Code Releases Activity

http: remove Request.RawURL

Browse Source 
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
This commit is contained in:
Brad Fitzpatrick 2011-10-12 11:48:25 -07:00
parent fefadcf51c
commit 36036781d5
10 changed files with 61 additions and 94 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/pkg/http/cgi/child.go
View 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
}

6
src/pkg/http/cgi/child_test.go
View 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")
}

1
src/pkg/http/cgi/host.go
View 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,

3
src/pkg/http/readrequest_test.go
View 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: "",

37
src/pkg/http/request.go
View 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
}

40
src/pkg/http/requestwrite_test.go
View 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
}

6
src/pkg/http/transport.go
View 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),
}

32
src/pkg/http/transport_test.go
View 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)
}

6
src/pkg/websocket/hixie_test.go
View 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)

12
src/pkg/websocket/hybi_test.go
View 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)