diff --git a/lib/go/http/request.go b/lib/go/http/request.go index b88689988..3918d5d0d 100644 --- a/lib/go/http/request.go +++ b/lib/go/http/request.go @@ -508,6 +508,8 @@ func ReadRequest(b *bufio.Reader) (req *Request, err os.Error) { return nil, err } + cleanURLForHTTPRequest(req.URL) + // Subsequent lines: Key: value. nheader := 0 req.Header = make(map[string]string) diff --git a/lib/go/http/serve_test.go b/lib/go/http/serve_test.go index 43e1b93a5..053d6dca4 100644 --- a/lib/go/http/serve_test.go +++ b/lib/go/http/serve_test.go @@ -7,7 +7,9 @@ package http import ( + "bufio" "bytes" + "io" "os" "net" "testing" @@ -133,3 +135,86 @@ func TestConsumingBodyOnNextConn(t *testing.T) { t.Errorf("Serve returned %q; expected EOF", serveerr) } } + +type responseWriterMethodCall struct { + method string + headerKey, headerValue string // if method == "SetHeader" + bytesWritten []byte // if method == "Write" + responseCode int // if method == "WriteHeader" +} + +type recordingResponseWriter struct { + log []*responseWriterMethodCall +} + +func (rw *recordingResponseWriter) RemoteAddr() string { + return "1.2.3.4" +} + +func (rw *recordingResponseWriter) UsingTLS() bool { + return false +} + +func (rw *recordingResponseWriter) SetHeader(k, v string) { + rw.log = append(rw.log, &responseWriterMethodCall{method: "SetHeader", headerKey: k, headerValue: v}) +} + +func (rw *recordingResponseWriter) Write(buf []byte) (int, os.Error) { + rw.log = append(rw.log, &responseWriterMethodCall{method: "Write", bytesWritten: buf}) + return len(buf), nil +} + +func (rw *recordingResponseWriter) WriteHeader(code int) { + rw.log = append(rw.log, &responseWriterMethodCall{method: "WriteHeader", responseCode: code}) +} + +func (rw *recordingResponseWriter) Flush() { + rw.log = append(rw.log, &responseWriterMethodCall{method: "Flush"}) +} + +func (rw *recordingResponseWriter) Hijack() (io.ReadWriteCloser, *bufio.ReadWriter, os.Error) { + panic("Not supported") +} + +// Tests for http://code.google.com/p/go/issues/detail?id=900 +func TestMuxRedirectLeadingSlashes(t *testing.T) { + paths := []string{"//foo.txt", "///foo.txt", "/../../foo.txt"} + for _, path := range paths { + req, err := ReadRequest(bufio.NewReader(bytes.NewBufferString("GET " + path + " HTTP/1.1\r\nHost: test\r\n\r\n"))) + if err != nil { + t.Errorf("%s", err) + } + mux := NewServeMux() + resp := new(recordingResponseWriter) + resp.log = make([]*responseWriterMethodCall, 0) + + mux.ServeHTTP(resp, req) + + dumpLog := func() { + t.Logf("For path %q:", path) + for _, call := range resp.log { + t.Logf("Got call: %s, header=%s, value=%s, buf=%q, code=%d", call.method, + call.headerKey, call.headerValue, call.bytesWritten, call.responseCode) + } + } + + if len(resp.log) != 2 { + dumpLog() + t.Errorf("expected 2 calls to response writer; got %d", len(resp.log)) + return + } + + if resp.log[0].method != "SetHeader" || + resp.log[0].headerKey != "Location" || resp.log[0].headerValue != "/foo.txt" { + dumpLog() + t.Errorf("Expected SetHeader of Location to /foo.txt") + return + } + + if resp.log[1].method != "WriteHeader" || resp.log[1].responseCode != StatusMovedPermanently { + dumpLog() + t.Errorf("Expected WriteHeader of StatusMovedPermanently") + return + } + } +} diff --git a/lib/go/http/url.go b/lib/go/http/url.go index b878c009f..c666d01bc 100644 --- a/lib/go/http/url.go +++ b/lib/go/http/url.go @@ -400,7 +400,7 @@ func ParseURL(rawurl string) (url *URL, err os.Error) { goto Error } - if url.Scheme != "" && (len(path) == 0 || path[0] != '/') { + if url.Scheme != "" && !strings.HasPrefix(path, "/") { // RFC 2396: // Absolute URI (has scheme) with non-rooted path // is uninterpreted. It doesn't even have a ?query. @@ -420,7 +420,7 @@ func ParseURL(rawurl string) (url *URL, err os.Error) { } // Maybe path is //authority/path - if url.Scheme != "" && len(path) > 2 && path[0:2] == "//" { + if strings.HasPrefix(path, "//") && !strings.HasPrefix(path, "///") { url.RawAuthority, path = split(path[2:], '/', false) url.RawPath = url.RawPath[2+len(url.RawAuthority):] } @@ -515,3 +515,19 @@ func (url *URL) String() string { } return result } + +// cleanURLForRequest cleans URLs as parsed from ReadRequest. +// ReadRequest uses ParseURL which accepts a superset of URL formats +// which are valid for web requests (scheme-relative URLs, for example) +// Ideally ReadRequest should use a different parse function for HTTP +// serving context, but for now we'll just fix it up here. +func cleanURLForHTTPRequest(url *URL) { + if url.Scheme == "" && url.RawAuthority != "" { + prefix := "//" + url.RawAuthority + url.Host = "" + url.RawAuthority = "" + url.RawUserinfo = "" + url.Path = prefix + url.Path + url.RawPath = prefix + url.RawPath + } +} diff --git a/lib/go/http/url_test.go b/lib/go/http/url_test.go index 8198e5f3e..f1f8c605f 100644 --- a/lib/go/http/url_test.go +++ b/lib/go/http/url_test.go @@ -188,14 +188,48 @@ var urltests = []URLTest{ }, "", }, - // leading // without scheme shouldn't create an authority + // leading // without scheme should create an authority { "//foo", &URL{ - Raw: "//foo", - Scheme: "", - RawPath: "//foo", - Path: "//foo", + RawAuthority: "foo", + Raw: "//foo", + Host: "foo", + Scheme: "", + RawPath: "", + Path: "", + }, + "", + }, + // leading // without scheme, with userinfo, path, and query + { + "//user@foo/path?a=b", + &URL{ + Raw: "//user@foo/path?a=b", + RawAuthority: "user@foo", + RawUserinfo: "user", + Scheme: "", + RawPath: "/path?a=b", + Path: "/path", + RawQuery: "a=b", + Host: "foo", + }, + "", + }, + // Three leading slashes isn't an authority, but doesn't return an error. + // (We can't return an error, as this code is also used via + // ServeHTTP -> ReadRequest -> ParseURL, which is arguably a + // different URL parsing context, but currently shares the + // same codepath) + { + "///threeslashes", + &URL{ + RawAuthority: "", + Raw: "///threeslashes", + Host: "", + Scheme: "", + RawPath: "///threeslashes", + Path: "///threeslashes", }, "", }, @@ -272,7 +306,7 @@ var urlfragtests = []URLTest{ // more useful string for debugging than fmt's struct printer func ufmt(u *URL) string { - return fmt.Sprintf("%q, %q, %q, %q, %q, %q, %q, %q, %q", + return fmt.Sprintf("raw=%q, scheme=%q, rawpath=%q, auth=%q, userinfo=%q, host=%q, path=%q, rawq=%q, frag=%q", u.Raw, u.Scheme, u.RawPath, u.RawAuthority, u.RawUserinfo, u.Host, u.Path, u.RawQuery, u.Fragment) } @@ -507,3 +541,30 @@ func TestUnescapeUserinfo(t *testing.T) { } } } + +func TestCleanURLForHTTPRequest(t *testing.T) { + path := "//user@foo/bar/" + url, _ := ParseURL(path) + if url.RawAuthority != "user@foo" { + t.Errorf("Expected authority of 'foo'; got %q", url.RawAuthority) + } + if url.RawUserinfo != "user" { + t.Errorf("Expected userinfo of 'user'; got %q", url.RawUserinfo) + } + cleanURLForHTTPRequest(url) + if url.RawAuthority != "" { + t.Errorf("Expected blank authority.") + } + if url.RawUserinfo != "" { + t.Errorf("Expected blank userinfo.") + } + if url.Host != "" { + t.Errorf("Expected blank host.") + } + if url.RawPath != path { + t.Errorf("Expected path %q; got %q", path, url.RawPath) + } + if url.Path != path { + t.Errorf("Expected path %q; got %q", path, url.Path) + } +}