diff options
Diffstat (limited to 'libgo/go/net/http')
26 files changed, 1310 insertions, 364 deletions
diff --git a/libgo/go/net/http/cgi/child.go b/libgo/go/net/http/cgi/child.go index 100b8b7..45fc2e5 100644 --- a/libgo/go/net/http/cgi/child.go +++ b/libgo/go/net/http/cgi/child.go @@ -100,10 +100,21 @@ func RequestFromMap(params map[string]string) (*http.Request, error) { uriStr += "?" + s } } + + // There's apparently a de-facto standard for this. + // http://docstore.mik.ua/orelly/linux/cgi/ch03_02.htm#ch03-35636 + if s := params["HTTPS"]; s == "on" || s == "ON" || s == "1" { + r.TLS = &tls.ConnectionState{HandshakeComplete: true} + } + if r.Host != "" { - // Hostname is provided, so we can reasonably construct a URL, - // even if we have to assume 'http' for the scheme. - rawurl := "http://" + r.Host + uriStr + // Hostname is provided, so we can reasonably construct a URL. + rawurl := r.Host + uriStr + if r.TLS == nil { + rawurl = "http://" + rawurl + } else { + rawurl = "https://" + rawurl + } url, err := url.Parse(rawurl) if err != nil { return nil, errors.New("cgi: failed to parse host and REQUEST_URI into a URL: " + rawurl) @@ -120,12 +131,6 @@ func RequestFromMap(params map[string]string) (*http.Request, error) { r.URL = url } - // There's apparently a de-facto standard for this. - // http://docstore.mik.ua/orelly/linux/cgi/ch03_02.htm#ch03-35636 - if s := params["HTTPS"]; s == "on" || s == "ON" || s == "1" { - r.TLS = &tls.ConnectionState{HandshakeComplete: true} - } - // Request.RemoteAddr has its port set by Go's standard http // server, so we do here too. We don't have one, though, so we // use a dummy one. diff --git a/libgo/go/net/http/cgi/child_test.go b/libgo/go/net/http/cgi/child_test.go index 74e0680..075d841 100644 --- a/libgo/go/net/http/cgi/child_test.go +++ b/libgo/go/net/http/cgi/child_test.go @@ -21,7 +21,6 @@ func TestRequest(t *testing.T) { "REQUEST_URI": "/path?a=b", "CONTENT_LENGTH": "123", "CONTENT_TYPE": "text/xml", - "HTTPS": "1", "REMOTE_ADDR": "5.6.7.8", } req, err := RequestFromMap(env) @@ -58,14 +57,37 @@ func TestRequest(t *testing.T) { if req.Trailer == nil { t.Errorf("unexpected nil Trailer") } - if req.TLS == nil { - t.Errorf("expected non-nil TLS") + if req.TLS != nil { + t.Errorf("expected nil TLS") } if e, g := "5.6.7.8:0", req.RemoteAddr; e != g { t.Errorf("RemoteAddr: got %q; want %q", g, e) } } +func TestRequestWithTLS(t *testing.T) { + env := map[string]string{ + "SERVER_PROTOCOL": "HTTP/1.1", + "REQUEST_METHOD": "GET", + "HTTP_HOST": "example.com", + "HTTP_REFERER": "elsewhere", + "REQUEST_URI": "/path?a=b", + "CONTENT_TYPE": "text/xml", + "HTTPS": "1", + "REMOTE_ADDR": "5.6.7.8", + } + req, err := RequestFromMap(env) + if err != nil { + t.Fatalf("RequestFromMap: %v", err) + } + if g, e := req.URL.String(), "https://example.com/path?a=b"; e != g { + t.Errorf("expected URL %q; got %q", e, g) + } + if req.TLS == nil { + t.Errorf("expected non-nil TLS") + } +} + func TestRequestWithoutHost(t *testing.T) { env := map[string]string{ "SERVER_PROTOCOL": "HTTP/1.1", diff --git a/libgo/go/net/http/client.go b/libgo/go/net/http/client.go index a34d47b..22f2e86 100644 --- a/libgo/go/net/http/client.go +++ b/libgo/go/net/http/client.go @@ -74,8 +74,8 @@ type RoundTripper interface { // authentication, or cookies. // // RoundTrip should not modify the request, except for - // consuming the Body. The request's URL and Header fields - // are guaranteed to be initialized. + // consuming and closing the Body. The request's URL and + // Header fields are guaranteed to be initialized. RoundTrip(*Request) (*Response, error) } @@ -161,7 +161,9 @@ func send(req *Request, t RoundTripper) (resp *Response, err error) { } if u := req.URL.User; u != nil { - req.Header.Set("Authorization", "Basic "+base64.URLEncoding.EncodeToString([]byte(u.String()))) + username := u.Username() + password, _ := u.Password() + req.Header.Set("Authorization", "Basic "+basicAuth(username, password)) } resp, err = t.RoundTrip(req) if err != nil { @@ -173,6 +175,16 @@ func send(req *Request, t RoundTripper) (resp *Response, err error) { return resp, nil } +// See 2 (end of page 4) http://www.ietf.org/rfc/rfc2617.txt +// "To receive authorization, the client sends the userid and password, +// separated by a single colon (":") character, within a base64 +// encoded string in the credentials." +// It is not meant to be urlencoded. +func basicAuth(username, password string) string { + auth := username + ":" + password + return base64.StdEncoding.EncodeToString([]byte(auth)) +} + // True if the specified HTTP status code is one for which the Get utility should // automatically redirect. func shouldRedirectGet(statusCode int) bool { @@ -335,6 +347,9 @@ func Post(url string, bodyType string, body io.Reader) (resp *Response, err erro // Post issues a POST to the specified URL. // // Caller should close resp.Body when done reading from it. +// +// If the provided body is also an io.Closer, it is closed after the +// body is successfully written to the server. func (c *Client) Post(url string, bodyType string, body io.Reader) (resp *Response, err error) { req, err := NewRequest("POST", url, body) if err != nil { diff --git a/libgo/go/net/http/client_test.go b/libgo/go/net/http/client_test.go index 73f1fe3..997d041 100644 --- a/libgo/go/net/http/client_test.go +++ b/libgo/go/net/http/client_test.go @@ -10,6 +10,7 @@ import ( "bytes" "crypto/tls" "crypto/x509" + "encoding/base64" "errors" "fmt" "io" @@ -665,6 +666,36 @@ func TestClientWithIncorrectTLSServerName(t *testing.T) { } } +// Test for golang.org/issue/5829; the Transport should respect TLSClientConfig.ServerName +// when not empty. +// +// tls.Config.ServerName (non-empty, set to "example.com") takes +// precedence over "some-other-host.tld" which previously incorrectly +// took precedence. We don't actually connect to (or even resolve) +// "some-other-host.tld", though, because of the Transport.Dial hook. +// +// The httptest.Server has a cert with "example.com" as its name. +func TestTransportUsesTLSConfigServerName(t *testing.T) { + defer afterTest(t) + ts := httptest.NewTLSServer(HandlerFunc(func(w ResponseWriter, r *Request) { + w.Write([]byte("Hello")) + })) + defer ts.Close() + + tr := newTLSTransport(t, ts) + tr.TLSClientConfig.ServerName = "example.com" // one of httptest's Server cert names + tr.Dial = func(netw, addr string) (net.Conn, error) { + return net.Dial(netw, ts.Listener.Addr().String()) + } + defer tr.CloseIdleConnections() + c := &Client{Transport: tr} + res, err := c.Get("https://some-other-host.tld/") + if err != nil { + t.Fatal(err) + } + res.Body.Close() +} + // Verify Response.ContentLength is populated. http://golang.org/issue/4126 func TestClientHeadContentLength(t *testing.T) { defer afterTest(t) @@ -700,3 +731,71 @@ func TestClientHeadContentLength(t *testing.T) { } } } + +func TestEmptyPasswordAuth(t *testing.T) { + defer afterTest(t) + gopher := "gopher" + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + auth := r.Header.Get("Authorization") + if strings.HasPrefix(auth, "Basic ") { + encoded := auth[6:] + decoded, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + t.Fatal(err) + } + expected := gopher + ":" + s := string(decoded) + if expected != s { + t.Errorf("Invalid Authorization header. Got %q, wanted %q", s, expected) + } + } else { + t.Errorf("Invalid auth %q", auth) + } + })) + defer ts.Close() + c := &Client{} + req, err := NewRequest("GET", ts.URL, nil) + if err != nil { + t.Fatal(err) + } + req.URL.User = url.User(gopher) + resp, err := c.Do(req) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() +} + +func TestBasicAuth(t *testing.T) { + defer afterTest(t) + tr := &recordingTransport{} + client := &Client{Transport: tr} + + url := "http://My%20User:My%20Pass@dummy.faketld/" + expected := "My User:My Pass" + client.Get(url) + + if tr.req.Method != "GET" { + t.Errorf("got method %q, want %q", tr.req.Method, "GET") + } + if tr.req.URL.String() != url { + t.Errorf("got URL %q, want %q", tr.req.URL.String(), url) + } + if tr.req.Header == nil { + t.Fatalf("expected non-nil request Header") + } + auth := tr.req.Header.Get("Authorization") + if strings.HasPrefix(auth, "Basic ") { + encoded := auth[6:] + decoded, err := base64.StdEncoding.DecodeString(encoded) + if err != nil { + t.Fatal(err) + } + s := string(decoded) + if expected != s { + t.Errorf("Invalid Authorization header. Got %q, wanted %q", s, expected) + } + } else { + t.Errorf("Invalid auth %q", auth) + } +} diff --git a/libgo/go/net/http/cookie.go b/libgo/go/net/http/cookie.go index 155b092..8b01c50 100644 --- a/libgo/go/net/http/cookie.go +++ b/libgo/go/net/http/cookie.go @@ -7,6 +7,8 @@ package http import ( "bytes" "fmt" + "log" + "net" "strconv" "strings" "time" @@ -139,12 +141,25 @@ func SetCookie(w ResponseWriter, cookie *Cookie) { // header (if other fields are set). func (c *Cookie) String() string { var b bytes.Buffer - fmt.Fprintf(&b, "%s=%s", sanitizeName(c.Name), sanitizeValue(c.Value)) + fmt.Fprintf(&b, "%s=%s", sanitizeCookieName(c.Name), sanitizeCookieValue(c.Value)) if len(c.Path) > 0 { - fmt.Fprintf(&b, "; Path=%s", sanitizeValue(c.Path)) + fmt.Fprintf(&b, "; Path=%s", sanitizeCookiePath(c.Path)) } if len(c.Domain) > 0 { - fmt.Fprintf(&b, "; Domain=%s", sanitizeValue(c.Domain)) + if validCookieDomain(c.Domain) { + // A c.Domain containing illegal characters is not + // sanitized but simply dropped which turns the cookie + // into a host-only cookie. A leading dot is okay + // but won't be sent. + d := c.Domain + if d[0] == '.' { + d = d[1:] + } + fmt.Fprintf(&b, "; Domain=%s", d) + } else { + log.Printf("net/http: invalid Cookie.Domain %q; dropping domain attribute", + c.Domain) + } } if c.Expires.Unix() > 0 { fmt.Fprintf(&b, "; Expires=%s", c.Expires.UTC().Format(time.RFC1123)) @@ -207,16 +222,122 @@ func readCookies(h Header, filter string) []*Cookie { return cookies } +// validCookieDomain returns wheter v is a valid cookie domain-value. +func validCookieDomain(v string) bool { + if isCookieDomainName(v) { + return true + } + if net.ParseIP(v) != nil && !strings.Contains(v, ":") { + return true + } + return false +} + +// isCookieDomainName returns whether s is a valid domain name or a valid +// domain name with a leading dot '.'. It is almost a direct copy of +// package net's isDomainName. +func isCookieDomainName(s string) bool { + if len(s) == 0 { + return false + } + if len(s) > 255 { + return false + } + + if s[0] == '.' { + // A cookie a domain attribute may start with a leading dot. + s = s[1:] + } + last := byte('.') + ok := false // Ok once we've seen a letter. + partlen := 0 + for i := 0; i < len(s); i++ { + c := s[i] + switch { + default: + return false + case 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z': + // No '_' allowed here (in contrast to package net). + ok = true + partlen++ + case '0' <= c && c <= '9': + // fine + partlen++ + case c == '-': + // Byte before dash cannot be dot. + if last == '.' { + return false + } + partlen++ + case c == '.': + // Byte before dot cannot be dot, dash. + if last == '.' || last == '-' { + return false + } + if partlen > 63 || partlen == 0 { + return false + } + partlen = 0 + } + last = c + } + if last == '-' || partlen > 63 { + return false + } + + return ok +} + var cookieNameSanitizer = strings.NewReplacer("\n", "-", "\r", "-") -func sanitizeName(n string) string { +func sanitizeCookieName(n string) string { return cookieNameSanitizer.Replace(n) } -var cookieValueSanitizer = strings.NewReplacer("\n", " ", "\r", " ", ";", " ") +// http://tools.ietf.org/html/rfc6265#section-4.1.1 +// cookie-value = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE ) +// cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E +// ; US-ASCII characters excluding CTLs, +// ; whitespace DQUOTE, comma, semicolon, +// ; and backslash +func sanitizeCookieValue(v string) string { + return sanitizeOrWarn("Cookie.Value", validCookieValueByte, v) +} + +func validCookieValueByte(b byte) bool { + return 0x20 < b && b < 0x7f && b != '"' && b != ',' && b != ';' && b != '\\' +} + +// path-av = "Path=" path-value +// path-value = <any CHAR except CTLs or ";"> +func sanitizeCookiePath(v string) string { + return sanitizeOrWarn("Cookie.Path", validCookiePathByte, v) +} -func sanitizeValue(v string) string { - return cookieValueSanitizer.Replace(v) +func validCookiePathByte(b byte) bool { + return 0x20 <= b && b < 0x7f && b != ';' +} + +func sanitizeOrWarn(fieldName string, valid func(byte) bool, v string) string { + ok := true + for i := 0; i < len(v); i++ { + if valid(v[i]) { + continue + } + log.Printf("net/http: invalid byte %q in %s; dropping invalid bytes", v[i], fieldName) + ok = false + break + } + if ok { + return v + } + buf := make([]byte, 0, len(v)) + for i := 0; i < len(v); i++ { + if b := v[i]; valid(b) { + buf = append(buf, b) + } + } + return string(buf) } func unquoteCookieValue(v string) string { diff --git a/libgo/go/net/http/cookie_test.go b/libgo/go/net/http/cookie_test.go index f84f739..11b01cc 100644 --- a/libgo/go/net/http/cookie_test.go +++ b/libgo/go/net/http/cookie_test.go @@ -26,12 +26,28 @@ var writeSetCookiesTests = []struct { }, { &Cookie{Name: "cookie-3", Value: "three", Domain: ".example.com"}, - "cookie-3=three; Domain=.example.com", + "cookie-3=three; Domain=example.com", }, { &Cookie{Name: "cookie-4", Value: "four", Path: "/restricted/"}, "cookie-4=four; Path=/restricted/", }, + { + &Cookie{Name: "cookie-5", Value: "five", Domain: "wrong;bad.abc"}, + "cookie-5=five", + }, + { + &Cookie{Name: "cookie-6", Value: "six", Domain: "bad-.abc"}, + "cookie-6=six", + }, + { + &Cookie{Name: "cookie-7", Value: "seven", Domain: "127.0.0.1"}, + "cookie-7=seven; Domain=127.0.0.1", + }, + { + &Cookie{Name: "cookie-8", Value: "eight", Domain: "::1"}, + "cookie-8=eight", + }, } func TestWriteSetCookies(t *testing.T) { @@ -226,3 +242,34 @@ func TestReadCookies(t *testing.T) { } } } + +func TestCookieSanitizeValue(t *testing.T) { + tests := []struct { + in, want string + }{ + {"foo", "foo"}, + {"foo bar", "foobar"}, + {"\x00\x7e\x7f\x80", "\x7e"}, + {`"withquotes"`, "withquotes"}, + } + for _, tt := range tests { + if got := sanitizeCookieValue(tt.in); got != tt.want { + t.Errorf("sanitizeCookieValue(%q) = %q; want %q", tt.in, got, tt.want) + } + } +} + +func TestCookieSanitizePath(t *testing.T) { + tests := []struct { + in, want string + }{ + {"/path", "/path"}, + {"/path with space/", "/path with space/"}, + {"/just;no;semicolon\x00orstuff/", "/justnosemicolonorstuff/"}, + } + for _, tt := range tests { + if got := sanitizeCookiePath(tt.in); got != tt.want { + t.Errorf("sanitizeCookiePath(%q) = %q; want %q", tt.in, got, tt.want) + } + } +} diff --git a/libgo/go/net/http/cookiejar/jar.go b/libgo/go/net/http/cookiejar/jar.go index 5977d48..389ab58 100644 --- a/libgo/go/net/http/cookiejar/jar.go +++ b/libgo/go/net/http/cookiejar/jar.go @@ -142,7 +142,7 @@ func (e *entry) pathMatch(requestPath string) bool { return false } -// hasDotSuffix returns whether s ends in "."+suffix. +// hasDotSuffix reports whether s ends in "."+suffix. func hasDotSuffix(s, suffix string) bool { return len(s) > len(suffix) && s[len(s)-len(suffix)-1] == '.' && s[len(s)-len(suffix):] == suffix } @@ -316,7 +316,7 @@ func canonicalHost(host string) (string, error) { return toASCII(host) } -// hasPort returns whether host contains a port number. host may be a host +// hasPort reports whether host contains a port number. host may be a host // name, an IPv4 or an IPv6 address. func hasPort(host string) bool { colons := strings.Count(host, ":") @@ -357,7 +357,7 @@ func jarKey(host string, psl PublicSuffixList) string { return host[prevDot+1:] } -// isIP returns whether host is an IP address. +// isIP reports whether host is an IP address. func isIP(host string) bool { return net.ParseIP(host) != nil } @@ -380,7 +380,7 @@ func defaultPath(path string) string { // is compared to c.Expires to determine deletion of c. defPath and host are the // default-path and the canonical host name of the URL c was received from. // -// remove is whether the jar should delete this cookie, as it has already +// remove records whether the jar should delete this cookie, as it has already // expired with respect to now. In this case, e may be incomplete, but it will // be valid to call e.id (which depends on e's Name, Domain and Path). // diff --git a/libgo/go/net/http/doc.go b/libgo/go/net/http/doc.go index b6ae8b8..b1216e8 100644 --- a/libgo/go/net/http/doc.go +++ b/libgo/go/net/http/doc.go @@ -5,7 +5,7 @@ /* Package http provides HTTP client and server implementations. -Get, Head, Post, and PostForm make HTTP requests: +Get, Head, Post, and PostForm make HTTP (or HTTPS) requests: resp, err := http.Get("http://example.com/") ... diff --git a/libgo/go/net/http/example_test.go b/libgo/go/net/http/example_test.go index bc60df7..88b97d9 100644 --- a/libgo/go/net/http/example_test.go +++ b/libgo/go/net/http/example_test.go @@ -68,3 +68,21 @@ func ExampleStripPrefix() { // URL's path before the FileServer sees it: http.Handle("/tmpfiles/", http.StripPrefix("/tmpfiles/", http.FileServer(http.Dir("/tmp")))) } + +type apiHandler struct{} + +func (apiHandler) ServeHTTP(http.ResponseWriter, *http.Request) {} + +func ExampleServeMux_Handle() { + mux := http.NewServeMux() + mux.Handle("/api/", apiHandler{}) + mux.HandleFunc("/", func(w http.ResponseWriter, req *http.Request) { + // The "/" pattern matches everything, so we need to check + // that we're at the root here. + if req.URL.Path != "/" { + http.NotFound(w, req) + return + } + fmt.Fprintf(w, "Welcome to the home page!") + }) +} diff --git a/libgo/go/net/http/export_test.go b/libgo/go/net/http/export_test.go index 3fc2453..22b7f27 100644 --- a/libgo/go/net/http/export_test.go +++ b/libgo/go/net/http/export_test.go @@ -16,6 +16,8 @@ func NewLoggingConn(baseName string, c net.Conn) net.Conn { return newLoggingConn(baseName, c) } +var ExportAppendTime = appendTime + func (t *Transport) NumPendingRequestsForTesting() int { t.reqMu.Lock() defer t.reqMu.Unlock() @@ -48,6 +50,12 @@ func (t *Transport) IdleConnCountForTesting(cacheKey string) int { return len(conns) } +func (t *Transport) IdleConnChMapSizeForTesting() int { + t.idleMu.Lock() + defer t.idleMu.Unlock() + return len(t.idleConnCh) +} + func NewTestTimeoutHandler(handler Handler, ch <-chan time.Time) Handler { f := func() <-chan time.Time { return ch diff --git a/libgo/go/net/http/fs.go b/libgo/go/net/http/fs.go index b6bea0d..8b32ca1 100644 --- a/libgo/go/net/http/fs.go +++ b/libgo/go/net/http/fs.go @@ -105,23 +105,31 @@ func dirList(w ResponseWriter, f File) { // // Note that *os.File implements the io.ReadSeeker interface. func ServeContent(w ResponseWriter, req *Request, name string, modtime time.Time, content io.ReadSeeker) { - size, err := content.Seek(0, os.SEEK_END) - if err != nil { - Error(w, "seeker can't seek", StatusInternalServerError) - return - } - _, err = content.Seek(0, os.SEEK_SET) - if err != nil { - Error(w, "seeker can't seek", StatusInternalServerError) - return + sizeFunc := func() (int64, error) { + size, err := content.Seek(0, os.SEEK_END) + if err != nil { + return 0, errSeeker + } + _, err = content.Seek(0, os.SEEK_SET) + if err != nil { + return 0, errSeeker + } + return size, nil } - serveContent(w, req, name, modtime, size, content) + serveContent(w, req, name, modtime, sizeFunc, content) } +// errSeeker is returned by ServeContent's sizeFunc when the content +// doesn't seek properly. The underlying Seeker's error text isn't +// included in the sizeFunc reply so it's not sent over HTTP to end +// users. +var errSeeker = errors.New("seeker can't seek") + // if name is empty, filename is unknown. (used for mime type, before sniffing) // if modtime.IsZero(), modtime is unknown. // content must be seeked to the beginning of the file. -func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time, size int64, content io.ReadSeeker) { +// The sizeFunc is called at most once. Its error, if any, is sent in the HTTP response. +func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time, sizeFunc func() (int64, error), content io.ReadSeeker) { if checkLastModified(w, r, modtime) { return } @@ -132,16 +140,17 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time, code := StatusOK - // If Content-Type isn't set, use the file's extension to find it. - ctype := w.Header().Get("Content-Type") - if ctype == "" { + // If Content-Type isn't set, use the file's extension to find it, but + // if the Content-Type is unset explicitly, do not sniff the type. + ctypes, haveType := w.Header()["Content-Type"] + var ctype string + if !haveType { ctype = mime.TypeByExtension(filepath.Ext(name)) if ctype == "" { // read a chunk to decide between utf-8 text and binary - var buf [1024]byte + var buf [sniffLen]byte n, _ := io.ReadFull(content, buf[:]) - b := buf[:n] - ctype = DetectContentType(b) + ctype = DetectContentType(buf[:n]) _, err := content.Seek(0, os.SEEK_SET) // rewind to output whole file if err != nil { Error(w, "seeker can't seek", StatusInternalServerError) @@ -149,6 +158,14 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time, } } w.Header().Set("Content-Type", ctype) + } else if len(ctypes) > 0 { + ctype = ctypes[0] + } + + size, err := sizeFunc() + if err != nil { + Error(w, err.Error(), StatusInternalServerError) + return } // handle Content-Range header. @@ -160,7 +177,7 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time, Error(w, err.Error(), StatusRequestedRangeNotSatisfiable) return } - if sumRangesSize(ranges) >= size { + if sumRangesSize(ranges) > size { // The total number of bytes in all the ranges // is larger than the size of the file by // itself, so this is probably an attack, or a @@ -378,7 +395,8 @@ func serveFile(w ResponseWriter, r *Request, fs FileSystem, name string, redirec } // serverContent will check modification time - serveContent(w, r, d.Name(), d.ModTime(), d.Size(), f) + sizeFunc := func() (int64, error) { return d.Size(), nil } + serveContent(w, r, d.Name(), d.ModTime(), sizeFunc, f) } // localRedirect gives a Moved Permanently response. diff --git a/libgo/go/net/http/fs_test.go b/libgo/go/net/http/fs_test.go index d389667..dd3e9fe 100644 --- a/libgo/go/net/http/fs_test.go +++ b/libgo/go/net/http/fs_test.go @@ -20,8 +20,10 @@ import ( "os/exec" "path" "path/filepath" + "reflect" "regexp" "runtime" + "strconv" "strings" "testing" "time" @@ -36,6 +38,8 @@ type wantRange struct { start, end int64 // range [start,end) } +var itoa = strconv.Itoa + var ServeFileRangeTests = []struct { r string code int @@ -50,7 +54,11 @@ var ServeFileRangeTests = []struct { {r: "bytes=0-0,-2", code: StatusPartialContent, ranges: []wantRange{{0, 1}, {testFileLen - 2, testFileLen}}}, {r: "bytes=0-1,5-8", code: StatusPartialContent, ranges: []wantRange{{0, 2}, {5, 9}}}, {r: "bytes=0-1,5-", code: StatusPartialContent, ranges: []wantRange{{0, 2}, {5, testFileLen}}}, + {r: "bytes=5-1000", code: StatusPartialContent, ranges: []wantRange{{5, testFileLen}}}, {r: "bytes=0-,1-,2-,3-,4-", code: StatusOK}, // ignore wasteful range request + {r: "bytes=0-" + itoa(testFileLen-2), code: StatusPartialContent, ranges: []wantRange{{0, testFileLen - 1}}}, + {r: "bytes=0-" + itoa(testFileLen-1), code: StatusPartialContent, ranges: []wantRange{{0, testFileLen}}}, + {r: "bytes=0-" + itoa(testFileLen), code: StatusPartialContent, ranges: []wantRange{{0, testFileLen}}}, } func TestServeFile(t *testing.T) { @@ -259,6 +267,9 @@ func TestFileServerImplicitLeadingSlash(t *testing.T) { } func TestDirJoin(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("skipping test on windows") + } wfi, err := os.Stat("/etc/hosts") if err != nil { t.Skip("skipping test; no /etc/hosts file") @@ -309,24 +320,29 @@ func TestServeFileContentType(t *testing.T) { defer afterTest(t) const ctype = "icecream/chocolate" ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { - if r.FormValue("override") == "1" { + switch r.FormValue("override") { + case "1": w.Header().Set("Content-Type", ctype) + case "2": + // Explicitly inhibit sniffing. + w.Header()["Content-Type"] = []string{} } ServeFile(w, r, "testdata/file") })) defer ts.Close() - get := func(override, want string) { + get := func(override string, want []string) { resp, err := Get(ts.URL + "?override=" + override) if err != nil { t.Fatal(err) } - if h := resp.Header.Get("Content-Type"); h != want { - t.Errorf("Content-Type mismatch: got %q, want %q", h, want) + if h := resp.Header["Content-Type"]; !reflect.DeepEqual(h, want) { + t.Errorf("Content-Type mismatch: got %v, want %v", h, want) } resp.Body.Close() } - get("0", "text/plain; charset=utf-8") - get("1", ctype) + get("0", []string{"text/plain; charset=utf-8"}) + get("1", []string{ctype}) + get("2", nil) } func TestServeFileMimeType(t *testing.T) { @@ -567,7 +583,10 @@ func TestServeContent(t *testing.T) { defer ts.Close() type testCase struct { - file string + // One of file or content must be set: + file string + content io.ReadSeeker + modtime time.Time serveETag string // optional serveContentType string // optional @@ -615,6 +634,14 @@ func TestServeContent(t *testing.T) { }, wantStatus: 304, }, + "not_modified_etag_no_seek": { + content: panicOnSeek{nil}, // should never be called + serveETag: `"foo"`, + reqHeader: map[string]string{ + "If-None-Match": `"foo"`, + }, + wantStatus: 304, + }, "range_good": { file: "testdata/style.css", serveETag: `"A"`, @@ -638,15 +665,21 @@ func TestServeContent(t *testing.T) { }, } for testName, tt := range tests { - f, err := os.Open(tt.file) - if err != nil { - t.Fatalf("test %q: %v", testName, err) + var content io.ReadSeeker + if tt.file != "" { + f, err := os.Open(tt.file) + if err != nil { + t.Fatalf("test %q: %v", testName, err) + } + defer f.Close() + content = f + } else { + content = tt.content } - defer f.Close() servec <- serveParam{ name: filepath.Base(tt.file), - content: f, + content: content, modtime: tt.modtime, etag: tt.serveETag, contentType: tt.serveContentType, @@ -768,3 +801,5 @@ func TestLinuxSendfileChild(*testing.T) { panic(err) } } + +type panicOnSeek struct{ io.ReadSeeker } diff --git a/libgo/go/net/http/header.go b/libgo/go/net/http/header.go index 6374237..ca1ae07 100644 --- a/libgo/go/net/http/header.go +++ b/libgo/go/net/http/header.go @@ -173,7 +173,7 @@ func (h Header) WriteSubset(w io.Writer, exclude map[string]bool) error { // canonical key for "accept-encoding" is "Accept-Encoding". func CanonicalHeaderKey(s string) string { return textproto.CanonicalMIMEHeaderKey(s) } -// hasToken returns whether token appears with v, ASCII +// hasToken reports whether token appears with v, ASCII // case-insensitive, with space or comma boundaries. // token must be all lowercase. // v may contain mixed cased. diff --git a/libgo/go/net/http/header_test.go b/libgo/go/net/http/header_test.go index 584f100..2c896c5 100644 --- a/libgo/go/net/http/header_test.go +++ b/libgo/go/net/http/header_test.go @@ -193,6 +193,9 @@ func BenchmarkHeaderWriteSubset(b *testing.B) { } func TestHeaderWriteSubsetMallocs(t *testing.T) { + if testing.Short() { + t.Skip("skipping malloc count in short mode") + } t.Skip("Skipping alloc count test on gccgo") if runtime.GOMAXPROCS(0) > 1 { t.Skip("skipping; GOMAXPROCS>1") @@ -202,6 +205,6 @@ func TestHeaderWriteSubsetMallocs(t *testing.T) { testHeader.WriteSubset(&buf, nil) }) if n > 0 { - t.Errorf("mallocs = %d; want 0", n) + t.Errorf("mallocs = %g; want 0", n) } } diff --git a/libgo/go/net/http/request.go b/libgo/go/net/http/request.go index 6d45691..57b5d09 100644 --- a/libgo/go/net/http/request.go +++ b/libgo/go/net/http/request.go @@ -10,7 +10,6 @@ import ( "bufio" "bytes" "crypto/tls" - "encoding/base64" "errors" "fmt" "io" @@ -106,7 +105,16 @@ type Request struct { // following a hyphen uppercase and the rest lowercase. Header Header - // The message body. + // Body is the request's body. + // + // For client requests, a nil body means the request has no + // body, such as a GET request. The HTTP Client's Transport + // is responsible for calling the Close method. + // + // For server requests, the Request Body is always non-nil + // but will return EOF immediately when no body is present. + // The Server will close the request body. The ServeHTTP + // Handler does not need to. Body io.ReadCloser // ContentLength records the length of the associated content. @@ -183,7 +191,7 @@ type Request struct { TLS *tls.ConnectionState } -// ProtoAtLeast returns whether the HTTP protocol used +// ProtoAtLeast reports whether the HTTP protocol used // in the request is at least major.minor. func (r *Request) ProtoAtLeast(major, minor int) bool { return r.ProtoMajor > major || @@ -216,7 +224,7 @@ func (r *Request) Cookie(name string) (*Cookie, error) { // means all cookies, if any, are written into the same line, // separated by semicolon. func (r *Request) AddCookie(c *Cookie) { - s := fmt.Sprintf("%s=%s", sanitizeName(c.Name), sanitizeValue(c.Value)) + s := fmt.Sprintf("%s=%s", sanitizeCookieName(c.Name), sanitizeCookieValue(c.Value)) if c := r.Header.Get("Cookie"); c != "" { r.Header.Set("Cookie", c+"; "+s) } else { @@ -283,6 +291,11 @@ func valueOrDefault(value, def string) string { return def } +// NOTE: This is not intended to reflect the actual Go version being used. +// It was changed from "Go http package" to "Go 1.1 package http" at the +// time of the Go 1.1 release because the former User-Agent had ended up +// on a blacklist for some intrusion detection systems. +// See https://codereview.appspot.com/7532043. const defaultUserAgent = "Go 1.1 package http" // Write writes an HTTP/1.1 request -- header and body -- in wire format. @@ -424,6 +437,10 @@ func ParseHTTPVersion(vers string) (major, minor int, ok bool) { } // NewRequest returns a new Request given a method, URL, and optional body. +// +// If the provided body is also an io.Closer, the returned +// Request.Body is set to body and will be closed by the Client +// methods Do, Post, and PostForm, and Transport.RoundTrip. func NewRequest(method, urlStr string, body io.Reader) (*Request, error) { u, err := url.Parse(urlStr) if err != nil { @@ -463,8 +480,7 @@ func NewRequest(method, urlStr string, body io.Reader) (*Request, error) { // With HTTP Basic Authentication the provided username and password // are not encrypted. func (r *Request) SetBasicAuth(username, password string) { - s := username + ":" + password - r.Header.Set("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(s))) + r.Header.Set("Authorization", "Basic "+basicAuth(username, password)) } // parseRequestLine parses "GET /foo HTTP/1.1" into its three parts. diff --git a/libgo/go/net/http/request_test.go b/libgo/go/net/http/request_test.go index 692485c..89303c3 100644 --- a/libgo/go/net/http/request_test.go +++ b/libgo/go/net/http/request_test.go @@ -332,7 +332,7 @@ func TestRequestWriteBufferedWriter(t *testing.T) { func testMissingFile(t *testing.T, req *Request) { f, fh, err := req.FormFile("missing") if f != nil { - t.Errorf("FormFile file = %q, want nil", f) + t.Errorf("FormFile file = %v, want nil", f) } if fh != nil { t.Errorf("FormFile file header = %q, want nil", fh) diff --git a/libgo/go/net/http/response.go b/libgo/go/net/http/response.go index 9a7e4e3..35d0ba3 100644 --- a/libgo/go/net/http/response.go +++ b/libgo/go/net/http/response.go @@ -32,7 +32,7 @@ type Response struct { ProtoMinor int // e.g. 0 // Header maps header keys to values. If the response had multiple - // headers with the same key, they will be concatenated, with comma + // headers with the same key, they may be concatenated, with comma // delimiters. (Section 4.2 of RFC 2616 requires that multiple headers // be semantically equivalent to a comma-delimited sequence.) Values // duplicated by other fields in this struct (e.g., ContentLength) are @@ -98,18 +98,17 @@ func (r *Response) Location() (*url.URL, error) { return url.Parse(lv) } -// ReadResponse reads and returns an HTTP response from r. The -// req parameter specifies the Request that corresponds to -// this Response. Clients must call resp.Body.Close when finished -// reading resp.Body. After that call, clients can inspect -// resp.Trailer to find key/value pairs included in the response -// trailer. -func ReadResponse(r *bufio.Reader, req *Request) (resp *Response, err error) { - +// ReadResponse reads and returns an HTTP response from r. +// The req parameter optionally specifies the Request that corresponds +// to this Response. If nil, a GET request is assumed. +// Clients must call resp.Body.Close when finished reading resp.Body. +// After that call, clients can inspect resp.Trailer to find key/value +// pairs included in the response trailer. +func ReadResponse(r *bufio.Reader, req *Request) (*Response, error) { tp := textproto.NewReader(r) - resp = new(Response) - - resp.Request = req + resp := &Response{ + Request: req, + } // Parse the first line of the response. line, err := tp.ReadLine() @@ -168,7 +167,7 @@ func fixPragmaCacheControl(header Header) { } } -// ProtoAtLeast returns whether the HTTP protocol used +// ProtoAtLeast reports whether the HTTP protocol used // in the response is at least major.minor. func (r *Response) ProtoAtLeast(major, minor int) bool { return r.ProtoMajor > major || diff --git a/libgo/go/net/http/response_test.go b/libgo/go/net/http/response_test.go index 02796e88..5044306 100644 --- a/libgo/go/net/http/response_test.go +++ b/libgo/go/net/http/response_test.go @@ -348,6 +348,29 @@ some body`, "some body", }, + + // Unchunked response without Content-Length, Request is nil + { + "HTTP/1.0 200 OK\r\n" + + "Connection: close\r\n" + + "\r\n" + + "Body here\n", + + Response{ + Status: "200 OK", + StatusCode: 200, + Proto: "HTTP/1.0", + ProtoMajor: 1, + ProtoMinor: 0, + Header: Header{ + "Connection": {"close"}, // TODO(rsc): Delete? + }, + Close: true, + ContentLength: -1, + }, + + "Body here\n", + }, } func TestReadResponse(t *testing.T) { @@ -565,3 +588,42 @@ func TestResponseStatusStutter(t *testing.T) { t.Errorf("stutter in status: %s", buf.String()) } } + +func TestResponseContentLengthShortBody(t *testing.T) { + const shortBody = "Short body, not 123 bytes." + br := bufio.NewReader(strings.NewReader("HTTP/1.1 200 OK\r\n" + + "Content-Length: 123\r\n" + + "\r\n" + + shortBody)) + res, err := ReadResponse(br, &Request{Method: "GET"}) + if err != nil { + t.Fatal(err) + } + if res.ContentLength != 123 { + t.Fatalf("Content-Length = %d; want 123", res.ContentLength) + } + var buf bytes.Buffer + n, err := io.Copy(&buf, res.Body) + if n != int64(len(shortBody)) { + t.Errorf("Copied %d bytes; want %d, len(%q)", n, len(shortBody), shortBody) + } + if buf.String() != shortBody { + t.Errorf("Read body %q; want %q", buf.String(), shortBody) + } + if err != io.ErrUnexpectedEOF { + t.Errorf("io.Copy error = %#v; want io.ErrUnexpectedEOF", err) + } +} + +func TestNeedsSniff(t *testing.T) { + // needsSniff returns true with an empty response. + r := &response{} + if got, want := r.needsSniff(), true; got != want { + t.Errorf("needsSniff = %t; want %t", got, want) + } + // needsSniff returns false when Content-Type = nil. + r.handlerHeader = Header{"Content-Type": nil} + if got, want := r.needsSniff(), false; got != want { + t.Errorf("needsSniff empty Content-Type = %t; want %t", got, want) + } +} diff --git a/libgo/go/net/http/serve_test.go b/libgo/go/net/http/serve_test.go index d7b3215..8961cf4 100644 --- a/libgo/go/net/http/serve_test.go +++ b/libgo/go/net/http/serve_test.go @@ -116,6 +116,34 @@ func (c *testConn) Close() error { return nil } +// reqBytes treats req as a request (with \n delimiters) and returns it with \r\n delimiters, +// ending in \r\n\r\n +func reqBytes(req string) []byte { + return []byte(strings.Replace(strings.TrimSpace(req), "\n", "\r\n", -1) + "\r\n\r\n") +} + +type handlerTest struct { + handler Handler +} + +func newHandlerTest(h Handler) handlerTest { + return handlerTest{h} +} + +func (ht handlerTest) rawResponse(req string) string { + reqb := reqBytes(req) + var output bytes.Buffer + conn := &rwTestConn{ + Reader: bytes.NewReader(reqb), + Writer: &output, + closec: make(chan bool, 1), + } + ln := &oneConnListener{conn: conn} + go Serve(ln, ht.handler) + <-conn.closec + return output.String() +} + func TestConsumingBodyOnNextConn(t *testing.T) { conn := new(testConn) for i := 0; i < 2; i++ { @@ -241,6 +269,152 @@ func TestHostHandlers(t *testing.T) { } } +var serveMuxRegister = []struct { + pattern string + h Handler +}{ + {"/dir/", serve(200)}, + {"/search", serve(201)}, + {"codesearch.google.com/search", serve(202)}, + {"codesearch.google.com/", serve(203)}, + {"example.com/", HandlerFunc(checkQueryStringHandler)}, +} + +// serve returns a handler that sends a response with the given code. +func serve(code int) HandlerFunc { + return func(w ResponseWriter, r *Request) { + w.WriteHeader(code) + } +} + +// checkQueryStringHandler checks if r.URL.RawQuery has the same value +// as the URL excluding the scheme and the query string and sends 200 +// response code if it is, 500 otherwise. +func checkQueryStringHandler(w ResponseWriter, r *Request) { + u := *r.URL + u.Scheme = "http" + u.Host = r.Host + u.RawQuery = "" + if "http://"+r.URL.RawQuery == u.String() { + w.WriteHeader(200) + } else { + w.WriteHeader(500) + } +} + +var serveMuxTests = []struct { + method string + host string + path string + code int + pattern string +}{ + {"GET", "google.com", "/", 404, ""}, + {"GET", "google.com", "/dir", 301, "/dir/"}, + {"GET", "google.com", "/dir/", 200, "/dir/"}, + {"GET", "google.com", "/dir/file", 200, "/dir/"}, + {"GET", "google.com", "/search", 201, "/search"}, + {"GET", "google.com", "/search/", 404, ""}, + {"GET", "google.com", "/search/foo", 404, ""}, + {"GET", "codesearch.google.com", "/search", 202, "codesearch.google.com/search"}, + {"GET", "codesearch.google.com", "/search/", 203, "codesearch.google.com/"}, + {"GET", "codesearch.google.com", "/search/foo", 203, "codesearch.google.com/"}, + {"GET", "codesearch.google.com", "/", 203, "codesearch.google.com/"}, + {"GET", "images.google.com", "/search", 201, "/search"}, + {"GET", "images.google.com", "/search/", 404, ""}, + {"GET", "images.google.com", "/search/foo", 404, ""}, + {"GET", "google.com", "/../search", 301, "/search"}, + {"GET", "google.com", "/dir/..", 301, ""}, + {"GET", "google.com", "/dir/..", 301, ""}, + {"GET", "google.com", "/dir/./file", 301, "/dir/"}, + + // The /foo -> /foo/ redirect applies to CONNECT requests + // but the path canonicalization does not. + {"CONNECT", "google.com", "/dir", 301, "/dir/"}, + {"CONNECT", "google.com", "/../search", 404, ""}, + {"CONNECT", "google.com", "/dir/..", 200, "/dir/"}, + {"CONNECT", "google.com", "/dir/..", 200, "/dir/"}, + {"CONNECT", "google.com", "/dir/./file", 200, "/dir/"}, +} + +func TestServeMuxHandler(t *testing.T) { + mux := NewServeMux() + for _, e := range serveMuxRegister { + mux.Handle(e.pattern, e.h) + } + + for _, tt := range serveMuxTests { + r := &Request{ + Method: tt.method, + Host: tt.host, + URL: &url.URL{ + Path: tt.path, + }, + } + h, pattern := mux.Handler(r) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, r) + if pattern != tt.pattern || rr.Code != tt.code { + t.Errorf("%s %s %s = %d, %q, want %d, %q", tt.method, tt.host, tt.path, rr.Code, pattern, tt.code, tt.pattern) + } + } +} + +var serveMuxTests2 = []struct { + method string + host string + url string + code int + redirOk bool +}{ + {"GET", "google.com", "/", 404, false}, + {"GET", "example.com", "/test/?example.com/test/", 200, false}, + {"GET", "example.com", "test/?example.com/test/", 200, true}, +} + +// TestServeMuxHandlerRedirects tests that automatic redirects generated by +// mux.Handler() shouldn't clear the request's query string. +func TestServeMuxHandlerRedirects(t *testing.T) { + mux := NewServeMux() + for _, e := range serveMuxRegister { + mux.Handle(e.pattern, e.h) + } + + for _, tt := range serveMuxTests2 { + tries := 1 + turl := tt.url + for tries > 0 { + u, e := url.Parse(turl) + if e != nil { + t.Fatal(e) + } + r := &Request{ + Method: tt.method, + Host: tt.host, + URL: u, + } + h, _ := mux.Handler(r) + rr := httptest.NewRecorder() + h.ServeHTTP(rr, r) + if rr.Code != 301 { + if rr.Code != tt.code { + t.Errorf("%s %s %s = %d, want %d", tt.method, tt.host, tt.url, rr.Code, tt.code) + } + break + } + if !tt.redirOk { + t.Errorf("%s %s %s, unexpected redirect", tt.method, tt.host, tt.url) + break + } + turl = rr.HeaderMap.Get("Location") + tries-- + } + if tries < 0 { + t.Errorf("%s %s %s, too many redirects", tt.method, tt.host, tt.url) + } + } +} + // 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"} @@ -626,22 +800,20 @@ func Test304Responses(t *testing.T) { } } -// TestHeadResponses verifies that responses to HEAD requests don't -// declare that they're chunking in their response headers, aren't -// allowed to produce output, and don't set a Content-Type since -// the real type of the body data cannot be inferred. +// TestHeadResponses verifies that all MIME type sniffing and Content-Length +// counting of GET requests also happens on HEAD requests. func TestHeadResponses(t *testing.T) { defer afterTest(t) ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { - _, err := w.Write([]byte("Ignored body")) - if err != ErrBodyNotAllowed { - t.Errorf("on Write, expected ErrBodyNotAllowed, got %v", err) + _, err := w.Write([]byte("<html>")) + if err != nil { + t.Errorf("ResponseWriter.Write: %v", err) } // Also exercise the ReaderFrom path - _, err = io.Copy(w, strings.NewReader("Ignored body")) - if err != ErrBodyNotAllowed { - t.Errorf("on Copy, expected ErrBodyNotAllowed, got %v", err) + _, err = io.Copy(w, strings.NewReader("789a")) + if err != nil { + t.Errorf("Copy(ResponseWriter, ...): %v", err) } })) defer ts.Close() @@ -652,9 +824,11 @@ func TestHeadResponses(t *testing.T) { if len(res.TransferEncoding) > 0 { t.Errorf("expected no TransferEncoding; got %v", res.TransferEncoding) } - ct := res.Header.Get("Content-Type") - if ct != "" { - t.Errorf("expected no Content-Type; got %s", ct) + if ct := res.Header.Get("Content-Type"); ct != "text/html; charset=utf-8" { + t.Errorf("Content-Type: %q; want text/html; charset=utf-8", ct) + } + if v := res.ContentLength; v != 10 { + t.Errorf("Content-Length: %d; want 10", v) } body, err := ioutil.ReadAll(res.Body) if err != nil { @@ -975,6 +1149,23 @@ func TestRedirectMunging(t *testing.T) { } } +func TestRedirectBadPath(t *testing.T) { + // This used to crash. It's not valid input (bad path), but it + // shouldn't crash. + rr := httptest.NewRecorder() + req := &Request{ + Method: "GET", + URL: &url.URL{ + Scheme: "http", + Path: "not-empty-but-no-leading-slash", // bogus + }, + } + Redirect(rr, req, "", 304) + if rr.Code != 304 { + t.Errorf("Code = %d; want 304", rr.Code) + } +} + // TestZeroLengthPostAndResponse exercises an optimization done by the Transport: // when there is no body (either because the method doesn't permit a body, or an // explicit Content-Length of zero is present), then the transport can re-use the @@ -1408,10 +1599,7 @@ For: func TestCloseNotifierChanLeak(t *testing.T) { defer afterTest(t) - req := []byte(strings.Replace(`GET / HTTP/1.0 -Host: golang.org - -`, "\n", "\r\n", -1)) + req := reqBytes("GET / HTTP/1.0\nHost: golang.org") for i := 0; i < 20; i++ { var output bytes.Buffer conn := &rwTestConn{ @@ -1493,11 +1681,6 @@ func TestOptions(t *testing.T) { // ones, even if the handler modifies them (~erroneously) after the // first Write. func TestHeaderToWire(t *testing.T) { - req := []byte(strings.Replace(`GET / HTTP/1.1 -Host: golang.org - -`, "\n", "\r\n", -1)) - tests := []struct { name string handler func(ResponseWriter, *Request) @@ -1660,17 +1843,10 @@ Host: golang.org }, } for _, tc := range tests { - var output bytes.Buffer - conn := &rwTestConn{ - Reader: bytes.NewReader(req), - Writer: &output, - closec: make(chan bool, 1), - } - ln := &oneConnListener{conn: conn} - go Serve(ln, HandlerFunc(tc.handler)) - <-conn.closec - if err := tc.check(output.String()); err != nil { - t.Errorf("%s: %v\nGot response:\n%s", tc.name, err, output.Bytes()) + ht := newHandlerTest(HandlerFunc(tc.handler)) + got := ht.rawResponse("GET / HTTP/1.1\nHost: golang.org") + if err := tc.check(got); err != nil { + t.Errorf("%s: %v\nGot response:\n%s", tc.name, err, got) } } } @@ -1726,7 +1902,200 @@ func TestAcceptMaxFds(t *testing.T) { } } +func TestWriteAfterHijack(t *testing.T) { + req := reqBytes("GET / HTTP/1.1\nHost: golang.org") + var buf bytes.Buffer + wrotec := make(chan bool, 1) + conn := &rwTestConn{ + Reader: bytes.NewReader(req), + Writer: &buf, + closec: make(chan bool, 1), + } + handler := HandlerFunc(func(rw ResponseWriter, r *Request) { + conn, bufrw, err := rw.(Hijacker).Hijack() + if err != nil { + t.Error(err) + return + } + go func() { + bufrw.Write([]byte("[hijack-to-bufw]")) + bufrw.Flush() + conn.Write([]byte("[hijack-to-conn]")) + conn.Close() + wrotec <- true + }() + }) + ln := &oneConnListener{conn: conn} + go Serve(ln, handler) + <-conn.closec + <-wrotec + if g, w := buf.String(), "[hijack-to-bufw][hijack-to-conn]"; g != w { + t.Errorf("wrote %q; want %q", g, w) + } +} + +// http://code.google.com/p/go/issues/detail?id=5955 +// Note that this does not test the "request too large" +// exit path from the http server. This is intentional; +// not sending Connection: close is just a minor wire +// optimization and is pointless if dealing with a +// badly behaved client. +func TestHTTP10ConnectionHeader(t *testing.T) { + defer afterTest(t) + + mux := NewServeMux() + mux.Handle("/", HandlerFunc(func(resp ResponseWriter, req *Request) {})) + ts := httptest.NewServer(mux) + defer ts.Close() + + // net/http uses HTTP/1.1 for requests, so write requests manually + tests := []struct { + req string // raw http request + expect []string // expected Connection header(s) + }{ + { + req: "GET / HTTP/1.0\r\n\r\n", + expect: nil, + }, + { + req: "OPTIONS * HTTP/1.0\r\n\r\n", + expect: nil, + }, + { + req: "GET / HTTP/1.0\r\nConnection: keep-alive\r\n\r\n", + expect: []string{"keep-alive"}, + }, + } + + for _, tt := range tests { + conn, err := net.Dial("tcp", ts.Listener.Addr().String()) + if err != nil { + t.Fatal("dial err:", err) + } + + _, err = fmt.Fprint(conn, tt.req) + if err != nil { + t.Fatal("conn write err:", err) + } + + resp, err := ReadResponse(bufio.NewReader(conn), &Request{Method: "GET"}) + if err != nil { + t.Fatal("ReadResponse err:", err) + } + conn.Close() + resp.Body.Close() + + got := resp.Header["Connection"] + if !reflect.DeepEqual(got, tt.expect) { + t.Errorf("wrong Connection headers for request %q. Got %q expect %q", tt.req, got, tt.expect) + } + } +} + +// See golang.org/issue/5660 +func TestServerReaderFromOrder(t *testing.T) { + defer afterTest(t) + pr, pw := io.Pipe() + const size = 3 << 20 + ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) { + rw.Header().Set("Content-Type", "text/plain") // prevent sniffing path + done := make(chan bool) + go func() { + io.Copy(rw, pr) + close(done) + }() + time.Sleep(25 * time.Millisecond) // give Copy a chance to break things + n, err := io.Copy(ioutil.Discard, req.Body) + if err != nil { + t.Errorf("handler Copy: %v", err) + return + } + if n != size { + t.Errorf("handler Copy = %d; want %d", n, size) + } + pw.Write([]byte("hi")) + pw.Close() + <-done + })) + defer ts.Close() + + req, err := NewRequest("POST", ts.URL, io.LimitReader(neverEnding('a'), size)) + if err != nil { + t.Fatal(err) + } + res, err := DefaultClient.Do(req) + if err != nil { + t.Fatal(err) + } + all, err := ioutil.ReadAll(res.Body) + if err != nil { + t.Fatal(err) + } + res.Body.Close() + if string(all) != "hi" { + t.Errorf("Body = %q; want hi", all) + } +} + +// Issue 6157 +func TestNoContentTypeOnNotModified(t *testing.T) { + ht := newHandlerTest(HandlerFunc(func(w ResponseWriter, r *Request) { + if r.URL.Path == "/header" { + w.Header().Set("Content-Length", "123") + } + w.WriteHeader(StatusNotModified) + if r.URL.Path == "/more" { + w.Write([]byte("stuff")) + } + })) + for _, req := range []string{ + "GET / HTTP/1.0", + "GET /header HTTP/1.0", + "GET /more HTTP/1.0", + "GET / HTTP/1.1", + "GET /header HTTP/1.1", + "GET /more HTTP/1.1", + } { + got := ht.rawResponse(req) + if !strings.Contains(got, "304 Not Modified") { + t.Errorf("Non-304 Not Modified for %q: %s", req, got) + } else if strings.Contains(got, "Content-Length") { + t.Errorf("Got a Content-Length from %q: %s", req, got) + } + } +} + +func TestResponseWriterWriteStringAllocs(t *testing.T) { + t.Skip("allocs test unreliable with gccgo") + ht := newHandlerTest(HandlerFunc(func(w ResponseWriter, r *Request) { + if r.URL.Path == "/s" { + io.WriteString(w, "Hello world") + } else { + w.Write([]byte("Hello world")) + } + })) + before := testing.AllocsPerRun(25, func() { ht.rawResponse("GET / HTTP/1.0") }) + after := testing.AllocsPerRun(25, func() { ht.rawResponse("GET /s HTTP/1.0") }) + if int(after) >= int(before) { + t.Errorf("WriteString allocs of %v >= Write allocs of %v", after, before) + } +} + +func TestAppendTime(t *testing.T) { + var b [len(TimeFormat)]byte + t1 := time.Date(2013, 9, 21, 15, 41, 0, 0, time.FixedZone("CEST", 2*60*60)) + res := ExportAppendTime(b[:0], t1) + t2, err := ParseTime(string(res)) + if err != nil { + t.Fatalf("Error parsing time: %s", err) + } + if !t1.Equal(t2) { + t.Fatalf("Times differ; expected: %v, got %v (%s)", t1, t2, string(res)) + } +} + func BenchmarkClientServer(b *testing.B) { + b.ReportAllocs() b.StopTimer() ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, r *Request) { fmt.Fprintf(rw, "Hello world.\n") @@ -1761,6 +2130,7 @@ func BenchmarkClientServerParallel64(b *testing.B) { } func benchmarkClientServerParallel(b *testing.B, conc int) { + b.ReportAllocs() b.StopTimer() ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, r *Request) { fmt.Fprintf(rw, "Hello world.\n") @@ -1805,6 +2175,7 @@ func benchmarkClientServerParallel(b *testing.B, conc int) { // $ go tool pprof http.test http.prof // (pprof) web func BenchmarkServer(b *testing.B) { + b.ReportAllocs() // Child process mode; if url := os.Getenv("TEST_BENCH_SERVER_URL"); url != "" { n, err := strconv.Atoi(os.Getenv("TEST_BENCH_CLIENT_N")) @@ -1851,15 +2222,14 @@ func BenchmarkServer(b *testing.B) { func BenchmarkServerFakeConnNoKeepAlive(b *testing.B) { b.ReportAllocs() - req := []byte(strings.Replace(`GET / HTTP/1.0 + req := reqBytes(`GET / HTTP/1.0 Host: golang.org Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.52 Safari/537.17 Accept-Encoding: gzip,deflate,sdch Accept-Language: en-US,en;q=0.8 Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3 - -`, "\n", "\r\n", -1)) +`) res := []byte("Hello world!\n") conn := &testConn{ @@ -1905,15 +2275,14 @@ func (r *repeatReader) Read(p []byte) (n int, err error) { func BenchmarkServerFakeConnWithKeepAlive(b *testing.B) { b.ReportAllocs() - req := []byte(strings.Replace(`GET / HTTP/1.1 + req := reqBytes(`GET / HTTP/1.1 Host: golang.org Accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_8_2) AppleWebKit/537.17 (KHTML, like Gecko) Chrome/24.0.1312.52 Safari/537.17 Accept-Encoding: gzip,deflate,sdch Accept-Language: en-US,en;q=0.8 Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3 - -`, "\n", "\r\n", -1)) +`) res := []byte("Hello world!\n") conn := &rwTestConn{ @@ -1940,10 +2309,9 @@ Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.3 func BenchmarkServerFakeConnWithKeepAliveLite(b *testing.B) { b.ReportAllocs() - req := []byte(strings.Replace(`GET / HTTP/1.1 + req := reqBytes(`GET / HTTP/1.1 Host: golang.org - -`, "\n", "\r\n", -1)) +`) res := []byte("Hello world!\n") conn := &rwTestConn{ @@ -2003,10 +2371,9 @@ func BenchmarkServerHandlerNoHeader(b *testing.B) { func benchmarkHandler(b *testing.B, h Handler) { b.ReportAllocs() - req := []byte(strings.Replace(`GET / HTTP/1.1 + req := reqBytes(`GET / HTTP/1.1 Host: golang.org - -`, "\n", "\r\n", -1)) +`) conn := &rwTestConn{ Reader: &repeatReader{content: req, count: b.N}, Writer: ioutil.Discard, diff --git a/libgo/go/net/http/server.go b/libgo/go/net/http/server.go index b259607..0e46863 100644 --- a/libgo/go/net/http/server.go +++ b/libgo/go/net/http/server.go @@ -16,6 +16,7 @@ import ( "log" "net" "net/url" + "os" "path" "runtime" "strconv" @@ -109,8 +110,6 @@ type conn struct { sr liveSwitchReader // where the LimitReader reads from; usually the rwc lr *io.LimitedReader // io.LimitReader(sr) buf *bufio.ReadWriter // buffered(lr,rwc), reading from bufio->limitReader->sr->rwc - bufswr *switchReader // the *switchReader io.Reader source of buf - bufsww *switchWriter // the *switchWriter io.Writer dest of buf tlsState *tls.ConnectionState // or nil when not using TLS mu sync.Mutex // guards the following @@ -246,6 +245,10 @@ func (cw *chunkWriter) Write(p []byte) (n int, err error) { if !cw.wroteHeader { cw.writeHeader(p) } + if cw.res.req.Method == "HEAD" { + // Eat writes. + return len(p), nil + } if cw.chunking { _, err = fmt.Fprintf(cw.res.conn.buf, "%x\r\n", len(p)) if err != nil { @@ -278,7 +281,7 @@ func (cw *chunkWriter) close() { // zero EOF chunk, trailer key/value pairs (currently // unsupported in Go's server), followed by a blank // line. - io.WriteString(cw.res.conn.buf, "0\r\n\r\n") + cw.res.conn.buf.WriteString("0\r\n\r\n") } } @@ -320,6 +323,10 @@ type response struct { requestBodyLimitHit bool handlerDone bool // set true when the handler exits + + // Buffers for Date and Content-Length + dateBuf [len(TimeFormat)]byte + clenBuf [10]byte } // requestTooLarge is called by maxBytesReader when too much input has @@ -332,16 +339,50 @@ func (w *response) requestTooLarge() { } } -// needsSniff returns whether a Content-Type still needs to be sniffed. +// needsSniff reports whether a Content-Type still needs to be sniffed. func (w *response) needsSniff() bool { - return !w.cw.wroteHeader && w.handlerHeader.Get("Content-Type") == "" && w.written < sniffLen + _, haveType := w.handlerHeader["Content-Type"] + return !w.cw.wroteHeader && !haveType && w.written < sniffLen } +// writerOnly hides an io.Writer value's optional ReadFrom method +// from io.Copy. type writerOnly struct { io.Writer } +func srcIsRegularFile(src io.Reader) (isRegular bool, err error) { + switch v := src.(type) { + case *os.File: + fi, err := v.Stat() + if err != nil { + return false, err + } + return fi.Mode().IsRegular(), nil + case *io.LimitedReader: + return srcIsRegularFile(v.R) + default: + return + } +} + +// ReadFrom is here to optimize copying from an *os.File regular file +// to a *net.TCPConn with sendfile. func (w *response) ReadFrom(src io.Reader) (n int64, err error) { + // Our underlying w.conn.rwc is usually a *TCPConn (with its + // own ReadFrom method). If not, or if our src isn't a regular + // file, just fall back to the normal copy method. + rf, ok := w.conn.rwc.(io.ReaderFrom) + regFile, err := srcIsRegularFile(src) + if err != nil { + return 0, err + } + if !ok || !regFile { + return io.Copy(writerOnly{w}, src) + } + + // sendfile path: + if !w.wroteHeader { w.WriteHeader(StatusOK) } @@ -359,16 +400,12 @@ func (w *response) ReadFrom(src io.Reader) (n int64, err error) { // Now that cw has been flushed, its chunking field is guaranteed initialized. if !w.cw.chunking && w.bodyAllowed() { - if rf, ok := w.conn.rwc.(io.ReaderFrom); ok { - n0, err := rf.ReadFrom(src) - n += n0 - w.written += n0 - return n, err - } + n0, err := rf.ReadFrom(src) + n += n0 + w.written += n0 + return n, err } - // Fall back to default io.Copy implementation. - // Use wrapper to hide w.ReadFrom from io.Copy. n0, err := io.Copy(writerOnly{w}, src) n += n0 return n, err @@ -392,34 +429,20 @@ func (srv *Server) newConn(rwc net.Conn) (c *conn, err error) { } c.sr = liveSwitchReader{r: c.rwc} c.lr = io.LimitReader(&c.sr, noLimit).(*io.LimitedReader) - br, sr := newBufioReader(c.lr) - bw, sw := newBufioWriterSize(c.rwc, 4<<10) + br := newBufioReader(c.lr) + bw := newBufioWriterSize(c.rwc, 4<<10) c.buf = bufio.NewReadWriter(br, bw) - c.bufswr = sr - c.bufsww = sw return c, nil } -// TODO: remove this, if issue 5100 is fixed -type bufioReaderPair struct { - br *bufio.Reader - sr *switchReader // from which the bufio.Reader is reading -} - -// TODO: remove this, if issue 5100 is fixed -type bufioWriterPair struct { - bw *bufio.Writer - sw *switchWriter // to which the bufio.Writer is writing -} - // TODO: use a sync.Cache instead var ( - bufioReaderCache = make(chan bufioReaderPair, 4) - bufioWriterCache2k = make(chan bufioWriterPair, 4) - bufioWriterCache4k = make(chan bufioWriterPair, 4) + bufioReaderCache = make(chan *bufio.Reader, 4) + bufioWriterCache2k = make(chan *bufio.Writer, 4) + bufioWriterCache4k = make(chan *bufio.Writer, 4) ) -func bufioWriterCache(size int) chan bufioWriterPair { +func bufioWriterCache(size int) chan *bufio.Writer { switch size { case 2 << 10: return bufioWriterCache2k @@ -429,55 +452,38 @@ func bufioWriterCache(size int) chan bufioWriterPair { return nil } -func newBufioReader(r io.Reader) (*bufio.Reader, *switchReader) { +func newBufioReader(r io.Reader) *bufio.Reader { select { case p := <-bufioReaderCache: - p.sr.Reader = r - return p.br, p.sr + p.Reset(r) + return p default: - sr := &switchReader{r} - return bufio.NewReader(sr), sr + return bufio.NewReader(r) } } -func putBufioReader(br *bufio.Reader, sr *switchReader) { - if n := br.Buffered(); n > 0 { - io.CopyN(ioutil.Discard, br, int64(n)) - } - br.Read(nil) // clears br.err - sr.Reader = nil +func putBufioReader(br *bufio.Reader) { + br.Reset(nil) select { - case bufioReaderCache <- bufioReaderPair{br, sr}: + case bufioReaderCache <- br: default: } } -func newBufioWriterSize(w io.Writer, size int) (*bufio.Writer, *switchWriter) { +func newBufioWriterSize(w io.Writer, size int) *bufio.Writer { select { case p := <-bufioWriterCache(size): - p.sw.Writer = w - return p.bw, p.sw + p.Reset(w) + return p default: - sw := &switchWriter{w} - return bufio.NewWriterSize(sw, size), sw + return bufio.NewWriterSize(w, size) } } -func putBufioWriter(bw *bufio.Writer, sw *switchWriter) { - if bw.Buffered() > 0 { - // It must have failed to flush to its target - // earlier. We can't reuse this bufio.Writer. - return - } - if err := bw.Flush(); err != nil { - // Its sticky error field is set, which is returned by - // Flush even when there's no data buffered. This - // bufio Writer is dead to us. Don't reuse it. - return - } - sw.Writer = nil +func putBufioWriter(bw *bufio.Writer) { + bw.Reset(nil) select { - case bufioWriterCache(bw.Available()) <- bufioWriterPair{bw, sw}: + case bufioWriterCache(bw.Available()) <- bw: default: } } @@ -508,7 +514,7 @@ func (ecr *expectContinueReader) Read(p []byte) (n int, err error) { } if !ecr.resp.wroteContinue && !ecr.resp.conn.hijacked() { ecr.resp.wroteContinue = true - io.WriteString(ecr.resp.conn.buf, "HTTP/1.1 100 Continue\r\n\r\n") + ecr.resp.conn.buf.WriteString("HTTP/1.1 100 Continue\r\n\r\n") ecr.resp.conn.buf.Flush() } return ecr.readCloser.Read(p) @@ -525,6 +531,28 @@ func (ecr *expectContinueReader) Close() error { // It is like time.RFC1123 but hard codes GMT as the time zone. const TimeFormat = "Mon, 02 Jan 2006 15:04:05 GMT" +// appendTime is a non-allocating version of []byte(t.UTC().Format(TimeFormat)) +func appendTime(b []byte, t time.Time) []byte { + const days = "SunMonTueWedThuFriSat" + const months = "JanFebMarAprMayJunJulAugSepOctNovDec" + + t = t.UTC() + yy, mm, dd := t.Date() + hh, mn, ss := t.Clock() + day := days[3*t.Weekday():] + mon := months[3*(mm-1):] + + return append(b, + day[0], day[1], day[2], ',', ' ', + byte('0'+dd/10), byte('0'+dd%10), ' ', + mon[0], mon[1], mon[2], ' ', + byte('0'+yy/1000), byte('0'+(yy/100)%10), byte('0'+(yy/10)%10), byte('0'+yy%10), ' ', + byte('0'+hh/10), byte('0'+hh%10), ':', + byte('0'+mn/10), byte('0'+mn%10), ':', + byte('0'+ss/10), byte('0'+ss%10), ' ', + 'G', 'M', 'T') +} + var errTooLarge = errors.New("http: request too large") // Read next request from connection. @@ -562,7 +590,7 @@ func (c *conn) readRequest() (w *response, err error) { contentLength: -1, } w.cw.res = w - w.w, w.sw = newBufioWriterSize(&w.cw, bufferBeforeChunkingSize) + w.w = newBufioWriterSize(&w.cw, bufferBeforeChunkingSize) return w, nil } @@ -620,27 +648,45 @@ func (w *response) WriteHeader(code int) { // the response Header map and all its 1-element slices. type extraHeader struct { contentType string - contentLength string connection string - date string transferEncoding string + date []byte // written if not nil + contentLength []byte // written if not nil } // Sorted the same as extraHeader.Write's loop. var extraHeaderKeys = [][]byte{ - []byte("Content-Type"), []byte("Content-Length"), - []byte("Connection"), []byte("Date"), []byte("Transfer-Encoding"), + []byte("Content-Type"), + []byte("Connection"), + []byte("Transfer-Encoding"), } -// The value receiver, despite copying 5 strings to the stack, -// prevents an extra allocation. The escape analysis isn't smart -// enough to realize this doesn't mutate h. -func (h extraHeader) Write(w io.Writer) { - for i, v := range []string{h.contentType, h.contentLength, h.connection, h.date, h.transferEncoding} { +var ( + headerContentLength = []byte("Content-Length: ") + headerDate = []byte("Date: ") +) + +// Write writes the headers described in h to w. +// +// This method has a value receiver, despite the somewhat large size +// of h, because it prevents an allocation. The escape analysis isn't +// smart enough to realize this function doesn't mutate h. +func (h extraHeader) Write(w *bufio.Writer) { + if h.date != nil { + w.Write(headerDate) + w.Write(h.date) + w.Write(crlf) + } + if h.contentLength != nil { + w.Write(headerContentLength) + w.Write(h.contentLength) + w.Write(crlf) + } + for i, v := range []string{h.contentType, h.connection, h.transferEncoding} { if v != "" { w.Write(extraHeaderKeys[i]) w.Write(colonSpace) - io.WriteString(w, v) + w.WriteString(v) w.Write(crlf) } } @@ -661,6 +707,7 @@ func (cw *chunkWriter) writeHeader(p []byte) { cw.wroteHeader = true w := cw.res + isHEAD := w.req.Method == "HEAD" // header is written out to w.conn.buf below. Depending on the // state of the handler, we either own the map or not. If we @@ -692,9 +739,17 @@ func (cw *chunkWriter) writeHeader(p []byte) { // response header and this is our first (and last) write, set // it, even to zero. This helps HTTP/1.0 clients keep their // "keep-alive" connections alive. - if w.handlerDone && header.get("Content-Length") == "" && w.req.Method != "HEAD" { + // Exceptions: 304 responses never get Content-Length, and if + // it was a HEAD request, we don't know the difference between + // 0 actual bytes and 0 bytes because the handler noticed it + // was a HEAD request and chose not to write anything. So for + // HEAD, the handler should either write the Content-Length or + // write non-zero bytes. If it's actually 0 bytes and the + // handler never looked at the Request.Method, we just don't + // send a Content-Length header. + if w.handlerDone && w.status != StatusNotModified && header.get("Content-Length") == "" && (!isHEAD || len(p) > 0) { w.contentLength = int64(len(p)) - setHeader.contentLength = strconv.Itoa(len(p)) + setHeader.contentLength = strconv.AppendInt(cw.res.clenBuf[:0], int64(len(p)), 10) } // If this was an HTTP/1.0 request with keep-alive and we sent a @@ -709,7 +764,7 @@ func (cw *chunkWriter) writeHeader(p []byte) { // Check for a explicit (and valid) Content-Length header. hasCL := w.contentLength != -1 - if w.req.wantsHttp10KeepAlive() && (w.req.Method == "HEAD" || hasCL) { + if w.req.wantsHttp10KeepAlive() && (isHEAD || hasCL) { _, connectionHeaderSet := header["Connection"] if !connectionHeaderSet { setHeader.connection = "keep-alive" @@ -749,13 +804,14 @@ func (cw *chunkWriter) writeHeader(p []byte) { } } else { // If no content type, apply sniffing algorithm to body. - if header.get("Content-Type") == "" && w.req.Method != "HEAD" { + _, haveType := header["Content-Type"] + if !haveType { setHeader.contentType = DetectContentType(p) } } if _, ok := header["Date"]; !ok { - setHeader.date = time.Now().UTC().Format(TimeFormat) + setHeader.date = appendTime(cw.res.dateBuf[:0], time.Now()) } te := header.get("Transfer-Encoding") @@ -801,12 +857,14 @@ func (cw *chunkWriter) writeHeader(p []byte) { if w.closeAfterReply && !hasToken(cw.header.get("Connection"), "close") { delHeader("Connection") - setHeader.connection = "close" + if w.req.ProtoAtLeast(1, 1) { + setHeader.connection = "close" + } } - io.WriteString(w.conn.buf, statusLine(w.req, code)) + w.conn.buf.WriteString(statusLine(w.req, code)) cw.header.WriteSubset(w.conn.buf, excludeHeader) - setHeader.Write(w.conn.buf) + setHeader.Write(w.conn.buf.Writer) w.conn.buf.Write(crlf) } @@ -861,7 +919,7 @@ func (w *response) bodyAllowed() bool { if !w.wroteHeader { panic("") } - return w.status != StatusNotModified && w.req.Method != "HEAD" + return w.status != StatusNotModified } // The Life Of A Write is like this: @@ -897,6 +955,15 @@ func (w *response) bodyAllowed() bool { // bufferBeforeChunkingSize smaller and having bufio's fast-paths deal // with this instead. func (w *response) Write(data []byte) (n int, err error) { + return w.write(len(data), data, "") +} + +func (w *response) WriteString(data string) (n int, err error) { + return w.write(len(data), nil, data) +} + +// either dataB or dataS is non-zero. +func (w *response) write(lenData int, dataB []byte, dataS string) (n int, err error) { if w.conn.hijacked() { log.Print("http: response.Write on hijacked connection") return 0, ErrHijacked @@ -904,18 +971,22 @@ func (w *response) Write(data []byte) (n int, err error) { if !w.wroteHeader { w.WriteHeader(StatusOK) } - if len(data) == 0 { + if lenData == 0 { return 0, nil } if !w.bodyAllowed() { return 0, ErrBodyNotAllowed } - w.written += int64(len(data)) // ignoring errors, for errorKludge + w.written += int64(lenData) // ignoring errors, for errorKludge if w.contentLength != -1 && w.written > w.contentLength { return 0, ErrContentLength } - return w.w.Write(data) + if dataB != nil { + return w.w.Write(dataB) + } else { + return w.w.WriteString(dataS) + } } func (w *response) finishRequest() { @@ -926,7 +997,7 @@ func (w *response) finishRequest() { } w.w.Flush() - putBufioWriter(w.w, w.sw) + putBufioWriter(w.w) w.cw.close() w.conn.buf.Flush() @@ -939,7 +1010,7 @@ func (w *response) finishRequest() { w.req.MultipartForm.RemoveAll() } - if w.contentLength != -1 && w.bodyAllowed() && w.contentLength != w.written { + if w.req.Method != "HEAD" && w.contentLength != -1 && w.bodyAllowed() && w.contentLength != w.written { // Did not write enough. Avoid getting out of sync. w.closeAfterReply = true } @@ -959,11 +1030,11 @@ func (c *conn) finalFlush() { // Steal the bufio.Reader (~4KB worth of memory) and its associated // reader for a future connection. - putBufioReader(c.buf.Reader, c.bufswr) + putBufioReader(c.buf.Reader) // Steal the bufio.Writer (~4KB worth of memory) and its associated // writer for a future connection. - putBufioWriter(c.buf.Writer, c.bufsww) + putBufioWriter(c.buf.Writer) c.buf = nil } @@ -1001,7 +1072,7 @@ func (c *conn) closeWriteAndWait() { time.Sleep(rstAvoidanceDelay) } -// validNPN returns whether the proto is not a blacklisted Next +// validNPN reports whether the proto is not a blacklisted Next // Protocol Negotiation protocol. Empty and built-in protocol types // are blacklisted and can't be overridden with alternate // implementations. @@ -1152,6 +1223,7 @@ func (f HandlerFunc) ServeHTTP(w ResponseWriter, r *Request) { // Helper handlers // Error replies to the request with the specified error message and HTTP code. +// The error message should be plain text. func Error(w ResponseWriter, error string, code int) { w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(code) @@ -1288,6 +1360,10 @@ func RedirectHandler(url string, code int) Handler { // former will receive requests for any other paths in the // "/images/" subtree. // +// Note that since a pattern ending in a slash names a rooted subtree, +// the pattern "/" matches all paths not matched by other registered +// patterns, not just the URL with Path == "/". +// // Patterns may optionally begin with a host name, restricting matches to // URLs on that host only. Host-specific patterns take precedence over // general patterns, so that a handler might register for the two patterns @@ -1378,7 +1454,9 @@ func (mux *ServeMux) Handler(r *Request) (h Handler, pattern string) { if r.Method != "CONNECT" { if p := cleanPath(r.URL.Path); p != r.URL.Path { _, pattern = mux.handler(r.Host, p) - return RedirectHandler(p, StatusMovedPermanently), pattern + url := *r.URL + url.Path = p + return RedirectHandler(url.String(), StatusMovedPermanently), pattern } } @@ -1408,7 +1486,9 @@ func (mux *ServeMux) handler(host, path string) (h Handler, pattern string) { // pattern most closely matches the request URL. func (mux *ServeMux) ServeHTTP(w ResponseWriter, r *Request) { if r.RequestURI == "*" { - w.Header().Set("Connection", "close") + if r.ProtoAtLeast(1, 1) { + w.Header().Set("Connection", "close") + } w.WriteHeader(StatusBadRequest) return } @@ -1771,7 +1851,15 @@ func (globalOptionsHandler) ServeHTTP(w ResponseWriter, r *Request) { } // eofReader is a non-nil io.ReadCloser that always returns EOF. -var eofReader = ioutil.NopCloser(strings.NewReader("")) +// It embeds a *strings.Reader so it still has a WriteTo method +// and io.Copy won't need a buffer. +var eofReader = &struct { + *strings.Reader + io.Closer +}{ + strings.NewReader(""), + ioutil.NopCloser(nil), +} // initNPNRequest is an HTTP handler that initializes certain // uninitialized fields in its *Request. Such partially-initialized diff --git a/libgo/go/net/http/server_test.go b/libgo/go/net/http/server_test.go deleted file mode 100644 index e8b69f7..0000000 --- a/libgo/go/net/http/server_test.go +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright 2012 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -package http_test - -import ( - . "net/http" - "net/http/httptest" - "net/url" - "testing" -) - -var serveMuxRegister = []struct { - pattern string - h Handler -}{ - {"/dir/", serve(200)}, - {"/search", serve(201)}, - {"codesearch.google.com/search", serve(202)}, - {"codesearch.google.com/", serve(203)}, -} - -// serve returns a handler that sends a response with the given code. -func serve(code int) HandlerFunc { - return func(w ResponseWriter, r *Request) { - w.WriteHeader(code) - } -} - -var serveMuxTests = []struct { - method string - host string - path string - code int - pattern string -}{ - {"GET", "google.com", "/", 404, ""}, - {"GET", "google.com", "/dir", 301, "/dir/"}, - {"GET", "google.com", "/dir/", 200, "/dir/"}, - {"GET", "google.com", "/dir/file", 200, "/dir/"}, - {"GET", "google.com", "/search", 201, "/search"}, - {"GET", "google.com", "/search/", 404, ""}, - {"GET", "google.com", "/search/foo", 404, ""}, - {"GET", "codesearch.google.com", "/search", 202, "codesearch.google.com/search"}, - {"GET", "codesearch.google.com", "/search/", 203, "codesearch.google.com/"}, - {"GET", "codesearch.google.com", "/search/foo", 203, "codesearch.google.com/"}, - {"GET", "codesearch.google.com", "/", 203, "codesearch.google.com/"}, - {"GET", "images.google.com", "/search", 201, "/search"}, - {"GET", "images.google.com", "/search/", 404, ""}, - {"GET", "images.google.com", "/search/foo", 404, ""}, - {"GET", "google.com", "/../search", 301, "/search"}, - {"GET", "google.com", "/dir/..", 301, ""}, - {"GET", "google.com", "/dir/..", 301, ""}, - {"GET", "google.com", "/dir/./file", 301, "/dir/"}, - - // The /foo -> /foo/ redirect applies to CONNECT requests - // but the path canonicalization does not. - {"CONNECT", "google.com", "/dir", 301, "/dir/"}, - {"CONNECT", "google.com", "/../search", 404, ""}, - {"CONNECT", "google.com", "/dir/..", 200, "/dir/"}, - {"CONNECT", "google.com", "/dir/..", 200, "/dir/"}, - {"CONNECT", "google.com", "/dir/./file", 200, "/dir/"}, -} - -func TestServeMuxHandler(t *testing.T) { - mux := NewServeMux() - for _, e := range serveMuxRegister { - mux.Handle(e.pattern, e.h) - } - - for _, tt := range serveMuxTests { - r := &Request{ - Method: tt.method, - Host: tt.host, - URL: &url.URL{ - Path: tt.path, - }, - } - h, pattern := mux.Handler(r) - rr := httptest.NewRecorder() - h.ServeHTTP(rr, r) - if pattern != tt.pattern || rr.Code != tt.code { - t.Errorf("%s %s %s = %d, %q, want %d, %q", tt.method, tt.host, tt.path, rr.Code, pattern, tt.code, tt.pattern) - } - } -} - -func TestServerRedirect(t *testing.T) { - // This used to crash. It's not valid input (bad path), but it - // shouldn't crash. - rr := httptest.NewRecorder() - req := &Request{ - Method: "GET", - URL: &url.URL{ - Scheme: "http", - Path: "not-empty-but-no-leading-slash", // bogus - }, - } - Redirect(rr, req, "", 304) - if rr.Code != 304 { - t.Errorf("Code = %d; want 304", rr.Code) - } -} diff --git a/libgo/go/net/http/sniff_test.go b/libgo/go/net/http/sniff_test.go index 106d94ec..24ca27a 100644 --- a/libgo/go/net/http/sniff_test.go +++ b/libgo/go/net/http/sniff_test.go @@ -12,6 +12,7 @@ import ( "log" . "net/http" "net/http/httptest" + "reflect" "strconv" "strings" "testing" @@ -84,6 +85,29 @@ func TestServerContentType(t *testing.T) { } } +// Issue 5953: shouldn't sniff if the handler set a Content-Type header, +// even if it's the empty string. +func TestServerIssue5953(t *testing.T) { + defer afterTest(t) + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + w.Header()["Content-Type"] = []string{""} + fmt.Fprintf(w, "<html><head></head><body>hi</body></html>") + })) + defer ts.Close() + + resp, err := Get(ts.URL) + if err != nil { + t.Fatal(err) + } + + got := resp.Header["Content-Type"] + want := []string{""} + if !reflect.DeepEqual(got, want) { + t.Errorf("Content-Type = %q; want %q", got, want) + } + resp.Body.Close() +} + func TestContentTypeWithCopy(t *testing.T) { defer afterTest(t) diff --git a/libgo/go/net/http/transfer.go b/libgo/go/net/http/transfer.go index 53569bc..bacd837 100644 --- a/libgo/go/net/http/transfer.go +++ b/libgo/go/net/http/transfer.go @@ -238,7 +238,7 @@ type transferReader struct { Trailer Header } -// bodyAllowedForStatus returns whether a given response status code +// bodyAllowedForStatus reports whether a given response status code // permits a body. See RFC2616, section 4.4. func bodyAllowedForStatus(status int) bool { switch { @@ -254,7 +254,7 @@ func bodyAllowedForStatus(status int) bool { // msg is *Request or *Response. func readTransfer(msg interface{}, r *bufio.Reader) (err error) { - t := &transferReader{} + t := &transferReader{RequestMethod: "GET"} // Unify input isResponse := false @@ -262,11 +262,13 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { case *Response: t.Header = rr.Header t.StatusCode = rr.StatusCode - t.RequestMethod = rr.Request.Method t.ProtoMajor = rr.ProtoMajor t.ProtoMinor = rr.ProtoMinor t.Close = shouldClose(t.ProtoMajor, t.ProtoMinor, t.Header) isResponse = true + if rr.Request != nil { + t.RequestMethod = rr.Request.Method + } case *Request: t.Header = rr.Header t.ProtoMajor = rr.ProtoMajor @@ -274,7 +276,6 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { // Transfer semantics for Requests are exactly like those for // Responses with status code 200, responding to a GET method t.StatusCode = 200 - t.RequestMethod = "GET" default: panic("unexpected type") } @@ -328,12 +329,12 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { switch { case chunked(t.TransferEncoding): if noBodyExpected(t.RequestMethod) { - t.Body = &body{Reader: eofReader, closing: t.Close} + t.Body = eofReader } else { t.Body = &body{Reader: newChunkedReader(r), hdr: msg, r: r, closing: t.Close} } case realLength == 0: - t.Body = &body{Reader: eofReader, closing: t.Close} + t.Body = eofReader case realLength > 0: t.Body = &body{Reader: io.LimitReader(r, realLength), closing: t.Close} default: @@ -343,7 +344,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { t.Body = &body{Reader: r, closing: t.Close} } else { // Persistent connection (i.e. HTTP/1.1) - t.Body = &body{Reader: eofReader, closing: t.Close} + t.Body = eofReader } } @@ -518,8 +519,6 @@ type body struct { r *bufio.Reader // underlying wire-format reader for the trailer closing bool // is the connection to be closed after reading body? closed bool - - res *response // response writer for server requests, else nil } // ErrBodyReadAfterClose is returned when reading a Request or Response @@ -534,13 +533,22 @@ func (b *body) Read(p []byte) (n int, err error) { } n, err = b.Reader.Read(p) - // Read the final trailer once we hit EOF. - if err == io.EOF && b.hdr != nil { - if e := b.readTrailer(); e != nil { - err = e + if err == io.EOF { + // Chunked case. Read the trailer. + if b.hdr != nil { + if e := b.readTrailer(); e != nil { + err = e + } + b.hdr = nil + } else { + // If the server declared the Content-Length, our body is a LimitedReader + // and we need to check whether this EOF arrived early. + if lr, ok := b.Reader.(*io.LimitedReader); ok && lr.N > 0 { + err = io.ErrUnexpectedEOF + } } - b.hdr = nil } + return n, err } @@ -618,14 +626,6 @@ func (b *body) Close() error { case b.hdr == nil && b.closing: // no trailer and closing the connection next. // no point in reading to EOF. - case b.res != nil && b.res.requestBodyLimitHit: - // In a server request, don't continue reading from the client - // if we've already hit the maximum body size set by the - // handler. If this is set, that also means the TCP connection - // is about to be closed, so getting to the next HTTP request - // in the stream is not necessary. - case b.Reader == eofReader: - // Nothing to read. No need to io.Copy from it. default: // Fully consume the body, which will also lead to us reading // the trailer headers after the body, if present. diff --git a/libgo/go/net/http/transport.go b/libgo/go/net/http/transport.go index 4cd0533..f6871af 100644 --- a/libgo/go/net/http/transport.go +++ b/libgo/go/net/http/transport.go @@ -13,7 +13,6 @@ import ( "bufio" "compress/gzip" "crypto/tls" - "encoding/base64" "errors" "fmt" "io" @@ -109,9 +108,11 @@ func ProxyFromEnvironment(req *Request) (*url.URL, error) { } proxyURL, err := url.Parse(proxy) if err != nil || !strings.HasPrefix(proxyURL.Scheme, "http") { - if u, err := url.Parse("http://" + proxy); err == nil { - proxyURL = u - err = nil + // proxy was bogus. Try prepending "http://" to it and + // see if that parses correctly. If not, we fall + // through and complain about the original one. + if proxyURL, err := url.Parse("http://" + proxy); err == nil { + return proxyURL, nil } } if err != nil { @@ -215,6 +216,7 @@ func (t *Transport) CloseIdleConnections() { t.idleMu.Lock() m := t.idleConn t.idleConn = nil + t.idleConnCh = nil t.idleMu.Unlock() if m == nil { return @@ -270,7 +272,9 @@ func (cm *connectMethod) proxyAuth() string { return "" } if u := cm.proxyURL.User; u != nil { - return "Basic " + base64.URLEncoding.EncodeToString([]byte(u.String())) + username := u.Username() + password, _ := u.Password() + return "Basic " + basicAuth(username, password) } return "" } @@ -293,8 +297,10 @@ func (t *Transport) putIdleConn(pconn *persistConn) bool { max = DefaultMaxIdleConnsPerHost } t.idleMu.Lock() + + waitingDialer := t.idleConnCh[key] select { - case t.idleConnCh[key] <- pconn: + case waitingDialer <- pconn: // We're done with this pconn and somebody else is // currently waiting for a conn of this type (they're // actively dialing, but this conn is ready @@ -303,6 +309,11 @@ func (t *Transport) putIdleConn(pconn *persistConn) bool { t.idleMu.Unlock() return true default: + if waitingDialer != nil { + // They had populated this, but their dial won + // first, so we can clean up this map entry. + delete(t.idleConnCh, key) + } } if t.idleConn == nil { t.idleConn = make(map[string][]*persistConn) @@ -322,7 +333,13 @@ func (t *Transport) putIdleConn(pconn *persistConn) bool { return true } +// getIdleConnCh returns a channel to receive and return idle +// persistent connection for the given connectMethod. +// It may return nil, if persistent connections are not being used. func (t *Transport) getIdleConnCh(cm *connectMethod) chan *persistConn { + if t.DisableKeepAlives { + return nil + } key := cm.key() t.idleMu.Lock() defer t.idleMu.Unlock() @@ -498,8 +515,8 @@ func (t *Transport) dialConn(cm *connectMethod) (*persistConn, error) { if err = conn.(*tls.Conn).Handshake(); err != nil { return nil, err } - if t.TLSClientConfig == nil || !t.TLSClientConfig.InsecureSkipVerify { - if err = conn.(*tls.Conn).VerifyHostname(cm.tlsHost()); err != nil { + if !cfg.InsecureSkipVerify { + if err = conn.(*tls.Conn).VerifyHostname(cfg.ServerName); err != nil { return nil, err } } @@ -831,10 +848,15 @@ func (pc *persistConn) roundTrip(req *transportRequest) (resp *Response, err err // uncompress the gzip stream if we were the layer that // requested it. requestedGzip := false - if !pc.t.DisableCompression && req.Header.Get("Accept-Encoding") == "" { + if !pc.t.DisableCompression && req.Header.Get("Accept-Encoding") == "" && req.Method != "HEAD" { // Request gzip only, not deflate. Deflate is ambiguous and // not as universally supported anyway. // See: http://www.gzip.org/zlib/zlib_faq.html#faq38 + // + // Note that we don't request this for HEAD requests, + // due to a bug in nginx: + // http://trac.nginx.org/nginx/ticket/358 + // http://golang.org/issue/5522 requestedGzip = true req.extraHeaders().Set("Accept-Encoding", "gzip") } diff --git a/libgo/go/net/http/transport_test.go b/libgo/go/net/http/transport_test.go index 9f64a6e..e4df30a 100644 --- a/libgo/go/net/http/transport_test.go +++ b/libgo/go/net/http/transport_test.go @@ -15,6 +15,7 @@ import ( "io" "io/ioutil" "net" + "net/http" . "net/http" "net/http/httptest" "net/url" @@ -469,6 +470,7 @@ func TestTransportHeadResponses(t *testing.T) { res, err := c.Head(ts.URL) if err != nil { t.Errorf("error on loop %d: %v", i, err) + continue } if e, g := "123", res.Header.Get("Content-Length"); e != g { t.Errorf("loop %d: expected Content-Length header of %q, got %q", i, e, g) @@ -476,6 +478,11 @@ func TestTransportHeadResponses(t *testing.T) { if e, g := int64(123), res.ContentLength; e != g { t.Errorf("loop %d: expected res.ContentLength of %v, got %v", i, e, g) } + if all, err := ioutil.ReadAll(res.Body); err != nil { + t.Errorf("loop %d: Body ReadAll: %v", i, err) + } else if len(all) != 0 { + t.Errorf("Bogus body %q", all) + } } } @@ -553,12 +560,13 @@ func TestRoundTripGzip(t *testing.T) { res, err := DefaultTransport.RoundTrip(req) var body []byte if test.compressed { - gzip, err := gzip.NewReader(res.Body) + var r *gzip.Reader + r, err = gzip.NewReader(res.Body) if err != nil { t.Errorf("%d. gzip NewReader: %v", i, err) continue } - body, err = ioutil.ReadAll(gzip) + body, err = ioutil.ReadAll(r) res.Body.Close() } else { body, err = ioutil.ReadAll(res.Body) @@ -585,13 +593,16 @@ func TestTransportGzip(t *testing.T) { const testString = "The test string aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa" const nRandBytes = 1024 * 1024 ts := httptest.NewServer(HandlerFunc(func(rw ResponseWriter, req *Request) { + if req.Method == "HEAD" { + if g := req.Header.Get("Accept-Encoding"); g != "" { + t.Errorf("HEAD request sent with Accept-Encoding of %q; want none", g) + } + return + } if g, e := req.Header.Get("Accept-Encoding"), "gzip"; g != e { t.Errorf("Accept-Encoding = %q, want %q", g, e) } rw.Header().Set("Content-Encoding", "gzip") - if req.Method == "HEAD" { - return - } var w io.Writer = rw var buf bytes.Buffer @@ -819,7 +830,7 @@ func TestTransportPersistConnLeakShortBody(t *testing.T) { } nhigh := runtime.NumGoroutine() tr.CloseIdleConnections() - time.Sleep(50 * time.Millisecond) + time.Sleep(400 * time.Millisecond) runtime.GC() nfinal := runtime.NumGoroutine() @@ -1571,6 +1582,77 @@ func TestProxyFromEnvironment(t *testing.T) { } } +func TestIdleConnChannelLeak(t *testing.T) { + var mu sync.Mutex + var n int + + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + mu.Lock() + n++ + mu.Unlock() + })) + defer ts.Close() + + tr := &Transport{ + Dial: func(netw, addr string) (net.Conn, error) { + return net.Dial(netw, ts.Listener.Addr().String()) + }, + } + defer tr.CloseIdleConnections() + + c := &Client{Transport: tr} + + // First, without keep-alives. + for _, disableKeep := range []bool{true, false} { + tr.DisableKeepAlives = disableKeep + for i := 0; i < 5; i++ { + _, err := c.Get(fmt.Sprintf("http://foo-host-%d.tld/", i)) + if err != nil { + t.Fatal(err) + } + } + if got := tr.IdleConnChMapSizeForTesting(); got != 0 { + t.Fatalf("ForDisableKeepAlives = %v, map size = %d; want 0", disableKeep, got) + } + } +} + +// Verify the status quo: that the Client.Post function coerces its +// body into a ReadCloser if it's a Closer, and that the Transport +// then closes it. +func TestTransportClosesRequestBody(t *testing.T) { + defer afterTest(t) + ts := httptest.NewServer(http.HandlerFunc(func(w ResponseWriter, r *Request) { + io.Copy(ioutil.Discard, r.Body) + })) + defer ts.Close() + + tr := &Transport{} + defer tr.CloseIdleConnections() + cl := &Client{Transport: tr} + + closes := 0 + + res, err := cl.Post(ts.URL, "text/plain", countCloseReader{&closes, strings.NewReader("hello")}) + if err != nil { + t.Fatal(err) + } + res.Body.Close() + if closes != 1 { + t.Errorf("closes = %d; want 1", closes) + } +} + +type countCloseReader struct { + n *int + io.Reader +} + +func (cr countCloseReader) Close() error { + (*cr.n)++ + return nil +} + // rgz is a gzip quine that uncompresses to itself. var rgz = []byte{ 0x1f, 0x8b, 0x08, 0x08, 0x00, 0x00, 0x00, 0x00, diff --git a/libgo/go/net/http/z_last_test.go b/libgo/go/net/http/z_last_test.go index 2161db7..5a0cc11 100644 --- a/libgo/go/net/http/z_last_test.go +++ b/libgo/go/net/http/z_last_test.go @@ -23,7 +23,6 @@ func interestingGoroutines() (gs []string) { } stack := strings.TrimSpace(sl[1]) if stack == "" || - strings.Contains(stack, "created by net.newPollServer") || strings.Contains(stack, "created by net.startServer") || strings.Contains(stack, "created by testing.RunTests") || strings.Contains(stack, "closeWriteAndWait") || |