acme/autocert: fix race in test.
The timeNow package variable doesn't work well here: since the renewal
functionality spawns goroutines that invoke timeNow, once a test has
caused such goroutines to exist, another test can't fiddle with it
without the race detector triggering.
Instead, have a private member of Manager that the tests can set if they
need.
Change-Id: Iaf1a68d8efb84c9c5e2804aeb9cc6b2d3f3fef43
Reviewed-on: https://go-review.googlesource.com/128655
Run-TryBot: Adam Langley <agl@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
diff --git a/acme/autocert/autocert.go b/acme/autocert/autocert.go
index 23efae5..4c2fc07 100644
--- a/acme/autocert/autocert.go
+++ b/acme/autocert/autocert.go
@@ -44,7 +44,7 @@
var pseudoRand *lockedMathRand
func init() {
- src := mathrand.NewSource(timeNow().UnixNano())
+ src := mathrand.NewSource(time.Now().UnixNano())
pseudoRand = &lockedMathRand{rnd: mathrand.New(src)}
}
@@ -183,6 +183,9 @@
// for tls-alpn.
// The entries are stored for the duration of the authorization flow.
certTokens map[string]*tls.Certificate
+ // nowFunc, if not nil, returns the current time. This may be set for
+ // testing purposes.
+ nowFunc func() time.Time
}
// certKey is the key by which certificates are tracked in state, renewal and cache.
@@ -480,7 +483,7 @@
}
// verify and create TLS cert
- leaf, err := validCert(ck, pubDER, privKey)
+ leaf, err := validCert(ck, pubDER, privKey, m.now())
if err != nil {
return nil, ErrCacheMiss
}
@@ -575,7 +578,7 @@
if !ok {
return
}
- if _, err := validCert(ck, s.cert, s.key); err == nil {
+ if _, err := validCert(ck, s.cert, s.key, m.now()); err == nil {
return
}
delete(m.state, ck)
@@ -644,7 +647,7 @@
if err != nil {
return nil, nil, err
}
- leaf, err = validCert(ck, der, key)
+ leaf, err = validCert(ck, der, key, m.now())
if err != nil {
return nil, nil, err
}
@@ -988,6 +991,13 @@
return 720 * time.Hour // 30 days
}
+func (m *Manager) now() time.Time {
+ if m.nowFunc != nil {
+ return m.nowFunc()
+ }
+ return time.Now()
+}
+
// certState is ready when its mutex is unlocked for reading.
type certState struct {
sync.RWMutex
@@ -1054,7 +1064,7 @@
// are valid. It doesn't do any revocation checking.
//
// The returned value is the verified leaf cert.
-func validCert(ck certKey, der [][]byte, key crypto.Signer) (leaf *x509.Certificate, err error) {
+func validCert(ck certKey, der [][]byte, key crypto.Signer, now time.Time) (leaf *x509.Certificate, err error) {
// parse public part(s)
var n int
for _, b := range der {
@@ -1071,7 +1081,6 @@
}
// verify the leaf is not expired and matches the domain name
leaf = x509Cert[0]
- now := timeNow()
if now.Before(leaf.NotBefore) {
return nil, errors.New("acme/autocert: certificate is not valid yet")
}
@@ -1125,8 +1134,6 @@
// For easier testing.
var (
- timeNow = time.Now
-
// Called when a state is removed.
testDidRemoveState = func(certKey) {}
)
diff --git a/acme/autocert/autocert_test.go b/acme/autocert/autocert_test.go
index ced1759..95e12e1 100644
--- a/acme/autocert/autocert_test.go
+++ b/acme/autocert/autocert_test.go
@@ -986,7 +986,7 @@
{certKey{domain: "example.org"}, key3, [][]byte{cert3}, false},
}
for i, test := range tt {
- leaf, err := validCert(test.ck, test.cert, test.key)
+ leaf, err := validCert(test.ck, test.cert, test.key, now)
if err != nil && test.ok {
t.Errorf("%d: err = %v", i, err)
}
diff --git a/acme/autocert/renewal.go b/acme/autocert/renewal.go
index ef3e44e..665f870 100644
--- a/acme/autocert/renewal.go
+++ b/acme/autocert/renewal.go
@@ -128,7 +128,7 @@
}
func (dr *domainRenewal) next(expiry time.Time) time.Duration {
- d := expiry.Sub(timeNow()) - dr.m.renewBefore()
+ d := expiry.Sub(dr.m.now()) - dr.m.renewBefore()
// add a bit of randomness to renew deadline
n := pseudoRand.int63n(int64(renewJitter))
d -= time.Duration(n)
diff --git a/acme/autocert/renewal_test.go b/acme/autocert/renewal_test.go
index 634305a..5d1c63f 100644
--- a/acme/autocert/renewal_test.go
+++ b/acme/autocert/renewal_test.go
@@ -23,10 +23,10 @@
func TestRenewalNext(t *testing.T) {
now := time.Now()
- timeNow = func() time.Time { return now }
- defer func() { timeNow = time.Now }()
-
- man := &Manager{RenewBefore: 7 * 24 * time.Hour}
+ man := &Manager{
+ RenewBefore: 7 * 24 * time.Hour,
+ nowFunc: func() time.Time { return now },
+ }
defer man.stopRenew()
tt := []struct {
expiry time.Time
@@ -207,7 +207,7 @@
if err != nil {
t.Fatal(err)
}
- newLeaf, err := validCert(exampleCertKey, [][]byte{newCert}, newKey)
+ newLeaf, err := validCert(exampleCertKey, [][]byte{newCert}, newKey, now)
if err != nil {
t.Fatal(err)
}
@@ -225,7 +225,7 @@
if err != nil {
t.Fatal(err)
}
- oldLeaf, err := validCert(exampleCertKey, [][]byte{oldCert}, key)
+ oldLeaf, err := validCert(exampleCertKey, [][]byte{oldCert}, key, now)
if err != nil {
t.Fatal(err)
}
