net/http: drop custom tls.Config cloning code.

Now that we have the Clone method on tls.Config, net/http doesn't need
any custom functions to do that any more.

Change-Id: Ib60707d37f1a7f9a7d7723045f83e59eceffd026
Reviewed-on: https://go-review.googlesource.com/31595
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/crypto/tls/common.go b/src/crypto/tls/common.go
index 7199cd9d714b7c031a3aa8a01153ca7bca2a8eec..fc898e202acdad123185c647fd8d0ef5af6eb20a 100644 (file)
--- a/src/crypto/tls/common.go
+++ b/src/crypto/tls/common.go
@@ -455,9 +455,13 @@ func ticketKeyFromBytes(b [32]byte) (key ticketKey) {
        return key
 }
 
-// Clone returns a shallow clone of c.
-// Only the exported fields are copied.
+// Clone returns a shallow clone of c. It is safe to clone a Config that is
+// being used concurrently by a TLS client or server.
 func (c *Config) Clone() *Config {
+       // Running serverInit ensures that it's safe to read
+       // SessionTicketsDisabled.
+       c.serverInitOnce.Do(c.serverInit)
+
        var sessionTicketKeys []ticketKey
        c.mutex.RLock()
        sessionTicketKeys = c.sessionTicketKeys

diff --git a/src/crypto/tls/tls_test.go b/src/crypto/tls/tls_test.go
index d2a674d08b6aab5e12755a71db5eff8ead60e885..87cfa3f7f1dcdf500a8554c2bd0e710224e81664 100644 (file)
--- a/src/crypto/tls/tls_test.go
+++ b/src/crypto/tls/tls_test.go
@@ -508,6 +508,10 @@ func TestClone(t *testing.T) {
        }
 
        c2 := c1.Clone()
+       // DeepEqual also compares unexported fields, thus c2 needs to have run
+       // serverInit in order to be DeepEqual to c1. Cloning it and discarding
+       // the result is sufficient.
+       c2.Clone()
 
        if !reflect.DeepEqual(&c1, c2) {
                t.Errorf("clone failed to copy a field")

diff --git a/src/net/http/transport.go b/src/net/http/transport.go
index bce9e34de124c19cef00233557b4f6772662adb2..8162f9a998905e7b1edc6f0b023b606955923e57 100644 (file)
--- a/src/net/http/transport.go
+++ b/src/net/http/transport.go
@@ -1040,7 +1040,7 @@ func (t *Transport) dialConn(ctx context.Context, cm connectMethod) (*persistCon
 
        if cm.targetScheme == "https" && !tlsDial {
                // Initiate TLS and check remote host name against certificate.
-               cfg := cloneTLSClientConfig(t.TLSClientConfig)
+               cfg := cloneTLSConfig(t.TLSClientConfig)
                if cfg.ServerName == "" {
                        cfg.ServerName = cm.tlsHost()
                }
@@ -2099,17 +2099,9 @@ type fakeLocker struct{}
 func (fakeLocker) Lock()   {}
 func (fakeLocker) Unlock() {}
 
-// cloneTLSConfig returns a shallow clone of the exported
-// fields of cfg, ignoring the unexported sync.Once, which
-// contains a mutex and must not be copied.
-//
-// The cfg must not be in active use by tls.Server, or else
-// there can still be a race with tls.Server updating SessionTicketKey
-// and our copying it, and also a race with the server setting
-// SessionTicketsDisabled=false on failure to set the random
-// ticket key.
-//
-// If cfg is nil, a new zero tls.Config is returned.
+// clneTLSConfig returns a shallow clone of cfg, or a new zero tls.Config if
+// cfg is nil. This is safe to call even if cfg is in active use by a TLS
+// client or server.
 func cloneTLSConfig(cfg *tls.Config) *tls.Config {
        if cfg == nil {
                return &tls.Config{}
@@ -2117,38 +2109,6 @@ func cloneTLSConfig(cfg *tls.Config) *tls.Config {
        return cfg.Clone()
 }
 
-// cloneTLSClientConfig is like cloneTLSConfig but omits
-// the fields SessionTicketsDisabled and SessionTicketKey.
-// This makes it safe to call cloneTLSClientConfig on a config
-// in active use by a server.
-func cloneTLSClientConfig(cfg *tls.Config) *tls.Config {
-       if cfg == nil {
-               return &tls.Config{}
-       }
-       return &tls.Config{
-               Rand:                        cfg.Rand,
-               Time:                        cfg.Time,
-               Certificates:                cfg.Certificates,
-               NameToCertificate:           cfg.NameToCertificate,
-               GetCertificate:              cfg.GetCertificate,
-               RootCAs:                     cfg.RootCAs,
-               NextProtos:                  cfg.NextProtos,
-               ServerName:                  cfg.ServerName,
-               ClientAuth:                  cfg.ClientAuth,
-               ClientCAs:                   cfg.ClientCAs,
-               InsecureSkipVerify:          cfg.InsecureSkipVerify,
-               CipherSuites:                cfg.CipherSuites,
-               PreferServerCipherSuites:    cfg.PreferServerCipherSuites,
-               ClientSessionCache:          cfg.ClientSessionCache,
-               MinVersion:                  cfg.MinVersion,
-               MaxVersion:                  cfg.MaxVersion,
-               CurvePreferences:            cfg.CurvePreferences,
-               DynamicRecordSizingDisabled: cfg.DynamicRecordSizingDisabled,
-               Renegotiation:               cfg.Renegotiation,
-               KeyLogWriter:                cfg.KeyLogWriter,
-       }
-}
-
 type connLRU struct {
        ll *list.List // list.Element.Value type is of *persistConn
        m  map[*persistConn]*list.Element