net/http: make Transport respect {X-,}Idempotency-Key header

Fixes #19943

Change-Id: I5e0fefe44791d7b3556095d726c2a753ec551ef2
Reviewed-on: https://go-review.googlesource.com/c/147457
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>

diff --git a/src/net/http/export_test.go b/src/net/http/export_test.go
index 716e8ecac70a90c3596240e59b2211f388db70a4..b6965c239e95ecbdcf000d0f53dd7744a55546fa 100644 (file)
--- a/src/net/http/export_test.go
+++ b/src/net/http/export_test.go
@@ -242,3 +242,5 @@ func ExportSetH2GoawayTimeout(d time.Duration) (restore func()) {
        http2goAwayTimeout = d
        return func() { http2goAwayTimeout = old }
 }
+
+func (r *Request) ExportIsReplayable() bool { return r.isReplayable() }

diff --git a/src/net/http/header.go b/src/net/http/header.go
index 611ee047050c76062ff61769199c193c01893ccf..6cf13e5c4412654ba5dfd3ac8ce9f8e9adf6feb4 100644 (file)
--- a/src/net/http/header.go
+++ b/src/net/http/header.go
@@ -52,6 +52,13 @@ func (h Header) get(key string) string {
        return ""
 }
 
+// has reports whether h has the provided key defined, even if it's
+// set to 0-length slice.
+func (h Header) has(key string) bool {
+       _, ok := h[key]
+       return ok
+}
+
 // Del deletes the values associated with key.
 // The key is case insensitive; it is canonicalized by
 // textproto.CanonicalMIMEHeaderKey.

diff --git a/src/net/http/request.go b/src/net/http/request.go
index 0bcdeae0df1f582cc57a7c0c5bbbe18346e8e616..5b7e6564ae5044aa345dc45bdeb982d526a4068f 100644 (file)
--- a/src/net/http/request.go
+++ b/src/net/http/request.go
@@ -579,7 +579,7 @@ func (r *Request) write(w io.Writer, usingProxy bool, extraHeaders Header, waitF
        // Use the defaultUserAgent unless the Header contains one, which
        // may be blank to not send the header.
        userAgent := defaultUserAgent
-       if _, ok := r.Header["User-Agent"]; ok {
+       if r.Header.has("User-Agent") {
                userAgent = r.Header.Get("User-Agent")
        }
        if userAgent != "" {
@@ -1345,6 +1345,12 @@ func (r *Request) isReplayable() bool {
                case "GET", "HEAD", "OPTIONS", "TRACE":
                        return true
                }
+               // The Idempotency-Key, while non-standard, is widely used to
+               // mean a POST or other request is idempotent. See
+               // https://golang.org/issue/19943#issuecomment-421092421
+               if r.Header.has("Idempotency-Key") || r.Header.has("X-Idempotency-Key") {
+                       return true
+               }
        }
        return false
 }

diff --git a/src/net/http/requestwrite_test.go b/src/net/http/requestwrite_test.go
index 246fb4e65d85217f2331c4ef052964e915501f7f..7dbf0d4e8a175328e82728835b549d41221f4acc 100644 (file)
--- a/src/net/http/requestwrite_test.go
+++ b/src/net/http/requestwrite_test.go
@@ -544,6 +544,38 @@ var reqWriteTests = []reqWriteTest{
                        "User-Agent: Go-http-client/1.1\r\n" +
                        "\r\n",
        },
+
+       // Verify that a nil header value doesn't get written.
+       23: {
+               Req: Request{
+                       Method: "GET",
+                       URL:    mustParseURL("/foo"),
+                       Header: Header{
+                               "X-Foo":             []string{"X-Bar"},
+                               "X-Idempotency-Key": nil,
+                       },
+               },
+
+               WantWrite: "GET /foo HTTP/1.1\r\n" +
+                       "Host: \r\n" +
+                       "User-Agent: Go-http-client/1.1\r\n" +
+                       "X-Foo: X-Bar\r\n\r\n",
+       },
+       24: {
+               Req: Request{
+                       Method: "GET",
+                       URL:    mustParseURL("/foo"),
+                       Header: Header{
+                               "X-Foo":             []string{"X-Bar"},
+                               "X-Idempotency-Key": []string{},
+                       },
+               },
+
+               WantWrite: "GET /foo HTTP/1.1\r\n" +
+                       "Host: \r\n" +
+                       "User-Agent: Go-http-client/1.1\r\n" +
+                       "X-Foo: X-Bar\r\n\r\n",
+       },
 }
 
 func TestRequestWrite(t *testing.T) {

diff --git a/src/net/http/server.go b/src/net/http/server.go
index a7e79c2d913762f522655a17ded1713eeaab0cac..cf03b09f844145c444c5ea9a684101924272e024 100644 (file)
--- a/src/net/http/server.go
+++ b/src/net/http/server.go
@@ -1390,7 +1390,7 @@ func (cw *chunkWriter) writeHeader(p []byte) {
                }
        }
 
