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/request.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/net/http/request.go') 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-5-g9baa 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/request.go') 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-5-g9baa 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/request.go') 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-5-g9baa 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/request.go') 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-5-g9baa