From cb284313f7e24319e7d22a551bd04ad9632db659 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 5 Apr 2016 20:01:50 +0000 Subject: net/http, net/http/httputil: start with capitals in deprecation sentences Fixes #15139 Change-Id: I73111137907e612af871b77ccf166572bf78c840 Reviewed-on: https://go-review.googlesource.com/21544 Reviewed-by: Andrew Gerrand --- src/net/http/httputil/persist.go | 10 +++++----- src/net/http/request.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/httputil/persist.go b/src/net/http/httputil/persist.go index 7874da3bec..51486e78e2 100644 --- a/src/net/http/httputil/persist.go +++ b/src/net/http/httputil/persist.go @@ -28,7 +28,7 @@ var errClosed = errors.New("i/o operation on closed connection") // Is is low-level, old, and unused by Go's current HTTP stack. // We should have deleted it before Go 1. // -// Deprecated: use the Server in package net/http instead. +// Deprecated: Use the Server in package net/http instead. type ServerConn struct { mu sync.Mutex // read-write protects the following fields c net.Conn @@ -45,7 +45,7 @@ type ServerConn struct { // Is is low-level, old, and unused by Go's current HTTP stack. // We should have deleted it before Go 1. // -// Deprecated: use the Server in package net/http instead. +// Deprecated: Use the Server in package net/http instead. func NewServerConn(c net.Conn, r *bufio.Reader) *ServerConn { if r == nil { r = bufio.NewReader(c) @@ -221,7 +221,7 @@ func (sc *ServerConn) Write(req *http.Request, resp *http.Response) error { // Is is low-level, old, and unused by Go's current HTTP stack. // We should have deleted it before Go 1. // -// Deprecated: use Client or Transport in package net/http instead. +// Deprecated: Use Client or Transport in package net/http instead. type ClientConn struct { mu sync.Mutex // read-write protects the following fields c net.Conn @@ -239,7 +239,7 @@ type ClientConn struct { // Is is low-level, old, and unused by Go's current HTTP stack. // We should have deleted it before Go 1. // -// Deprecated: use the Client or Transport in package net/http instead. +// Deprecated: Use the Client or Transport in package net/http instead. func NewClientConn(c net.Conn, r *bufio.Reader) *ClientConn { if r == nil { r = bufio.NewReader(c) @@ -256,7 +256,7 @@ func NewClientConn(c net.Conn, r *bufio.Reader) *ClientConn { // Is is low-level, old, and unused by Go's current HTTP stack. // We should have deleted it before Go 1. // -// Deprecated: use the Client or Transport in package net/http instead. +// Deprecated: Use the Client or Transport in package net/http instead. func NewProxyClientConn(c net.Conn, r *bufio.Reader) *ClientConn { cc := NewClientConn(c, r) cc.writeReq = (*http.Request).WriteProxy diff --git a/src/net/http/request.go b/src/net/http/request.go index 371d36b097..5510691912 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -249,7 +249,7 @@ type Request struct { // // For server requests, this field is not applicable. // - // Deprecated: use the Context and WithContext methods + // Deprecated: Use the Context and WithContext methods // instead. If a Request's Cancel field and context are both // set, it is undefined whether Cancel is respected. Cancel <-chan struct{} -- cgit v1.3 From e0307c25bebd694b98ae538065cda0681ef9ecf1 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 5 Apr 2016 15:59:55 +0000 Subject: net/http: document that Handlers shouldn't mutate Request Also, don't read from the Request.Headers in the http Server code once ServeHTTP has started. This is partially redundant with documenting that handlers shouldn't mutate request, but: the space is free due to bool packing, it's faster to do the checks once instead of N times in writeChunk, and it's a little nicer to code which previously didn't play by the unwritten rules. But I'm not going to fix all the cases. Fixes #14940 Change-Id: I612a8826b41c8682b59515081c590c512ee6949e Reviewed-on: https://go-review.googlesource.com/21530 Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot Reviewed-by: Andrew Gerrand --- src/net/http/server.go | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/server.go b/src/net/http/server.go index a2ef0ddf20..f4e697169d 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -50,6 +50,9 @@ var ( // ResponseWriter. Cautious handlers should read the Request.Body // first, and then reply. // +// Except for reading the body, handlers should not modify the +// provided Request. +// // If ServeHTTP panics, the server (the caller of ServeHTTP) assumes // that the effect of the panic was isolated to the active request. // It recovers the panic, logs a stack trace to the server error log, @@ -306,11 +309,13 @@ func (cw *chunkWriter) close() { // A response represents the server side of an HTTP response. type response struct { - conn *conn - req *Request // request for this response - reqBody io.ReadCloser - wroteHeader bool // reply header has been (logically) written - wroteContinue bool // 100 Continue response was written + conn *conn + req *Request // request for this response + reqBody io.ReadCloser + wroteHeader bool // reply header has been (logically) written + wroteContinue bool // 100 Continue response was written + wants10KeepAlive bool // HTTP/1.0 w/ Connection "keep-alive" + wantsClose bool // HTTP request has Connection "close" w *bufio.Writer // buffers output in chunks to chunkWriter cw chunkWriter @@ -748,6 +753,12 @@ func (c *conn) readRequest() (w *response, err error) { reqBody: req.Body, handlerHeader: make(Header), contentLength: -1, + + // We populate these ahead of time so we're not + // reading from req.Header after their Handler starts + // and maybe mutates it (Issue 14940) + wants10KeepAlive: req.wantsHttp10KeepAlive(), + wantsClose: req.wantsClose(), } if isH2Upgrade { w.closeAfterReply = true @@ -929,7 +940,7 @@ func (cw *chunkWriter) writeHeader(p []byte) { // If this was an HTTP/1.0 request with keep-alive and we sent a // Content-Length back, we can make this a keep-alive response ... - if w.req.wantsHttp10KeepAlive() && keepAlivesEnabled { + if w.wants10KeepAlive && keepAlivesEnabled { sentLength := header.get("Content-Length") != "" if sentLength && header.get("Connection") == "keep-alive" { w.closeAfterReply = false @@ -939,12 +950,12 @@ func (cw *chunkWriter) writeHeader(p []byte) { // Check for a explicit (and valid) Content-Length header. hasCL := w.contentLength != -1 - if w.req.wantsHttp10KeepAlive() && (isHEAD || hasCL) { + if w.wants10KeepAlive && (isHEAD || hasCL) { _, connectionHeaderSet := header["Connection"] if !connectionHeaderSet { setHeader.connection = "keep-alive" } - } else if !w.req.ProtoAtLeast(1, 1) || w.req.wantsClose() { + } else if !w.req.ProtoAtLeast(1, 1) || w.wantsClose { w.closeAfterReply = true } -- cgit v1.3 From 870d997ab47fe88c33f4dadef38d7e85eeabf17c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 5 Apr 2016 17:24:23 +0000 Subject: net/http: keep request context during Client redirects Change-Id: I25c51280ba55120ffeaf08222f5ac5d471632d89 Reviewed-on: https://go-review.googlesource.com/21535 Reviewed-by: Andrew Gerrand Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/client.go | 1 + src/net/http/client_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) (limited to 'src/net/http') diff --git a/src/net/http/client.go b/src/net/http/client.go index 10f5684a79..ee0fd2cb62 100644 --- a/src/net/http/client.go +++ b/src/net/http/client.go @@ -475,6 +475,7 @@ func (c *Client) doFollowingRedirects(req *Request, shouldRedirect func(int) boo URL: u, Header: make(Header), Cancel: ireq.Cancel, + ctx: ireq.ctx, } if ireq.Method == "POST" || ireq.Method == "PUT" { req.Method = "GET" diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go index e4fed26803..b9e17c5270 100644 --- a/src/net/http/client_test.go +++ b/src/net/http/client_test.go @@ -8,6 +8,7 @@ package http_test import ( "bytes" + "context" "crypto/tls" "crypto/x509" "encoding/base64" @@ -290,6 +291,33 @@ func TestClientRedirects(t *testing.T) { } } +func TestClientRedirectContext(t *testing.T) { + defer afterTest(t) + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + Redirect(w, r, "/", StatusFound) + })) + defer ts.Close() + + ctx, cancel := context.WithCancel(context.Background()) + c := &Client{CheckRedirect: func(req *Request, via []*Request) error { + cancel() + if len(via) > 2 { + return errors.New("too many redirects") + } + return nil + }} + req, _ := NewRequest("GET", ts.URL, nil) + req = req.WithContext(ctx) + _, err := c.Do(req) + ue, ok := err.(*url.Error) + if !ok { + t.Fatalf("got error %T; want *url.Error") + } + if ue.Err != ExportErrRequestCanceled && ue.Err != ExportErrRequestCanceledConn { + t.Errorf("url.Error.Err = %v; want errRequestCanceled or errRequestCanceledConn", ue.Err) + } +} + func TestPostRedirects(t *testing.T) { defer afterTest(t) var log struct { -- cgit v1.3 From b2fc9f1c23453e16ab08d411ed0e439212d6e5e6 Mon Sep 17 00:00:00 2001 From: Dan Peterson Date: Wed, 6 Apr 2016 11:18:55 -0300 Subject: net/http/pprof: note calling runtime.SetBlockProfileRate is required for block profile Fixes #15076 Change-Id: I5ce8f6253245d8cc1f862a1bf618775f557f955d Reviewed-on: https://go-review.googlesource.com/21610 Reviewed-by: Brad Fitzpatrick --- src/net/http/pprof/pprof.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/net/http') diff --git a/src/net/http/pprof/pprof.go b/src/net/http/pprof/pprof.go index 2357d8ed1e..44afa2d8d8 100644 --- a/src/net/http/pprof/pprof.go +++ b/src/net/http/pprof/pprof.go @@ -30,7 +30,8 @@ // // go tool pprof http://localhost:6060/debug/pprof/profile // -// Or to look at the goroutine blocking profile: +// Or to look at the goroutine blocking profile, after calling +// runtime.SetBlockProfileRate in your program: // // go tool pprof http://localhost:6060/debug/pprof/block // -- cgit v1.3 From 2cefd12a1bf7ee1d1aad03e17c4680d4b611d6da Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 6 Apr 2016 19:02:27 +0000 Subject: net, runtime: skip flaky tests on OpenBSD Flaky tests are a distraction and cover up real problems. File bugs instead and mark them as flaky. This moves the net/http flaky test flagging mechanism to internal/testenv. Updates #15156 Updates #15157 Updates #15158 Change-Id: I0e561cd2a09c0dec369cd4ed93bc5a2b40233dfe Reviewed-on: https://go-review.googlesource.com/21614 Reviewed-by: Matthew Dempsky Run-TryBot: Brad Fitzpatrick --- src/context/context_test.go | 4 ++++ src/go/build/deps_test.go | 2 +- src/internal/testenv/testenv.go | 9 +++++++++ src/net/dial_test.go | 4 ++++ src/net/http/main_test.go | 9 --------- src/net/http/transport_test.go | 3 ++- src/net/timeout_test.go | 4 ++++ src/net/unixsock_test.go | 4 ++++ src/runtime/pprof/pprof_test.go | 3 +++ 9 files changed, 31 insertions(+), 11 deletions(-) (limited to 'src/net/http') diff --git a/src/context/context_test.go b/src/context/context_test.go index 05345fc5e5..60020303c7 100644 --- a/src/context/context_test.go +++ b/src/context/context_test.go @@ -6,6 +6,7 @@ package context import ( "fmt" + "internal/testenv" "math/rand" "runtime" "strings" @@ -258,6 +259,9 @@ func TestDeadline(t *testing.T) { } func TestTimeout(t *testing.T) { + if runtime.GOOS == "openbsd" { + testenv.SkipFlaky(t, 15158) + } c, _ := WithTimeout(Background(), 100*time.Millisecond) if got, prefix := fmt.Sprint(c), "context.Background.WithDeadline("; !strings.HasPrefix(got, prefix) { t.Errorf("c.String() = %q want prefix %q", got, prefix) diff --git a/src/go/build/deps_test.go b/src/go/build/deps_test.go index c066048630..8e2fd6e584 100644 --- a/src/go/build/deps_test.go +++ b/src/go/build/deps_test.go @@ -168,7 +168,7 @@ var pkgDeps = map[string][]string{ "testing": {"L2", "flag", "fmt", "os", "runtime/debug", "runtime/pprof", "runtime/trace", "time"}, "testing/iotest": {"L2", "log"}, "testing/quick": {"L2", "flag", "fmt", "reflect"}, - "internal/testenv": {"L2", "OS", "testing"}, + "internal/testenv": {"L2", "OS", "flag", "testing"}, // L4 is defined as L3+fmt+log+time, because in general once // you're using L3 packages, use of fmt, log, or time is not a big deal. diff --git a/src/internal/testenv/testenv.go b/src/internal/testenv/testenv.go index e751e0cf11..9e684e3034 100644 --- a/src/internal/testenv/testenv.go +++ b/src/internal/testenv/testenv.go @@ -11,6 +11,7 @@ package testenv import ( + "flag" "os" "os/exec" "path/filepath" @@ -124,3 +125,11 @@ func MustHaveExternalNetwork(t *testing.T) { t.Skipf("skipping test: no external network in -short mode") } } + +var flaky = flag.Bool("flaky", false, "run known-flaky tests too") + +func SkipFlaky(t *testing.T, issue int) { + if !*flaky { + t.Skipf("skipping known flaky test without the -flaky flag; see golang.org/issue/%d", issue) + } +} diff --git a/src/net/dial_test.go b/src/net/dial_test.go index 2fc75c6356..f8e90abb48 100644 --- a/src/net/dial_test.go +++ b/src/net/dial_test.go @@ -59,6 +59,8 @@ func TestDialTimeoutFDLeak(t *testing.T) { switch runtime.GOOS { case "plan9": t.Skipf("%s does not have full support of socktest", runtime.GOOS) + case "openbsd": + testenv.SkipFlaky(t, 15157) } const T = 100 * time.Millisecond @@ -126,6 +128,8 @@ func TestDialerDualStackFDLeak(t *testing.T) { t.Skipf("%s does not have full support of socktest", runtime.GOOS) case "windows": t.Skipf("not implemented a way to cancel dial racers in TCP SYN-SENT state on %s", runtime.GOOS) + case "openbsd": + testenv.SkipFlaky(t, 15157) } if !supportsIPv4 || !supportsIPv6 { t.Skip("both IPv4 and IPv6 are required") diff --git a/src/net/http/main_test.go b/src/net/http/main_test.go index 299cd7b2d2..1163874ac2 100644 --- a/src/net/http/main_test.go +++ b/src/net/http/main_test.go @@ -5,7 +5,6 @@ package http_test import ( - "flag" "fmt" "net/http" "os" @@ -16,8 +15,6 @@ import ( "time" ) -var flaky = flag.Bool("flaky", false, "run known-flaky tests too") - func TestMain(m *testing.M) { v := m.Run() if v == 0 && goroutineLeaked() { @@ -91,12 +88,6 @@ func setParallel(t *testing.T) { } } -func setFlaky(t *testing.T, issue int) { - if !*flaky { - t.Skipf("skipping known flaky test; see golang.org/issue/%d", issue) - } -} - func afterTest(t testing.TB) { http.DefaultTransport.(*http.Transport).CloseIdleConnections() if testing.Short() { diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 7a01dca394..1aa26610b0 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -18,6 +18,7 @@ import ( "crypto/tls" "errors" "fmt" + "internal/testenv" "io" "io/ioutil" "log" @@ -2229,7 +2230,7 @@ func TestTransportTLSHandshakeTimeout(t *testing.T) { // Trying to repro golang.org/issue/3514 func TestTLSServerClosesConnection(t *testing.T) { defer afterTest(t) - setFlaky(t, 7634) + testenv.SkipFlaky(t, 7634) closedc := make(chan bool, 1) ts := httptest.NewTLSServer(HandlerFunc(func(w ResponseWriter, r *Request) { diff --git a/src/net/timeout_test.go b/src/net/timeout_test.go index d80e478c77..3ea0ec1ebd 100644 --- a/src/net/timeout_test.go +++ b/src/net/timeout_test.go @@ -6,6 +6,7 @@ package net import ( "fmt" + "internal/testenv" "io" "io/ioutil" "net/internal/socktest" @@ -112,6 +113,9 @@ var dialTimeoutMaxDurationTests = []struct { func TestDialTimeoutMaxDuration(t *testing.T) { t.Parallel() + if runtime.GOOS == "openbsd" { + testenv.SkipFlaky(t, 15157) + } ln, err := newLocalListener("tcp") if err != nil { diff --git a/src/net/unixsock_test.go b/src/net/unixsock_test.go index d70c0d1953..f0f88ed37b 100644 --- a/src/net/unixsock_test.go +++ b/src/net/unixsock_test.go @@ -8,6 +8,7 @@ package net import ( "bytes" + "internal/testenv" "os" "reflect" "runtime" @@ -20,6 +21,9 @@ func TestReadUnixgramWithUnnamedSocket(t *testing.T) { if !testableNetwork("unixgram") { t.Skip("unixgram test") } + if runtime.GOOS == "openbsd" { + testenv.SkipFlaky(t, 15157) + } addr := testUnixAddr() la, err := ResolveUnixAddr("unixgram", addr) diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index fa0af59b37..23bc72c1e4 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -585,6 +585,9 @@ func func3(c chan int) { <-c } func func4(c chan int) { <-c } func TestGoroutineCounts(t *testing.T) { + if runtime.GOOS == "openbsd" { + testenv.SkipFlaky(t, 15156) + } c := make(chan int) for i := 0; i < 100; i++ { if i%10 == 0 { -- cgit v1.3 From 853f1a1a63b759686421196d419ddaa626a44bf5 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Thu, 7 Apr 2016 19:59:59 -0700 Subject: net/http: fixed trivial go vet warnings Updates #15177 Change-Id: I748f025461f313b5b426821ead695f90d3011a6b Reviewed-on: https://go-review.googlesource.com/21677 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/client_test.go | 2 +- src/net/http/httptest/httptest_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/client_test.go b/src/net/http/client_test.go index b9e17c5270..a9b30b1bf5 100644 --- a/src/net/http/client_test.go +++ b/src/net/http/client_test.go @@ -311,7 +311,7 @@ func TestClientRedirectContext(t *testing.T) { _, err := c.Do(req) ue, ok := err.(*url.Error) if !ok { - t.Fatalf("got error %T; want *url.Error") + t.Fatalf("got error %T; want *url.Error", err) } if ue.Err != ExportErrRequestCanceled && ue.Err != ExportErrRequestCanceledConn { t.Errorf("url.Error.Err = %v; want errRequestCanceled or errRequestCanceledConn", ue.Err) diff --git a/src/net/http/httptest/httptest_test.go b/src/net/http/httptest/httptest_test.go index 18ba73880e..4f9ecbd8bb 100644 --- a/src/net/http/httptest/httptest_test.go +++ b/src/net/http/httptest/httptest_test.go @@ -155,10 +155,10 @@ func TestNewRequest(t *testing.T) { got := NewRequest(tt.method, tt.uri, tt.body) slurp, err := ioutil.ReadAll(got.Body) if err != nil { - t.Errorf("%i. ReadAll: %v", i, err) + t.Errorf("%d. ReadAll: %v", i, err) } if string(slurp) != tt.wantBody { - t.Errorf("%i. Body = %q; want %q", i, slurp, tt.wantBody) + t.Errorf("%d. Body = %q; want %q", i, slurp, tt.wantBody) } got.Body = nil // before DeepEqual if !reflect.DeepEqual(got.URL, tt.want.URL) { -- cgit v1.3 From 3598d4b8387a9e1c9afd522e0d18201f855f613b Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Sat, 9 Apr 2016 00:34:18 -0700 Subject: net/http/httputil: DumpRequest dumps Content-Length if set in header Fixes #7215 Change-Id: I108171ef18cac66d0dc11ce3553c26fc49529807 Reviewed-on: https://go-review.googlesource.com/21790 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick --- src/net/http/httputil/dump.go | 1 - src/net/http/httputil/dump_test.go | 40 +++++++++++++++++++++++++++++++++++ src/net/http/httputil/example_test.go | 2 +- 3 files changed, 41 insertions(+), 2 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/httputil/dump.go b/src/net/http/httputil/dump.go index ddde11a0e4..692ab62c9b 100644 --- a/src/net/http/httputil/dump.go +++ b/src/net/http/httputil/dump.go @@ -163,7 +163,6 @@ func valueOrDefault(value, def string) string { var reqWriteExcludeHeaderDump = map[string]bool{ "Host": true, // not in Header map anyway - "Content-Length": true, "Transfer-Encoding": true, "Trailer": true, } diff --git a/src/net/http/httputil/dump_test.go b/src/net/http/httputil/dump_test.go index fc884347a6..2e980d39f8 100644 --- a/src/net/http/httputil/dump_test.go +++ b/src/net/http/httputil/dump_test.go @@ -122,6 +122,10 @@ var dumpTests = []dumpTest{ Host: "post.tld", Path: "/", }, + Header: http.Header{ + "Content-Length": []string{"8193"}, + }, + ContentLength: 8193, ProtoMajor: 1, ProtoMinor: 1, @@ -135,6 +139,10 @@ var dumpTests = []dumpTest{ "Content-Length: 8193\r\n" + "Accept-Encoding: gzip\r\n\r\n" + strings.Repeat("a", 8193), + WantDump: "POST / HTTP/1.1\r\n" + + "Host: post.tld\r\n" + + "Content-Length: 8193\r\n\r\n" + + strings.Repeat("a", 8193), }, { @@ -144,6 +152,38 @@ var dumpTests = []dumpTest{ WantDump: "GET http://foo.com/ HTTP/1.1\r\n" + "User-Agent: blah\r\n\r\n", }, + + // Issue #7215. DumpRequest should return the "Content-Length" when set + { + Req: *mustReadRequest("POST /v2/api/?login HTTP/1.1\r\n" + + "Host: passport.myhost.com\r\n" + + "Content-Length: 3\r\n" + + "\r\nkey1=name1&key2=name2"), + WantDump: "POST /v2/api/?login HTTP/1.1\r\n" + + "Host: passport.myhost.com\r\n" + + "Content-Length: 3\r\n" + + "\r\nkey", + }, + + // Issue #7215. DumpRequest should return the "Content-Length" in ReadRequest + { + Req: *mustReadRequest("POST /v2/api/?login HTTP/1.1\r\n" + + "Host: passport.myhost.com\r\n" + + "Content-Length: 0\r\n" + + "\r\nkey1=name1&key2=name2"), + WantDump: "POST /v2/api/?login HTTP/1.1\r\n" + + "Host: passport.myhost.com\r\n" + + "Content-Length: 0\r\n\r\n", + }, + + // Issue #7215. DumpRequest should not return the "Content-Length" if unset + { + Req: *mustReadRequest("POST /v2/api/?login HTTP/1.1\r\n" + + "Host: passport.myhost.com\r\n" + + "\r\nkey1=name1&key2=name2"), + WantDump: "POST /v2/api/?login HTTP/1.1\r\n" + + "Host: passport.myhost.com\r\n\r\n", + }, } func TestDumpRequest(t *testing.T) { diff --git a/src/net/http/httputil/example_test.go b/src/net/http/httputil/example_test.go index f856135742..6191603674 100644 --- a/src/net/http/httputil/example_test.go +++ b/src/net/http/httputil/example_test.go @@ -47,7 +47,7 @@ func ExampleDumpRequest() { fmt.Printf("%s", b) // Output: - // "POST / HTTP/1.1\r\nHost: www.example.org\r\nAccept-Encoding: gzip\r\nUser-Agent: Go-http-client/1.1\r\n\r\nGo is a general-purpose language designed with systems programming in mind." + // "POST / HTTP/1.1\r\nHost: www.example.org\r\nAccept-Encoding: gzip\r\nContent-Length: 75\r\nUser-Agent: Go-http-client/1.1\r\n\r\nGo is a general-purpose language designed with systems programming in mind." } func ExampleDumpRequestOut() { -- cgit v1.3 From 012557b3769f9286b9488fbfd4bddfeee66b6a55 Mon Sep 17 00:00:00 2001 From: Martin Möhrmann Date: Sun, 10 Apr 2016 08:48:55 +0200 Subject: all: replace magic 0x80 with named constant utf8.RuneSelf Change-Id: Id1c2e8e9d60588de866e8b6ca59cc83dd28f848f Reviewed-on: https://go-review.googlesource.com/21756 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/bufio/bufio.go | 2 +- src/cmd/compile/internal/gc/fmt.go | 2 +- src/encoding/asn1/asn1.go | 2 +- src/go/build/build.go | 2 +- src/go/build/read.go | 3 ++- src/go/scanner/scanner.go | 6 +++--- src/html/template/css.go | 2 +- src/net/http/cookiejar/punycode.go | 2 +- 8 files changed, 11 insertions(+), 10 deletions(-) (limited to 'src/net/http') diff --git a/src/bufio/bufio.go b/src/bufio/bufio.go index d2ccc74f52..3b30b8b80c 100644 --- a/src/bufio/bufio.go +++ b/src/bufio/bufio.go @@ -266,7 +266,7 @@ func (b *Reader) ReadRune() (r rune, size int, err error) { return 0, 0, b.readErr() } r, size = rune(b.buf[b.r]), 1 - if r >= 0x80 { + if r >= utf8.RuneSelf { r, size = utf8.DecodeRune(b.buf[b.r:b.w]) } b.r += size diff --git a/src/cmd/compile/internal/gc/fmt.go b/src/cmd/compile/internal/gc/fmt.go index 19f109055d..41d696574c 100644 --- a/src/cmd/compile/internal/gc/fmt.go +++ b/src/cmd/compile/internal/gc/fmt.go @@ -337,7 +337,7 @@ func Vconv(v Val, flag FmtFlag) string { case CTRUNE: x := v.U.(*Mpint).Int64() - if ' ' <= x && x < 0x80 && x != '\\' && x != '\'' { + if ' ' <= x && x < utf8.RuneSelf && x != '\\' && x != '\'' { return fmt.Sprintf("'%c'", int(x)) } if 0 <= x && x < 1<<16 { diff --git a/src/encoding/asn1/asn1.go b/src/encoding/asn1/asn1.go index bd2c96d887..2b5ad08551 100644 --- a/src/encoding/asn1/asn1.go +++ b/src/encoding/asn1/asn1.go @@ -393,7 +393,7 @@ func isPrintable(b byte) bool { // byte slice and returns it. func parseIA5String(bytes []byte) (ret string, err error) { for _, b := range bytes { - if b >= 0x80 { + if b >= utf8.RuneSelf { err = SyntaxError{"IA5String contains invalid character"} return } diff --git a/src/go/build/build.go b/src/go/build/build.go index e61d564fa3..04a41a6c2e 100644 --- a/src/go/build/build.go +++ b/src/go/build/build.go @@ -1266,7 +1266,7 @@ func safeCgoName(s string, spaces bool) bool { safe = safe[len(safeSpaces):] } for i := 0; i < len(s); i++ { - if c := s[i]; c < 0x80 && bytes.IndexByte(safe, c) < 0 { + if c := s[i]; c < utf8.RuneSelf && bytes.IndexByte(safe, c) < 0 { return false } } diff --git a/src/go/build/read.go b/src/go/build/read.go index d411c1980e..29b8cdc786 100644 --- a/src/go/build/read.go +++ b/src/go/build/read.go @@ -8,6 +8,7 @@ import ( "bufio" "errors" "io" + "unicode/utf8" ) type importReader struct { @@ -20,7 +21,7 @@ type importReader struct { } func isIdent(c byte) bool { - return 'A' <= c && c <= 'Z' || 'a' <= c && c <= 'z' || '0' <= c && c <= '9' || c == '_' || c >= 0x80 + return 'A' <= c && c <= 'Z' || 'a' <= c && c <= 'z' || '0' <= c && c <= '9' || c == '_' || c >= utf8.RuneSelf } var ( diff --git a/src/go/scanner/scanner.go b/src/go/scanner/scanner.go index 4041d9aa47..ce660c71d5 100644 --- a/src/go/scanner/scanner.go +++ b/src/go/scanner/scanner.go @@ -64,7 +64,7 @@ func (s *Scanner) next() { switch { case r == 0: s.error(s.offset, "illegal character NUL") - case r >= 0x80: + case r >= utf8.RuneSelf: // not ASCII r, w = utf8.DecodeRune(s.src[s.rdOffset:]) if r == utf8.RuneError && w == 1 { @@ -255,11 +255,11 @@ func (s *Scanner) findLineEnd() bool { } func isLetter(ch rune) bool { - return 'a' <= ch && ch <= 'z' || 'A' <= ch && ch <= 'Z' || ch == '_' || ch >= 0x80 && unicode.IsLetter(ch) + return 'a' <= ch && ch <= 'z' || 'A' <= ch && ch <= 'Z' || ch == '_' || ch >= utf8.RuneSelf && unicode.IsLetter(ch) } func isDigit(ch rune) bool { - return '0' <= ch && ch <= '9' || ch >= 0x80 && unicode.IsDigit(ch) + return '0' <= ch && ch <= '9' || ch >= utf8.RuneSelf && unicode.IsDigit(ch) } func (s *Scanner) scanIdentifier() string { diff --git a/src/html/template/css.go b/src/html/template/css.go index 4c27cce85a..9154d8636d 100644 --- a/src/html/template/css.go +++ b/src/html/template/css.go @@ -243,7 +243,7 @@ func cssValueFilter(args ...interface{}) string { return filterFailsafe } default: - if c < 0x80 && isCSSNmchar(rune(c)) { + if c < utf8.RuneSelf && isCSSNmchar(rune(c)) { id = append(id, c) } } diff --git a/src/net/http/cookiejar/punycode.go b/src/net/http/cookiejar/punycode.go index ea7ceb5ef3..a9cc666e8c 100644 --- a/src/net/http/cookiejar/punycode.go +++ b/src/net/http/cookiejar/punycode.go @@ -37,7 +37,7 @@ func encode(prefix, s string) (string, error) { delta, n, bias := int32(0), initialN, initialBias b, remaining := int32(0), int32(0) for _, r := range s { - if r < 0x80 { + if r < utf8.RuneSelf { b++ output = append(output, byte(r)) } else { -- cgit v1.3 From 1faa8869c6c72f055cdaa2b547964830909c96c6 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 6 Apr 2016 12:31:55 -0700 Subject: net/http: set the Request context for incoming server requests Updates #13021 Updates #15224 Change-Id: Ia3cd608bb887fcfd8d81b035fa57bd5eb8edf09b Reviewed-on: https://go-review.googlesource.com/21810 Reviewed-by: Andrew Gerrand Run-TryBot: Brad Fitzpatrick Reviewed-by: Emmanuel Odeke TryBot-Result: Gobot Gobot --- src/net/http/request.go | 8 ++++-- src/net/http/serve_test.go | 67 ++++++++++++++++++++++++++++++++++++++++++++++ src/net/http/server.go | 28 ++++++++++++++----- 3 files changed, 95 insertions(+), 8 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/request.go b/src/net/http/request.go index 5510691912..5bca888845 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -266,9 +266,13 @@ type Request struct { // // The returned context is always non-nil; it defaults to the // background context. +// +// For outgoing client requests, the context controls cancelation. +// +// For incoming server requests, the context is canceled when either +// the client's connection closes, or when the ServeHTTP method +// returns. func (r *Request) Context() context.Context { - // TODO(bradfitz): document above what Context means for server and client - // requests, once implemented. if r.ctx != nil { return r.ctx } diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 638ba5f48f..4cd6ed077f 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -9,6 +9,7 @@ package http_test import ( "bufio" "bytes" + "context" "crypto/tls" "errors" "fmt" @@ -3989,6 +3990,72 @@ func TestServerValidatesHeaders(t *testing.T) { } } +func TestServerRequestContextCancel_ServeHTTPDone(t *testing.T) { + defer afterTest(t) + ctxc := make(chan context.Context, 1) + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + ctx := r.Context() + select { + case <-ctx.Done(): + t.Error("should not be Done in ServeHTTP") + default: + } + ctxc <- ctx + })) + defer ts.Close() + res, err := Get(ts.URL) + if err != nil { + t.Fatal(err) + } + res.Body.Close() + ctx := <-ctxc + select { + case <-ctx.Done(): + default: + t.Error("context should be done after ServeHTTP completes") + } +} + +func TestServerRequestContextCancel_ConnClose(t *testing.T) { + // Currently the context is not canceled when the connection + // is closed because we're not reading from the connection + // until after ServeHTTP for the previous handler is done. + // Until the server code is modified to always be in a read + // (Issue 15224), this test doesn't work yet. + t.Skip("TODO(bradfitz): this test doesn't yet work; golang.org/issue/15224") + defer afterTest(t) + inHandler := make(chan struct{}) + handlerDone := make(chan struct{}) + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + close(inHandler) + select { + case <-r.Context().Done(): + case <-time.After(3 * time.Second): + t.Errorf("timeout waiting for context to be done") + } + close(handlerDone) + })) + defer ts.Close() + c, err := net.Dial("tcp", ts.Listener.Addr().String()) + if err != nil { + t.Fatal(err) + } + defer c.Close() + io.WriteString(c, "GET / HTTP/1.1\r\nHost: foo\r\n\r\n") + select { + case <-inHandler: + case <-time.After(3 * time.Second): + t.Fatalf("timeout waiting to see ServeHTTP get called") + } + c.Close() // this should trigger the context being done + + select { + case <-handlerDone: + case <-time.After(3 * time.Second): + t.Fatalf("timeout waiting to see ServeHTTP exit") + } +} + func BenchmarkClientServer(b *testing.B) { b.ReportAllocs() b.StopTimer() diff --git a/src/net/http/server.go b/src/net/http/server.go index f4e697169d..e37df99deb 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -9,6 +9,7 @@ package http import ( "bufio" "bytes" + "context" "crypto/tls" "errors" "fmt" @@ -312,10 +313,11 @@ type response struct { conn *conn req *Request // request for this response reqBody io.ReadCloser - wroteHeader bool // reply header has been (logically) written - wroteContinue bool // 100 Continue response was written - wants10KeepAlive bool // HTTP/1.0 w/ Connection "keep-alive" - wantsClose bool // HTTP request has Connection "close" + cancelCtx context.CancelFunc // when ServeHTTP exits + wroteHeader bool // reply header has been (logically) written + wroteContinue bool // 100 Continue response was written + wants10KeepAlive bool // HTTP/1.0 w/ Connection "keep-alive" + wantsClose bool // HTTP request has Connection "close" w *bufio.Writer // buffers output in chunks to chunkWriter cw chunkWriter @@ -686,7 +688,7 @@ func appendTime(b []byte, t time.Time) []byte { var errTooLarge = errors.New("http: request too large") // Read next request from connection. -func (c *conn) readRequest() (w *response, err error) { +func (c *conn) readRequest(ctx context.Context) (w *response, err error) { if c.hijacked() { return nil, ErrHijacked } @@ -715,6 +717,10 @@ func (c *conn) readRequest() (w *response, err error) { } return nil, err } + + ctx, cancelCtx := context.WithCancel(ctx) + req.ctx = ctx + c.lastMethod = req.Method c.r.setInfiniteReadLimit() @@ -749,6 +755,7 @@ func (c *conn) readRequest() (w *response, err error) { w = &response{ conn: c, + cancelCtx: cancelCtx, req: req, reqBody: req.Body, handlerHeader: make(Header), @@ -1432,12 +1439,20 @@ func (c *conn) serve() { } } + // HTTP/1.x from here on. + c.r = &connReader{r: c.rwc} c.bufr = newBufioReader(c.r) c.bufw = newBufioWriterSize(checkConnErrorWriter{c}, 4<<10) + // TODO: allow changing base context? can't imagine concrete + // use cases yet. + baseCtx := context.Background() + ctx, cancelCtx := context.WithCancel(baseCtx) + defer cancelCtx() + for { - w, err := c.readRequest() + w, err := c.readRequest(ctx) if c.r.remain != c.server.initialReadLimitSize() { // If we read any bytes off the wire, we're active. c.setState(c.rwc, StateActive) @@ -1485,6 +1500,7 @@ func (c *conn) serve() { // [*] Not strictly true: HTTP pipelining. We could let them all process // in parallel even if their responses need to be serialized. serverHandler{c.server}.ServeHTTP(w, w.req) + w.cancelCtx() if c.hijacked() { return } -- cgit v1.3 From b0eeb8b0aaaf4997c25e3048bfc40e53d556a8eb Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sun, 3 Apr 2016 12:52:12 -0400 Subject: net/http/pprof: accept fractional seconds in trace handler For heavily loaded servers, even 1 second of trace is too large to process with the trace viewer; using a float64 here allows fetching /debug/pprof/trace?seconds=0.1. Change-Id: I286c07abf04f9c1fe594b0e26799bf37f5c734db Reviewed-on: https://go-review.googlesource.com/21455 Reviewed-by: Austin Clements --- src/net/http/pprof/pprof.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/pprof/pprof.go b/src/net/http/pprof/pprof.go index 44afa2d8d8..cb4086b963 100644 --- a/src/net/http/pprof/pprof.go +++ b/src/net/http/pprof/pprof.go @@ -120,8 +120,8 @@ func Profile(w http.ResponseWriter, r *http.Request) { // Tracing lasts for duration specified in seconds GET parameter, or for 1 second if not specified. // The package initialization registers it as /debug/pprof/trace. func Trace(w http.ResponseWriter, r *http.Request) { - sec, _ := strconv.ParseInt(r.FormValue("seconds"), 10, 64) - if sec == 0 { + sec, err := strconv.ParseFloat(r.FormValue("seconds"), 64) + if sec <= 0 || err != nil { sec = 1 } @@ -136,7 +136,7 @@ func Trace(w http.ResponseWriter, r *http.Request) { fmt.Fprintf(w, "Could not enable tracing: %s\n", err) return } - sleep(w, time.Duration(sec)*time.Second) + sleep(w, time.Duration(sec*float64(time.Second))) trace.Stop() } -- cgit v1.3 From 683917a72154e3409e1ab5ef5b26030388312d0b Mon Sep 17 00:00:00 2001 From: Dominik Honnef Date: Fri, 1 Apr 2016 07:34:18 +0200 Subject: all: use bytes.Equal, bytes.Contains and strings.Contains, again The previous cleanup was done with a buggy tool, missing some potential rewrites. Change-Id: I333467036e355f999a6a493e8de87e084f374e26 Reviewed-on: https://go-review.googlesource.com/21378 Reviewed-by: Brad Fitzpatrick --- src/cmd/go/go_test.go | 4 ++-- src/html/template/url.go | 2 +- src/net/http/serve_test.go | 2 +- src/path/filepath/path_test.go | 2 +- src/runtime/gcinfo_test.go | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) (limited to 'src/net/http') diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 411fd1e322..42efa9f312 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -1466,7 +1466,7 @@ func TestGoTestWithPackageListedMultipleTimes(t *testing.T) { defer tg.cleanup() tg.parallel() tg.run("test", "errors", "errors", "errors", "errors", "errors") - if strings.Index(strings.TrimSpace(tg.getStdout()), "\n") != -1 { + if strings.Contains(strings.TrimSpace(tg.getStdout()), "\n") { t.Error("go test errors errors errors errors errors tested the same package multiple times") } } @@ -1495,7 +1495,7 @@ func TestGoListCmdOnlyShowsCommands(t *testing.T) { tg.run("list", "cmd") out := strings.TrimSpace(tg.getStdout()) for _, line := range strings.Split(out, "\n") { - if strings.Index(line, "cmd/") == -1 { + if !strings.Contains(line, "cmd/") { t.Error("go list cmd shows non-commands") break } diff --git a/src/html/template/url.go b/src/html/template/url.go index 2ca76bf389..246bfd32cd 100644 --- a/src/html/template/url.go +++ b/src/html/template/url.go @@ -17,7 +17,7 @@ func urlFilter(args ...interface{}) string { if t == contentTypeURL { return s } - if i := strings.IndexRune(s, ':'); i >= 0 && strings.IndexRune(s[:i], '/') < 0 { + if i := strings.IndexRune(s, ':'); i >= 0 && !strings.ContainsRune(s[:i], '/') { protocol := strings.ToLower(s[:i]) if protocol != "http" && protocol != "https" && protocol != "mailto" { return "#" + filterFailsafe diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index 4cd6ed077f..e0094234de 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -4267,7 +4267,7 @@ func BenchmarkClient(b *testing.B) { if err != nil { b.Fatalf("ReadAll: %v", err) } - if bytes.Compare(body, data) != 0 { + if !bytes.Equal(body, data) { b.Fatalf("Got body: %q", body) } } diff --git a/src/path/filepath/path_test.go b/src/path/filepath/path_test.go index 3622f9178e..1a4a9d2a1a 100644 --- a/src/path/filepath/path_test.go +++ b/src/path/filepath/path_test.go @@ -1015,7 +1015,7 @@ func TestAbs(t *testing.T) { vol := filepath.VolumeName(root) var extra []string for _, path := range absTests { - if strings.Index(path, "$") != -1 { + if strings.Contains(path, "$") { continue } path = vol + path diff --git a/src/runtime/gcinfo_test.go b/src/runtime/gcinfo_test.go index c1c2354bf9..9a61b4f2b2 100644 --- a/src/runtime/gcinfo_test.go +++ b/src/runtime/gcinfo_test.go @@ -59,7 +59,7 @@ func TestGCInfo(t *testing.T) { func verifyGCInfo(t *testing.T, name string, p interface{}, mask0 []byte) { mask := runtime.GCMask(p) - if bytes.Compare(mask, mask0) != 0 { + if !bytes.Equal(mask, mask0) { t.Errorf("bad GC program for %v:\nwant %+v\ngot %+v", name, mask0, mask) return } -- cgit v1.3 From 00681eec6aec03b8b2822c9220fba27c18923c01 Mon Sep 17 00:00:00 2001 From: Dan Peterson Date: Mon, 11 Apr 2016 11:15:00 -0300 Subject: net/http: document Error does not end the request Fixes #15205 Change-Id: Ia650806756758ca8ed2272b1696e59b809b16c61 Reviewed-on: https://go-review.googlesource.com/21836 Reviewed-by: Brad Fitzpatrick --- src/net/http/server.go | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/net/http') diff --git a/src/net/http/server.go b/src/net/http/server.go index e37df99deb..7a6950aee4 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -1652,6 +1652,8 @@ func (f HandlerFunc) ServeHTTP(w ResponseWriter, r *Request) { // Helper handlers // Error replies to the request with the specified error message and HTTP code. +// It does not otherwise end the request; the caller should ensure no further +// writes are done to w. // 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") -- cgit v1.3 From cabb1402568ae7d05d9d5adf56953a4792085a81 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Mon, 11 Apr 2016 05:12:43 +0000 Subject: net/http: add ServerContextKey to let a handler access its Server Fixes #12438 Updates #15229 (to decide context key variable naming convention) Change-Id: I3ba423e91b689e232143247d044495a12c97a7d2 Reviewed-on: https://go-review.googlesource.com/21829 Reviewed-by: Andrew Gerrand Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/http.go | 8 ++++++++ src/net/http/serve_test.go | 17 +++++++++++++++++ src/net/http/server.go | 21 +++++++++++++++------ 3 files changed, 40 insertions(+), 6 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/http.go b/src/net/http/http.go index a40b23dfdb..7484348f52 100644 --- a/src/net/http/http.go +++ b/src/net/http/http.go @@ -10,3 +10,11 @@ const maxInt64 = 1<<63 - 1 // TODO(bradfitz): move common stuff here. The other files have accumulated // generic http stuff in random places. + +// contextKey is a value for use with context.WithValue. It's used as +// a pointer so it fits in an interface{} without allocation. +type contextKey struct { + name string +} + +func (k *contextKey) String() string { return "net/http context value " + k.name } diff --git a/src/net/http/serve_test.go b/src/net/http/serve_test.go index e0094234de..5f206b1873 100644 --- a/src/net/http/serve_test.go +++ b/src/net/http/serve_test.go @@ -4056,6 +4056,23 @@ func TestServerRequestContextCancel_ConnClose(t *testing.T) { } } +func TestServerContext_ServerContextKey(t *testing.T) { + defer afterTest(t) + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + ctx := r.Context() + got := ctx.Value(ServerContextKey) + if _, ok := got.(*Server); !ok { + t.Errorf("context value = %T; want *http.Server") + } + })) + defer ts.Close() + res, err := Get(ts.URL) + if err != nil { + t.Fatal(err) + } + res.Body.Close() +} + func BenchmarkClientServer(b *testing.B) { b.ReportAllocs() b.StopTimer() diff --git a/src/net/http/server.go b/src/net/http/server.go index 7a6950aee4..deb170c334 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -147,6 +147,14 @@ type CloseNotifier interface { CloseNotify() <-chan bool } +var ( + // ServerContextKey is a context key. It can be used in HTTP + // handlers with context.WithValue to access the server that + // started the handler. The associated value will be of + // type *Server. + ServerContextKey = &contextKey{"http-server"} +) + // A conn represents the server side of an HTTP connection. type conn struct { // server is the server on which the connection arrived. @@ -1402,7 +1410,7 @@ type badRequestError string func (e badRequestError) Error() string { return "Bad Request: " + string(e) } // Serve a new connection. -func (c *conn) serve() { +func (c *conn) serve(ctx context.Context) { c.remoteAddr = c.rwc.RemoteAddr().String() defer func() { if err := recover(); err != nil { @@ -1445,10 +1453,7 @@ func (c *conn) serve() { c.bufr = newBufioReader(c.r) c.bufw = newBufioWriterSize(checkConnErrorWriter{c}, 4<<10) - // TODO: allow changing base context? can't imagine concrete - // use cases yet. - baseCtx := context.Background() - ctx, cancelCtx := context.WithCancel(baseCtx) + ctx, cancelCtx := context.WithCancel(ctx) defer cancelCtx() for { @@ -2151,6 +2156,10 @@ func (srv *Server) Serve(l net.Listener) error { if err := srv.setupHTTP2(); err != nil { return err } + // TODO: allow changing base context? can't imagine concrete + // use cases yet. + baseCtx := context.Background() + ctx := context.WithValue(baseCtx, ServerContextKey, srv) for { rw, e := l.Accept() if e != nil { @@ -2172,7 +2181,7 @@ func (srv *Server) Serve(l net.Listener) error { tempDelay = 0 c := srv.newConn(rw) c.setState(c.rwc, StateNew) // before Serve can return - go c.serve() + go c.serve(ctx) } } -- cgit v1.3 From b09c274bfabb3edef60b4df3375906852aab7da1 Mon Sep 17 00:00:00 2001 From: Michael Munday Date: Tue, 12 Apr 2016 12:46:54 -0400 Subject: net/http: fix TestLinuxSendfile on s390x s390x doesn't have sendfile64 so apply the same fix as MIPS (eebf7d27) and just use sendfile. Change-Id: If8fe2e974ed44a9883282430157c3545d5bd04bd Reviewed-on: https://go-review.googlesource.com/21892 Reviewed-by: Brad Fitzpatrick --- src/net/http/fs_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/fs_test.go b/src/net/http/fs_test.go index 9253ebe43a..c811891e87 100644 --- a/src/net/http/fs_test.go +++ b/src/net/http/fs_test.go @@ -978,9 +978,9 @@ func TestLinuxSendfile(t *testing.T) { syscalls := "sendfile,sendfile64" switch runtime.GOARCH { - case "mips64", "mips64le": - // mips64 strace doesn't support sendfile64 and will error out - // if we specify that with `-e trace='. + case "mips64", "mips64le", "s390x": + // strace on the above platforms doesn't support sendfile64 + // and will error out if we specify that with `-e trace='. syscalls = "sendfile" } -- cgit v1.3 From 022548cfe82915e5bf14ce7cb28f3ec651550662 Mon Sep 17 00:00:00 2001 From: Dan Peterson Date: Tue, 12 Apr 2016 16:58:56 -0300 Subject: all: standardize RFC mention format Standardize on space between "RFC" and number. Additionally change the couple "a RFC" instances to "an RFC." Fixes #15258 Change-Id: I2b17ecd06be07dfbb4207c690f52a59ea9b04808 Reviewed-on: https://go-review.googlesource.com/21902 Reviewed-by: Brad Fitzpatrick --- src/crypto/tls/common.go | 2 +- src/crypto/tls/prf.go | 2 +- src/crypto/x509/pkcs8.go | 2 +- src/crypto/x509/sec1.go | 4 ++-- src/mime/encodedword.go | 2 +- src/net/dnsname_test.go | 2 +- src/net/http/request.go | 2 +- src/net/http/response.go | 2 +- src/net/http/server.go | 2 +- src/net/http/transfer.go | 4 ++-- src/net/mail/message.go | 4 ++-- src/net/mail/message_test.go | 2 +- 12 files changed, 15 insertions(+), 15 deletions(-) (limited to 'src/net/http') diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go index 572266bc8f..b3399b063c 100644 --- a/src/crypto/tls/common.go +++ b/src/crypto/tls/common.go @@ -114,7 +114,7 @@ const ( certTypeRSAFixedDH = 3 // A certificate containing a static DH key certTypeDSSFixedDH = 4 // A certificate containing a static DH key - // See RFC4492 sections 3 and 5.5. + // See RFC 4492 sections 3 and 5.5. certTypeECDSASign = 64 // A certificate containing an ECDSA-capable public key, signed with ECDSA. certTypeRSAFixedECDH = 65 // A certificate containing an ECDH-capable public key, signed with RSA. certTypeECDSAFixedECDH = 66 // A certificate containing an ECDH-capable public key, signed with ECDSA. diff --git a/src/crypto/tls/prf.go b/src/crypto/tls/prf.go index 747b817ba3..5833fc1963 100644 --- a/src/crypto/tls/prf.go +++ b/src/crypto/tls/prf.go @@ -85,7 +85,7 @@ func prf30(result, secret, label, seed []byte) { done := 0 i := 0 - // RFC5246 section 6.3 says that the largest PRF output needed is 128 + // RFC 5246 section 6.3 says that the largest PRF output needed is 128 // bytes. Since no more ciphersuites will be added to SSLv3, this will // remain true. Each iteration gives us 16 bytes so 10 iterations will // be sufficient. diff --git a/src/crypto/x509/pkcs8.go b/src/crypto/x509/pkcs8.go index 6e56752c0e..b304a3f63c 100644 --- a/src/crypto/x509/pkcs8.go +++ b/src/crypto/x509/pkcs8.go @@ -13,7 +13,7 @@ import ( // pkcs8 reflects an ASN.1, PKCS#8 PrivateKey. See // ftp://ftp.rsasecurity.com/pub/pkcs/pkcs-8/pkcs-8v1_2.asn -// and RFC5208. +// and RFC 5208. type pkcs8 struct { Version int Algo pkix.AlgorithmIdentifier diff --git a/src/crypto/x509/sec1.go b/src/crypto/x509/sec1.go index 5f1b3ecc7c..33f376c072 100644 --- a/src/crypto/x509/sec1.go +++ b/src/crypto/x509/sec1.go @@ -17,9 +17,9 @@ const ecPrivKeyVersion = 1 // ecPrivateKey reflects an ASN.1 Elliptic Curve Private Key Structure. // References: -// RFC5915 +// RFC 5915 // SEC1 - http://www.secg.org/sec1-v2.pdf -// Per RFC5915 the NamedCurveOID is marked as ASN.1 OPTIONAL, however in +// Per RFC 5915 the NamedCurveOID is marked as ASN.1 OPTIONAL, however in // most cases it is not. type ecPrivateKey struct { Version int diff --git a/src/mime/encodedword.go b/src/mime/encodedword.go index e6cbebe946..c3ca4bacd1 100644 --- a/src/mime/encodedword.go +++ b/src/mime/encodedword.go @@ -16,7 +16,7 @@ import ( "unicode/utf8" ) -// A WordEncoder is a RFC 2047 encoded-word encoder. +// A WordEncoder is an RFC 2047 encoded-word encoder. type WordEncoder byte const ( diff --git a/src/net/dnsname_test.go b/src/net/dnsname_test.go index be07dc6a16..bc777b855e 100644 --- a/src/net/dnsname_test.go +++ b/src/net/dnsname_test.go @@ -15,7 +15,7 @@ type dnsNameTest struct { } var dnsNameTests = []dnsNameTest{ - // RFC2181, section 11. + // RFC 2181, section 11. {"_xmpp-server._tcp.google.com", true}, {"foo.com", true}, {"1foo.com", true}, diff --git a/src/net/http/request.go b/src/net/http/request.go index 5bca888845..bac2de1a2e 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -817,7 +817,7 @@ func readRequest(b *bufio.Reader, deleteHostHeader bool) (req *Request, err erro } req.Header = Header(mimeHeader) - // RFC2616: Must treat + // RFC 2616: Must treat // GET /index.html HTTP/1.1 // Host: www.google.com // and diff --git a/src/net/http/response.go b/src/net/http/response.go index b49b77d8b9..91d4ffb7ec 100644 --- a/src/net/http/response.go +++ b/src/net/http/response.go @@ -185,7 +185,7 @@ func ReadResponse(r *bufio.Reader, req *Request) (*Response, error) { return resp, nil } -// RFC2616: Should treat +// RFC 2616: Should treat // Pragma: no-cache // like // Cache-Control: no-cache diff --git a/src/net/http/server.go b/src/net/http/server.go index deb170c334..64529f1e96 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -1747,7 +1747,7 @@ func Redirect(w ResponseWriter, r *Request, urlStr string, code int) { w.Header().Set("Location", urlStr) w.WriteHeader(code) - // RFC2616 recommends that a short note "SHOULD" be included in the + // RFC 2616 recommends that a short note "SHOULD" be included in the // response because older user agents may not understand 301/307. // Shouldn't send the response for POST or HEAD; that leaves GET. if r.Method == "GET" { diff --git a/src/net/http/transfer.go b/src/net/http/transfer.go index 4c130f0cc4..501e4be08c 100644 --- a/src/net/http/transfer.go +++ b/src/net/http/transfer.go @@ -276,7 +276,7 @@ func (t *transferReader) protoAtLeast(m, n int) bool { } // bodyAllowedForStatus reports whether a given response status code -// permits a body. See RFC2616, section 4.4. +// permits a body. See RFC 2616, section 4.4. func bodyAllowedForStatus(status int) bool { switch { case status >= 100 && status <= 199: @@ -368,7 +368,7 @@ func readTransfer(msg interface{}, r *bufio.Reader) (err error) { // If there is no Content-Length or chunked Transfer-Encoding on a *Response // and the status is not 1xx, 204 or 304, then the body is unbounded. - // See RFC2616, section 4.4. + // See RFC 2616, section 4.4. switch msg.(type) { case *Response: if realLength == -1 && diff --git a/src/net/mail/message.go b/src/net/mail/message.go index 12342b368f..9e3a103a4f 100644 --- a/src/net/mail/message.go +++ b/src/net/mail/message.go @@ -570,7 +570,7 @@ func isQtext(c byte) bool { return '!' <= c && c <= '~' } -// quoteString renders a string as a RFC5322 quoted-string. +// quoteString renders a string as an RFC 5322 quoted-string. func quoteString(s string) string { var buf bytes.Buffer buf.WriteByte('"') @@ -594,7 +594,7 @@ func isVchar(c byte) bool { } // isWSP reports whether c is a WSP (white space). -// WSP is a space or horizontal tab (RFC5234 Appendix B). +// WSP is a space or horizontal tab (RFC 5234 Appendix B). func isWSP(c byte) bool { return c == ' ' || c == '\t' } diff --git a/src/net/mail/message_test.go b/src/net/mail/message_test.go index cf86ace68f..2669325c13 100644 --- a/src/net/mail/message_test.go +++ b/src/net/mail/message_test.go @@ -92,7 +92,7 @@ func TestDateParsing(t *testing.T) { "Fri, 21 Nov 1997 09:55:06 -0600", time.Date(1997, 11, 21, 9, 55, 6, 0, time.FixedZone("", -6*60*60)), }, - // RFC5322, Appendix A.6.2 + // RFC 5322, Appendix A.6.2 // Obsolete date. { "21 Nov 97 09:55:06 GMT", -- cgit v1.3 From a0ab6cd6852cec430e280217a9516d6be3c1ef5f Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 12 Apr 2016 00:22:39 +0000 Subject: net/http: add test that panic in a handler signals an error to the client Change-Id: Iba40edc9ddad62534b06c5af20bbc3dd3dc14d0a Reviewed-on: https://go-review.googlesource.com/21881 Reviewed-by: Andrew Gerrand Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/clientserver_test.go | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) (limited to 'src/net/http') diff --git a/src/net/http/clientserver_test.go b/src/net/http/clientserver_test.go index c2bab378e3..f721382365 100644 --- a/src/net/http/clientserver_test.go +++ b/src/net/http/clientserver_test.go @@ -1123,6 +1123,34 @@ func testBogusStatusWorks(t *testing.T, h2 bool) { } } +func TestInterruptWithPanic_h1(t *testing.T) { testInterruptWithPanic(t, h1Mode) } +func TestInterruptWithPanic_h2(t *testing.T) { testInterruptWithPanic(t, h2Mode) } +func testInterruptWithPanic(t *testing.T, h2 bool) { + log.SetOutput(ioutil.Discard) // is noisy otherwise + defer log.SetOutput(os.Stderr) + + const msg = "hello" + defer afterTest(t) + cst := newClientServerTest(t, h2, HandlerFunc(func(w ResponseWriter, r *Request) { + io.WriteString(w, msg) + w.(Flusher).Flush() + panic("no more") + })) + defer cst.close() + res, err := cst.c.Get(cst.ts.URL) + if err != nil { + t.Fatal(err) + } + defer res.Body.Close() + slurp, err := ioutil.ReadAll(res.Body) + if string(slurp) != msg { + t.Errorf("client read %q; want %q", slurp, msg) + } + if err == nil { + t.Errorf("client read all successfully; want some error") + } +} + type noteCloseConn struct { net.Conn closeFunc func() -- cgit v1.3 From 381e5eee39edfb3a43e294023957aff05e9ff349 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Wed, 13 Apr 2016 04:35:37 +0000 Subject: all: use new io.SeekFoo constants instead of os.SEEK_FOO Automated change. Fixes #15269 Change-Id: I8deb2ac0101d3f7c390467ceb0a1561b72edbb2f Reviewed-on: https://go-review.googlesource.com/21962 Run-TryBot: Brad Fitzpatrick Reviewed-by: Andrew Gerrand TryBot-Result: Gobot Gobot --- src/archive/tar/reader.go | 5 ++--- src/archive/zip/reader.go | 2 +- src/bytes/reader_test.go | 21 ++++++++++----------- src/debug/elf/file.go | 8 ++++---- src/debug/pe/file.go | 8 ++++---- src/net/http/fs.go | 10 +++++----- src/net/sendfile_dragonfly.go | 2 +- src/net/sendfile_freebsd.go | 2 +- src/net/sendfile_solaris.go | 2 +- src/os/exec/exec_test.go | 2 +- src/strings/reader_test.go | 21 ++++++++++----------- 11 files changed, 40 insertions(+), 43 deletions(-) (limited to 'src/net/http') diff --git a/src/archive/tar/reader.go b/src/archive/tar/reader.go index c8cb69a178..741fe0152b 100644 --- a/src/archive/tar/reader.go +++ b/src/archive/tar/reader.go @@ -13,7 +13,6 @@ import ( "io" "io/ioutil" "math" - "os" "strconv" "strings" "time" @@ -523,10 +522,10 @@ func (tr *Reader) skipUnread() error { // io.Seeker, but calling Seek always returns an error and performs // no action. Thus, we try an innocent seek to the current position // to see if Seek is really supported. - pos1, err := sr.Seek(0, os.SEEK_CUR) + pos1, err := sr.Seek(0, io.SeekCurrent) if err == nil { // Seek seems supported, so perform the real Seek. - pos2, err := sr.Seek(dataSkip-1, os.SEEK_CUR) + pos2, err := sr.Seek(dataSkip-1, io.SeekCurrent) if err != nil { tr.err = err return tr.err diff --git a/src/archive/zip/reader.go b/src/archive/zip/reader.go index d741d105dc..f6c3ead3be 100644 --- a/src/archive/zip/reader.go +++ b/src/archive/zip/reader.go @@ -87,7 +87,7 @@ func (z *Reader) init(r io.ReaderAt, size int64) error { z.File = make([]*File, 0, end.directoryRecords) z.Comment = end.comment rs := io.NewSectionReader(r, 0, size) - if _, err = rs.Seek(int64(end.directoryOffset), os.SEEK_SET); err != nil { + if _, err = rs.Seek(int64(end.directoryOffset), io.SeekStart); err != nil { return err } buf := bufio.NewReader(rs) diff --git a/src/bytes/reader_test.go b/src/bytes/reader_test.go index add985d57e..9341cd5b45 100644 --- a/src/bytes/reader_test.go +++ b/src/bytes/reader_test.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "io/ioutil" - "os" "sync" "testing" ) @@ -24,15 +23,15 @@ func TestReader(t *testing.T) { wantpos int64 seekerr string }{ - {seek: os.SEEK_SET, off: 0, n: 20, want: "0123456789"}, - {seek: os.SEEK_SET, off: 1, n: 1, want: "1"}, - {seek: os.SEEK_CUR, off: 1, wantpos: 3, n: 2, want: "34"}, - {seek: os.SEEK_SET, off: -1, seekerr: "bytes.Reader.Seek: negative position"}, - {seek: os.SEEK_SET, off: 1 << 33, wantpos: 1 << 33}, - {seek: os.SEEK_CUR, off: 1, wantpos: 1<<33 + 1}, - {seek: os.SEEK_SET, n: 5, want: "01234"}, - {seek: os.SEEK_CUR, n: 5, want: "56789"}, - {seek: os.SEEK_END, off: -1, n: 1, wantpos: 9, want: "9"}, + {seek: io.SeekStart, off: 0, n: 20, want: "0123456789"}, + {seek: io.SeekStart, off: 1, n: 1, want: "1"}, + {seek: io.SeekCurrent, off: 1, wantpos: 3, n: 2, want: "34"}, + {seek: io.SeekStart, off: -1, seekerr: "bytes.Reader.Seek: negative position"}, + {seek: io.SeekStart, off: 1 << 33, wantpos: 1 << 33}, + {seek: io.SeekCurrent, off: 1, wantpos: 1<<33 + 1}, + {seek: io.SeekStart, n: 5, want: "01234"}, + {seek: io.SeekCurrent, n: 5, want: "56789"}, + {seek: io.SeekEnd, off: -1, n: 1, wantpos: 9, want: "9"}, } for i, tt := range tests { @@ -63,7 +62,7 @@ func TestReader(t *testing.T) { func TestReadAfterBigSeek(t *testing.T) { r := NewReader([]byte("0123456789")) - if _, err := r.Seek(1<<31+5, os.SEEK_SET); err != nil { + if _, err := r.Seek(1<<31+5, io.SeekStart); err != nil { t.Fatal(err) } if n, err := r.Read(make([]byte, 10)); n != 0 || err != io.EOF { diff --git a/src/debug/elf/file.go b/src/debug/elf/file.go index c28a964b73..8fbf23fe5a 100644 --- a/src/debug/elf/file.go +++ b/src/debug/elf/file.go @@ -269,7 +269,7 @@ func NewFile(r io.ReaderAt) (*File, error) { switch f.Class { case ELFCLASS32: hdr := new(Header32) - sr.Seek(0, os.SEEK_SET) + sr.Seek(0, io.SeekStart) if err := binary.Read(sr, f.ByteOrder, hdr); err != nil { return nil, err } @@ -288,7 +288,7 @@ func NewFile(r io.ReaderAt) (*File, error) { shstrndx = int(hdr.Shstrndx) case ELFCLASS64: hdr := new(Header64) - sr.Seek(0, os.SEEK_SET) + sr.Seek(0, io.SeekStart) if err := binary.Read(sr, f.ByteOrder, hdr); err != nil { return nil, err } @@ -315,7 +315,7 @@ func NewFile(r io.ReaderAt) (*File, error) { f.Progs = make([]*Prog, phnum) for i := 0; i < phnum; i++ { off := phoff + int64(i)*int64(phentsize) - sr.Seek(off, os.SEEK_SET) + sr.Seek(off, io.SeekStart) p := new(Prog) switch f.Class { case ELFCLASS32: @@ -359,7 +359,7 @@ func NewFile(r io.ReaderAt) (*File, error) { names := make([]uint32, shnum) for i := 0; i < shnum; i++ { off := shoff + int64(i)*int64(shentsize) - sr.Seek(off, os.SEEK_SET) + sr.Seek(off, io.SeekStart) s := new(Section) switch f.Class { case ELFCLASS32: diff --git a/src/debug/pe/file.go b/src/debug/pe/file.go index 1acc368e1b..c68ff1bdce 100644 --- a/src/debug/pe/file.go +++ b/src/debug/pe/file.go @@ -150,7 +150,7 @@ func NewFile(r io.ReaderAt) (*File, error) { } else { base = int64(0) } - sr.Seek(base, os.SEEK_SET) + sr.Seek(base, io.SeekStart) if err := binary.Read(sr, binary.LittleEndian, &f.FileHeader); err != nil { return nil, err } @@ -161,7 +161,7 @@ func NewFile(r io.ReaderAt) (*File, error) { var ss []byte if f.FileHeader.NumberOfSymbols > 0 { // Get COFF string table, which is located at the end of the COFF symbol table. - sr.Seek(int64(f.FileHeader.PointerToSymbolTable+COFFSymbolSize*f.FileHeader.NumberOfSymbols), os.SEEK_SET) + sr.Seek(int64(f.FileHeader.PointerToSymbolTable+COFFSymbolSize*f.FileHeader.NumberOfSymbols), io.SeekStart) var l uint32 if err := binary.Read(sr, binary.LittleEndian, &l); err != nil { return nil, err @@ -172,7 +172,7 @@ func NewFile(r io.ReaderAt) (*File, error) { } // Process COFF symbol table. - sr.Seek(int64(f.FileHeader.PointerToSymbolTable), os.SEEK_SET) + sr.Seek(int64(f.FileHeader.PointerToSymbolTable), io.SeekStart) aux := uint8(0) for i := 0; i < int(f.FileHeader.NumberOfSymbols); i++ { cs := new(COFFSymbol) @@ -203,7 +203,7 @@ func NewFile(r io.ReaderAt) (*File, error) { } // Read optional header. - sr.Seek(base, os.SEEK_SET) + sr.Seek(base, io.SeekStart) if err := binary.Read(sr, binary.LittleEndian, &f.FileHeader); err != nil { return nil, err } diff --git a/src/net/http/fs.go b/src/net/http/fs.go index 5546d37516..c7a58a61df 100644 --- a/src/net/http/fs.go +++ b/src/net/http/fs.go @@ -121,11 +121,11 @@ 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) { sizeFunc := func() (int64, error) { - size, err := content.Seek(0, os.SEEK_END) + size, err := content.Seek(0, io.SeekEnd) if err != nil { return 0, errSeeker } - _, err = content.Seek(0, os.SEEK_SET) + _, err = content.Seek(0, io.SeekStart) if err != nil { return 0, errSeeker } @@ -166,7 +166,7 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time, var buf [sniffLen]byte n, _ := io.ReadFull(content, buf[:]) ctype = DetectContentType(buf[:n]) - _, err := content.Seek(0, os.SEEK_SET) // rewind to output whole file + _, err := content.Seek(0, io.SeekStart) // rewind to output whole file if err != nil { Error(w, "seeker can't seek", StatusInternalServerError) return @@ -213,7 +213,7 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time, // A response to a request for a single range MUST NOT // be sent using the multipart/byteranges media type." ra := ranges[0] - if _, err := content.Seek(ra.start, os.SEEK_SET); err != nil { + if _, err := content.Seek(ra.start, io.SeekStart); err != nil { Error(w, err.Error(), StatusRequestedRangeNotSatisfiable) return } @@ -236,7 +236,7 @@ func serveContent(w ResponseWriter, r *Request, name string, modtime time.Time, pw.CloseWithError(err) return } - if _, err := content.Seek(ra.start, os.SEEK_SET); err != nil { + if _, err := content.Seek(ra.start, io.SeekStart); err != nil { pw.CloseWithError(err) return } diff --git a/src/net/sendfile_dragonfly.go b/src/net/sendfile_dragonfly.go index 17021c3801..d4b825c370 100644 --- a/src/net/sendfile_dragonfly.go +++ b/src/net/sendfile_dragonfly.go @@ -53,7 +53,7 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) { // use the current position of the file -- if you pass it offset 0, it starts // from offset 0. There's no way to tell it "start from current position", so // we have to manage that explicitly. - pos, err := f.Seek(0, os.SEEK_CUR) + pos, err := f.Seek(0, io.SeekCurrent) if err != nil { return 0, err, false } diff --git a/src/net/sendfile_freebsd.go b/src/net/sendfile_freebsd.go index f7a8529560..18cbb27b53 100644 --- a/src/net/sendfile_freebsd.go +++ b/src/net/sendfile_freebsd.go @@ -53,7 +53,7 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) { // use the current position of the file -- if you pass it offset 0, it starts // from offset 0. There's no way to tell it "start from current position", so // we have to manage that explicitly. - pos, err := f.Seek(0, os.SEEK_CUR) + pos, err := f.Seek(0, io.SeekCurrent) if err != nil { return 0, err, false } diff --git a/src/net/sendfile_solaris.go b/src/net/sendfile_solaris.go index 20d2cddeea..add70c3147 100644 --- a/src/net/sendfile_solaris.go +++ b/src/net/sendfile_solaris.go @@ -57,7 +57,7 @@ func sendFile(c *netFD, r io.Reader) (written int64, err error, handled bool) { // use the current position of the file -- if you pass it offset 0, it starts // from offset 0. There's no way to tell it "start from current position", so // we have to manage that explicitly. - pos, err := f.Seek(0, os.SEEK_CUR) + pos, err := f.Seek(0, io.SeekCurrent) if err != nil { return 0, err, false } diff --git a/src/os/exec/exec_test.go b/src/os/exec/exec_test.go index 1f2fd12add..ed2721bb5e 100644 --- a/src/os/exec/exec_test.go +++ b/src/os/exec/exec_test.go @@ -479,7 +479,7 @@ func TestExtraFiles(t *testing.T) { if err != nil { t.Fatalf("Write: %v", err) } - _, err = tf.Seek(0, os.SEEK_SET) + _, err = tf.Seek(0, io.SeekStart) if err != nil { t.Fatalf("Seek: %v", err) } diff --git a/src/strings/reader_test.go b/src/strings/reader_test.go index 7bca2e89a1..6e9d904b9d 100644 --- a/src/strings/reader_test.go +++ b/src/strings/reader_test.go @@ -9,7 +9,6 @@ import ( "fmt" "io" "io/ioutil" - "os" "strings" "sync" "testing" @@ -25,15 +24,15 @@ func TestReader(t *testing.T) { wantpos int64 seekerr string }{ - {seek: os.SEEK_SET, off: 0, n: 20, want: "0123456789"}, - {seek: os.SEEK_SET, off: 1, n: 1, want: "1"}, - {seek: os.SEEK_CUR, off: 1, wantpos: 3, n: 2, want: "34"}, - {seek: os.SEEK_SET, off: -1, seekerr: "strings.Reader.Seek: negative position"}, - {seek: os.SEEK_SET, off: 1 << 33, wantpos: 1 << 33}, - {seek: os.SEEK_CUR, off: 1, wantpos: 1<<33 + 1}, - {seek: os.SEEK_SET, n: 5, want: "01234"}, - {seek: os.SEEK_CUR, n: 5, want: "56789"}, - {seek: os.SEEK_END, off: -1, n: 1, wantpos: 9, want: "9"}, + {seek: io.SeekStart, off: 0, n: 20, want: "0123456789"}, + {seek: io.SeekStart, off: 1, n: 1, want: "1"}, + {seek: io.SeekCurrent, off: 1, wantpos: 3, n: 2, want: "34"}, + {seek: io.SeekStart, off: -1, seekerr: "strings.Reader.Seek: negative position"}, + {seek: io.SeekStart, off: 1 << 33, wantpos: 1 << 33}, + {seek: io.SeekCurrent, off: 1, wantpos: 1<<33 + 1}, + {seek: io.SeekStart, n: 5, want: "01234"}, + {seek: io.SeekCurrent, n: 5, want: "56789"}, + {seek: io.SeekEnd, off: -1, n: 1, wantpos: 9, want: "9"}, } for i, tt := range tests { @@ -64,7 +63,7 @@ func TestReader(t *testing.T) { func TestReadAfterBigSeek(t *testing.T) { r := strings.NewReader("0123456789") - if _, err := r.Seek(1<<31+5, os.SEEK_SET); err != nil { + if _, err := r.Seek(1<<31+5, io.SeekStart); err != nil { t.Fatal(err) } if n, err := r.Read(make([]byte, 10)); n != 0 || err != io.EOF { -- cgit v1.3 From 585590549a3c6e26e7963081e11478a1913744a6 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 15 Apr 2016 22:59:36 +0000 Subject: net/http: add Transport.Dialer, plumb RoundTrip contexts to net package This simply connects the contexts, pushing them down the call stack. Future CLs will utilize them. For #12580 (http.Transport tracing/analytics) Updates #13021 Change-Id: I5b2074d6eb1e87d79a767fc0609c84e7928d1a16 Reviewed-on: https://go-review.googlesource.com/22124 Reviewed-by: Ian Lance Taylor Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/transport.go | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 7692abff47..0568822737 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -12,6 +12,7 @@ package http import ( "bufio" "compress/gzip" + "context" "crypto/tls" "errors" "fmt" @@ -32,10 +33,10 @@ import ( // $no_proxy) environment variables. var DefaultTransport RoundTripper = &Transport{ Proxy: ProxyFromEnvironment, - Dial: (&net.Dialer{ + Dialer: &net.Dialer{ Timeout: 30 * time.Second, KeepAlive: 30 * time.Second, - }).Dial, + }, TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, } @@ -80,10 +81,17 @@ type Transport struct { Proxy func(*Request) (*url.URL, error) // Dial specifies the dial function for creating unencrypted - // TCP connections. - // If Dial is nil, net.Dial is used. + // TCP connections. If Dial and Dialer are both nil, net.Dial + // is used. + // + // Deprecated: Use Dialer instead. If both are specified, Dialer + // takes precedence. Dial func(network, addr string) (net.Conn, error) + // Dialer optionally specifies a dialer configuration to use + // for new connections. + Dialer *net.Dialer + // DialTLS specifies an optional dial function for creating // TLS connections for non-proxied HTTPS requests. // @@ -689,7 +697,10 @@ func (t *Transport) replaceReqCanceler(r *Request, fn func()) bool { return true } -func (t *Transport) dial(network, addr string) (net.Conn, error) { +func (t *Transport) dial(ctx context.Context, network, addr string) (net.Conn, error) { + if t.Dialer != nil { + return t.Dialer.DialContext(ctx, network, addr) + } if t.Dial != nil { c, err := t.Dial(network, addr) if c == nil && err == nil { @@ -705,6 +716,7 @@ func (t *Transport) dial(network, addr string) (net.Conn, error) { // and/or setting up TLS. If this doesn't return an error, the persistConn // is ready to write requests to. func (t *Transport) getConn(req *Request, cm connectMethod) (*persistConn, error) { + ctx := req.Context() if pc := t.getIdleConn(cm); pc != nil { // set request canceler to some non-nil function so we // can detect whether it was cleared between now and when @@ -738,7 +750,7 @@ func (t *Transport) getConn(req *Request, cm connectMethod) (*persistConn, error t.setReqCanceler(req, func() { close(cancelc) }) go func() { - pc, err := t.dialConn(cm) + pc, err := t.dialConn(ctx, cm) dialc <- dialRes{pc, err} }() @@ -767,7 +779,7 @@ func (t *Transport) getConn(req *Request, cm connectMethod) (*persistConn, error } } -func (t *Transport) dialConn(cm connectMethod) (*persistConn, error) { +func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (*persistConn, error) { pconn := &persistConn{ t: t, cacheKey: cm.key(), @@ -797,7 +809,7 @@ func (t *Transport) dialConn(cm connectMethod) (*persistConn, error) { pconn.tlsState = &cs } } else { - conn, err := t.dial("tcp", cm.addr()) + conn, err := t.dial(ctx, "tcp", cm.addr()) if err != nil { if cm.proxyURL != nil { err = fmt.Errorf("http: error connecting to proxy %s: %v", cm.proxyURL, err) -- cgit v1.3 From 26ecb42fb4c5ee1d8b64f12e5bb8df6549523d23 Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Sat, 16 Apr 2016 02:04:00 -0700 Subject: net/http: normalize empty port in URL.Host's ":port" - Ensures that the empty port and preceeding ":" in a URL.Host are stripped. Normalize the empty port in a URL.Host's ":port" as mandated by RFC 3986 Section 6.2.3 which states that: `Likewise an explicit ":port", for which the port is empty or the default for the scheme, is equivalent to one where the port and its ":" delimiter are elided and thus should be removed by scheme-based normalization.` - Moves function `hasPort` from client.go (where it was defined but not used directly), to http.go the common area. Fixes #14836 Change-Id: I2067410377be9c71106b1717abddc2f8b1da1c03 Reviewed-on: https://go-review.googlesource.com/22140 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- src/net/http/client.go | 4 ---- src/net/http/http.go | 17 +++++++++++++++++ src/net/http/request.go | 2 ++ src/net/http/request_test.go | 2 ++ 4 files changed, 21 insertions(+), 4 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/client.go b/src/net/http/client.go index ee0fd2cb62..f8ab675a3d 100644 --- a/src/net/http/client.go +++ b/src/net/http/client.go @@ -110,10 +110,6 @@ type RoundTripper interface { RoundTrip(*Request) (*Response, error) } -// Given a string of the form "host", "host:port", or "[ipv6::address]:port", -// return true if the string includes a port. -func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") } - // refererForURL returns a referer without any authentication info or // an empty string if lastReq scheme is https and newReq scheme is http. func refererForURL(lastReq, newReq *url.URL) string { diff --git a/src/net/http/http.go b/src/net/http/http.go index 7484348f52..a121628632 100644 --- a/src/net/http/http.go +++ b/src/net/http/http.go @@ -4,6 +4,10 @@ package http +import ( + "strings" +) + // maxInt64 is the effective "infinite" value for the Server and // Transport's byte-limiting readers. const maxInt64 = 1<<63 - 1 @@ -18,3 +22,16 @@ type contextKey struct { } func (k *contextKey) String() string { return "net/http context value " + k.name } + +// Given a string of the form "host", "host:port", or "[ipv6::address]:port", +// return true if the string includes a port. +func hasPort(s string) bool { return strings.LastIndex(s, ":") > strings.LastIndex(s, "]") } + +// removeEmptyPort strips the empty port in ":port" to "" +// as mandated by RFC 3986 Section 6.2.3. +func removeEmptyPort(host string) string { + if hasPort(host) { + return strings.TrimSuffix(host, ":") + } + return host +} diff --git a/src/net/http/request.go b/src/net/http/request.go index bac2de1a2e..a49ab36964 100644 --- a/src/net/http/request.go +++ b/src/net/http/request.go @@ -660,6 +660,8 @@ func NewRequest(method, urlStr string, body io.Reader) (*Request, error) { if !ok && body != nil { rc = ioutil.NopCloser(body) } + // The host's colon:port should be normalized. See Issue 14836. + u.Host = removeEmptyPort(u.Host) req := &Request{ Method: method, URL: u, diff --git a/src/net/http/request_test.go b/src/net/http/request_test.go index ff4837f2fa..82c7af3cda 100644 --- a/src/net/http/request_test.go +++ b/src/net/http/request_test.go @@ -398,11 +398,13 @@ var newRequestHostTests = []struct { {"http://192.168.0.1/", "192.168.0.1"}, {"http://192.168.0.1:8080/", "192.168.0.1:8080"}, + {"http://192.168.0.1:/", "192.168.0.1"}, {"http://[fe80::1]/", "[fe80::1]"}, {"http://[fe80::1]:8080/", "[fe80::1]:8080"}, {"http://[fe80::1%25en0]/", "[fe80::1%en0]"}, {"http://[fe80::1%25en0]:8080/", "[fe80::1%en0]:8080"}, + {"http://[fe80::1%25en0]:/", "[fe80::1%en0]"}, } func TestNewRequestHost(t *testing.T) { -- cgit v1.3 From 0db2bf2313cdd7711c2215fab2ae234a0f591fe8 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 15 Apr 2016 21:56:30 +0000 Subject: net/http: document Hijacker and Flusher more Fixes #15312 Change-Id: I4fabef3f21081bc4b020069851b5c2504bc6b4d8 Reviewed-on: https://go-review.googlesource.com/22122 Reviewed-by: Emmanuel Odeke Reviewed-by: Andrew Gerrand --- src/net/http/server.go | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'src/net/http') diff --git a/src/net/http/server.go b/src/net/http/server.go index 64529f1e96..da17fccbae 100644 --- a/src/net/http/server.go +++ b/src/net/http/server.go @@ -94,6 +94,10 @@ type ResponseWriter interface { // The Flusher interface is implemented by ResponseWriters that allow // an HTTP handler to flush buffered data to the client. // +// The default HTTP/1.x and HTTP/2 ResponseWriter implementations +// support Flusher, but ResponseWriter wrappers may not. Handlers +// should always test for this ability at runtime. +// // Note that even for ResponseWriters that support Flush, // if the client is connected through an HTTP proxy, // the buffered data may not reach the client until the response @@ -105,6 +109,11 @@ type Flusher interface { // The Hijacker interface is implemented by ResponseWriters that allow // an HTTP handler to take over the connection. +// +// The default ResponseWriter for HTTP/1.x connections supports +// Hijacker, but HTTP/2 connections intentionally do not. +// ResponseWriter wrappers may also not support Hijacker. Handlers +// should always test for this ability at runtime. type Hijacker interface { // Hijack lets the caller take over the connection. // After a call to Hijack(), the HTTP server library -- cgit v1.3 From 2e30218223a7bf2b560fbaf79bac8d80ea4ece1c Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 26 Apr 2016 15:43:04 -0700 Subject: net/http: remove idle transport connections from Transport when server closes Previously the Transport would cache idle connections from the Transport for later reuse, but if a peer server disconnected (e.g. idle timeout), we would not proactively remove the *persistConn from the Transport's idle list, leading to a waste of memory (potentially forever). Instead, when the persistConn's readLoop terminates, remote it from the idle list, if present. This also adds the beginning of accounting for the total number of idle connections, which will be needed for Transport.MaxIdleConns later. Updates #15461 Change-Id: Iab091f180f8dd1ee0d78f34b9705d68743b5557b Reviewed-on: https://go-review.googlesource.com/22492 Reviewed-by: Andrew Gerrand --- src/net/http/main_test.go | 14 ++++++++++++ src/net/http/transport.go | 52 ++++++++++++++++++++++++++++++++++++++---- src/net/http/transport_test.go | 48 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 109 insertions(+), 5 deletions(-) (limited to 'src/net/http') diff --git a/src/net/http/main_test.go b/src/net/http/main_test.go index 1163874ac2..d10fd89b54 100644 --- a/src/net/http/main_test.go +++ b/src/net/http/main_test.go @@ -120,3 +120,17 @@ func afterTest(t testing.TB) { } t.Errorf("Test appears to have leaked %s:\n%s", bad, stacks) } + +// waitCondition reports whether fn eventually returned true, +// checking immediately and then every checkEvery amount, +// until waitFor has elpased, at which point it returns false. +func waitCondition(waitFor, checkEvery time.Duration, fn func() bool) bool { + deadline := time.Now().Add(waitFor) + for time.Now().Before(deadline) { + if fn() { + return true + } + time.Sleep(checkEvery) + } + return false +} diff --git a/src/net/http/transport.go b/src/net/http/transport.go index 0568822737..3ccc6dd0df 100644 --- a/src/net/http/transport.go +++ b/src/net/http/transport.go @@ -65,6 +65,7 @@ const DefaultMaxIdleConnsPerHost = 2 type Transport struct { idleMu sync.Mutex wantIdle bool // user has requested to close all idle conns + idleCount int idleConn map[connectMethodKey][]*persistConn idleConnCh map[connectMethodKey]chan *persistConn @@ -166,7 +167,7 @@ type Transport struct { nextProtoOnce sync.Once h2transport *http2Transport // non-nil if http2 wired up - // TODO: tunable on global max cached connections + // TODO: MaxIdleConns tunable for global max cached connections (Issue 15461) // TODO: tunable on timeout on cached connections // TODO: tunable on max per-host TCP dials in flight (Issue 13957) } @@ -613,6 +614,7 @@ func (t *Transport) tryPutIdleConn(pconn *persistConn) error { } } t.idleConn[key] = append(t.idleConn[key], pconn) + t.idleCount++ return nil } @@ -638,13 +640,14 @@ func (t *Transport) getIdleConnCh(cm connectMethod) chan *persistConn { return ch } -func (t *Transport) getIdleConn(cm connectMethod) (pconn *persistConn) { +func (t *Transport) getIdleConn(cm connectMethod) *persistConn { key := cm.key() t.idleMu.Lock() defer t.idleMu.Unlock() if t.idleConn == nil { return nil } + var pconn *persistConn for { pconns, ok := t.idleConn[key] if !ok { @@ -659,8 +662,44 @@ func (t *Transport) getIdleConn(cm connectMethod) (pconn *persistConn) { pconn = pconns[len(pconns)-1] t.idleConn[key] = pconns[:len(pconns)-1] } - if !pconn.isBroken() { - return + t.idleCount-- + if pconn.isBroken() { + // There is a tiny window where this is + // possible, between the connecting dying and + // the persistConn readLoop calling + // Transport.removeIdleConn. Just skip it and + // carry on. + continue + } + return pconn + } +} + +// removeIdleConn marks pconn as dead. +func (t *Transport) removeIdleConn(pconn *persistConn) { + key := pconn.cacheKey + t.idleMu.Lock() + defer t.idleMu.Unlock() + + pconns, _ := t.idleConn[key] + switch len(pconns) { + case 0: + // Nothing + case 1: + if pconns[0] == pconn { + t.idleCount-- + delete(t.idleConn, key) + } + default: + // TODO(bradfitz): map into LRU element? + for i, v := range pconns { + if v != pconn { + continue + } + pconns[i] = pconns[len(pconns)-1] + t.idleConn[key] = pconns[:len(pconns)-1] + t.idleCount-- + break } } } @@ -1120,7 +1159,10 @@ func (pc *persistConn) cancelRequest() { func (pc *persistConn) readLoop() { closeErr := errReadLoopExiting // default value, if not changed below - defer func() { pc.close(closeErr) }() + defer func() { + pc.close(closeErr) + pc.t.removeIdleConn(pc) + }() tryPutIdleConn := func() bool { if err := pc.t.tryPutIdleConn(pc); err != nil { diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go index 1aa26610b0..2e27cc1850 100644 --- a/src/net/http/transport_test.go +++ b/src/net/http/transport_test.go @@ -438,6 +438,54 @@ func TestTransportMaxPerHostIdleConns(t *testing.T) { } } +func TestTransportRemovesDeadIdleConnections(t *testing.T) { + defer afterTest(t) + ts := httptest.NewServer(HandlerFunc(func(w ResponseWriter, r *Request) { + io.WriteString(w, r.RemoteAddr) + })) + defer ts.Close() + + tr := &Transport{} + defer tr.CloseIdleConnections() + c := &Client{Transport: tr} + + doReq := func(name string) string { + // Do a POST instead of a GET to prevent the Transport's + // idempotent request retry logic from kicking in... + res, err := c.Post(ts.URL, "", nil) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + if res.StatusCode != 200 { + t.Fatalf("%s: %v", name, res.Status) + } + defer res.Body.Close() + slurp, err := ioutil.ReadAll(res.Body) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + return string(slurp) + } + + first := doReq("first") + keys1 := tr.IdleConnKeysForTesting() + + ts.CloseClientConnections() + + var keys2 []string + if !waitCondition(3*time.Second, 50*time.Millisecond, func() bool { + keys2 = tr.IdleConnKeysForTesting() + return len(keys2) == 0 + }) { + t.Fatalf("Transport didn't notice idle connection's death.\nbefore: %q\n after: %q\n", keys1, keys2) + } + + second := doReq("second") + if first == second { + t.Errorf("expected a different connection between requests. got %q both times", first) + } +} + func TestTransportServerClosingUnexpectedly(t *testing.T) { setParallel(t) defer afterTest(t) -- cgit v1.3