git » dnss » commit a8b9d89

diff --git a/internal/dnsserver/caching_test.go b/internal/dnsserver/caching_test.go
index 131eb71..522efe6 100644
--- a/internal/dnsserver/caching_test.go
+++ b/internal/dnsserver/caching_test.go
@@ -112,6 +112,67 @@ func TestTTL(t *testing.T) {
 	}
 }
 
+// Test that we use the minimum TTL of all the RRs in the answer.
+func TestMinTTL(t *testing.T) {
+	r := testutil.NewTestResolver()
+	c := NewCachingResolver(r)
+	c.Init()
+	resetStats()
+
+	tr := trace.New("test", "TestMinTTL")
+	defer tr.Finish()
+
+	// Long-lived CNAME to a short-lived A: should not be cached, as the A
+	// TTL is below minTTL.
+	r.Response = newReply(mustNewRR(t, "test. 86400 CNAME cdn.test."))
+	r.Response.Answer = append(r.Response.Answer,
+		mustNewRR(t, "cdn.test. 30 A 1.2.3.4"))
+	req := newQuery("test.", dns.TypeA)
+	for i := 0; i < 2; i++ {
+		if _, err := c.Query(req, tr); err != nil {
+			t.Fatalf("query failed: %v", err)
+		}
+	}
+	if !statsEquals(2, 0, 2) {
+		t.Errorf("bad stats: %v", dumpStats())
+	}
+
+	// Same, but with an A TTL that is cacheable: all RRs should get it.
+	resetStats()
+	r.Response = newReply(mustNewRR(t, "test2. 86400 CNAME cdn.test."))
+	r.Response.Answer = append(r.Response.Answer,
+		mustNewRR(t, "cdn.test. 600 A 1.2.3.4"))
+	req = newQuery("test2.", dns.TypeA)
+	for i := 0; i < 2; i++ {
+		resp, err := c.Query(req, tr)
+		if err != nil {
+			t.Fatalf("query failed: %v", err)
+		}
+		for _, rr := range resp.Answer {
+			if rr.Header().Ttl != 600 {
+				t.Errorf("expected TTL 600, got %v", rr)
+			}
+		}
+	}
+	if !statsEquals(2, 1, 1) {
+		t.Errorf("bad stats: %v", dumpStats())
+	}
+
+	// TTLs with the most significant bit set should be treated as 0, so
+	// not cached.
+	resetStats()
+	r.Response = newReply(mustNewRR(t, "test3. 2147483648 A 1.2.3.4"))
+	req = newQuery("test3.", dns.TypeA)
+	for i := 0; i < 2; i++ {
+		if _, err := c.Query(req, tr); err != nil {
+			t.Fatalf("query failed: %v", err)
+		}
+	}
+	if !statsEquals(2, 0, 2) {
+		t.Errorf("bad stats: %v", dumpStats())
+	}
+}
+
 // Test that we don't cache failed queries.
 func TestFailedQueries(t *testing.T) {
 	r := testutil.NewTestResolver()
diff --git a/internal/dnsserver/resolver.go b/internal/dnsserver/resolver.go
index 3392d37..bc60008 100644
--- a/internal/dnsserver/resolver.go
+++ b/internal/dnsserver/resolver.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"expvar"
 	"fmt"
+	"math"
 	"net/http"
 	"sort"
 	"sync"
@@ -253,22 +254,34 @@ func wantToCache(question dns.Question, reply *dns.Msg) error {
 }
 
 func limitTTL(answer []dns.RR) time.Duration {
-	// This assumes all RRs have the same TTL.  That may not be the case in
-	// theory, but we are ok not caring for this for now.
-	ttl := time.Duration(answer[0].Header().Ttl) * time.Second
-
-	// This helps prevent cache pollution due to unused but long entries, as
-	// we don't do usage-based caching yet.
-	if ttl > maxTTL {
-		ttl = maxTTL
+	// Use the minimum TTL of all the RRs, as the answer is only valid as
+	// long as all of them are. For example, a long-lived CNAME pointing to a
+	// short-lived A record.
+	//
+	// Starting from maxTTL caps long TTLs. This helps prevent cache
+	// pollution due to unused but long entries, as we don't do usage-based
+	// caching yet.
+	ttl := maxTTL
+	for _, rr := range answer {
+		rrTTL := rr.Header().Ttl
+
+		// RFC 2181 section 8: TTLs with the most significant bit set
+		// should be treated as 0.
+		if rrTTL > math.MaxInt32 {
+			rrTTL = 0
+		}
+
+		if t := time.Duration(rrTTL) * time.Second; t < ttl {
+			ttl = t
+		}
 	}
 
 	return ttl
 }
 
 func getTTL(answer []dns.RR) time.Duration {
-	// This assumes all RRs have the same TTL.  That may not be the case in
-	// theory, but we are ok not caring for this for now.
+	// This assumes all RRs have the same TTL, which is true for the cached
+	// answers because we set it with setTTL.
 	return time.Duration(answer[0].Header().Ttl) * time.Second
 }