-       if _, ok := header["Date"]; !ok {
+       if !header.has("Date") {
                setHeader.date = appendTime(cw.res.dateBuf[:0], time.Now())
        }
 

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index 7ef414ba53a2f52928b0df79f3024c4c54aaa02c..aa76e4f537637f2a41dd8a8d78f2d17eae18eba7 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -91,6 +91,15 @@ func init() {
 // considered a terminal status and returned by RoundTrip. To see the
 // ignored 1xx responses, use the httptrace trace package's
 // ClientTrace.Got1xxResponse.
+//
+// Transport only retries a request upon encountering a network error
+// if the request is idempotent and either has no body or has its
+// Request.GetBody defined. HTTP requests are considered idempotent if
+// they have HTTP methods GET, HEAD, OPTIONS, or TRACE; or if their
+// Header map contains an "Idempotency-Key" or "X-Idempotency-Key"
+// entry. If the idempotency key value is an zero-length slice, the
+// request is treated as idempotent but the header is not sent on the
+// wire.
 type Transport struct {
        idleMu     sync.Mutex
        wantIdle   bool                                // user has requested to close all idle conns

diff --git a/src/net/http/transport_test.go b/src/net/http/transport_test.go
index 3f9750392c61d0680fcae6a4b879a8e2de976796..22ca3f9550e40d15f2b4444178830ff8d0a9a067 100644 (file)
--- a/src/net/http/transport_test.go
+++ b/src/net/http/transport_test.go
@@ -4952,3 +4952,56 @@ func TestTransportCONNECTBidi(t *testing.T) {
                }
        }
 }
+
+func TestTransportRequestReplayable(t *testing.T) {
+       someBody := ioutil.NopCloser(strings.NewReader(""))
+       tests := []struct {
+               name string
+               req  *Request
+               want bool
+       }{
+               {
+                       name: "GET",
+                       req:  &Request{Method: "GET"},
+                       want: true,
+               },
+               {
+                       name: "GET_http.NoBody",
+                       req:  &Request{Method: "GET", Body: NoBody},
+                       want: true,
+               },
+               {
+                       name: "GET_body",
+                       req:  &Request{Method: "GET", Body: someBody},
+                       want: false,
+               },
+               {
+                       name: "POST",
+                       req:  &Request{Method: "POST"},
+                       want: false,
+               },
+               {
+                       name: "POST_idempotency-key",
+                       req:  &Request{Method: "POST", Header: Header{"Idempotency-Key": {"x"}}},
+                       want: true,
+               },
+               {
+                       name: "POST_x-idempotency-key",
+                       req:  &Request{Method: "POST", Header: Header{"X-Idempotency-Key": {"x"}}},
+                       want: true,
+               },
+               {
+                       name: "POST_body",
+                       req:  &Request{Method: "POST", Header: Header{"Idempotency-Key": {"x"}}, Body: someBody},
+                       want: false,
+               },
+       }
+       for _, tt := range tests {
+               t.Run(tt.name, func(t *testing.T) {
+                       got := tt.req.ExportIsReplayable()
+                       if got != tt.want {
+                               t.Errorf("replyable = %v; want %v", got, tt.want)
+                       }
+               })
+       }
+}