Address commments from Alex

This commit is contained in:
Miek Gieben 2015-08-26 14:13:14 +01:00
parent 6cf1a0db58
commit 0206070abf
2 changed files with 28 additions and 131 deletions

View File

@ -1,26 +1,13 @@
package dns package dns
import "strings"
// Dedup removes identical RRs from rrs. It preserves the original ordering. // Dedup removes identical RRs from rrs. It preserves the original ordering.
// The lowest TTL of any duplicates is used in the remaining one. // The lowest TTL of any duplicates is used in the remaining one.
//
// TODO(miek): This function will be extended to also look for CNAMEs and DNAMEs.
// if found, it will prune rrs from the "other data" that can exist. Example:
// if it finds a: a.miek.nl. CNAME foo, all other RRs with the ownername a.miek.nl.
// will be removed. When a DNAME is found all RRs with an ownername below that of
// the DNAME will be removed.
func Dedup(rrs []RR) []RR { func Dedup(rrs []RR) []RR {
m := make(map[string]RR) m := make(map[string]RR)
keys := make([]string, 0, len(rrs)) keys := make([]string, 0, len(rrs))
// Save possible cname and dname domainnames. Currently a slice, don't
// expect millions here..
cname := []string{}
dname := []string{}
for _, r := range rrs { for _, r := range rrs {
key, end := normalizedString(r) key := normalizedString(r)
keys = append(keys, key) keys = append(keys, key)
if _, ok := m[key]; ok { if _, ok := m[key]; ok {
// Shortest TTL wins. // Shortest TTL wins.
@ -30,37 +17,18 @@ func Dedup(rrs []RR) []RR {
continue continue
} }
if r.Header().Rrtype == TypeCNAME {
// we do end+3 here, so we capture the full domain name *and*
// the class field which mnemonic is always two chars.
cname = append(cname, key[:end+3])
}
if r.Header().Rrtype == TypeDNAME {
dname = append(dname, key[:end+3])
}
m[key] = r m[key] = r
} }
// If the length of the result map equals the amount of RRs we got, // If the length of the result map equals the amount of RRs we got,
// it means they were all different. We can then just return the original rrset. // it means they were all different. We can then just return the original rrset.
// We can only do this when we haven't found a CNAME or DNAME. if len(m) == len(rrs) {
if len(m) == len(rrs) && len(cname) == 0 && len(dname) == 0 {
return rrs return rrs
} }
ret := make([]RR, 0, len(rrs)) ret := make([]RR, 0, len(rrs))
for i, r := range rrs { for i, r := range rrs {
// If keys[i] lives in the map, we should copy it and remove // If keys[i] lives in the map, we should copy and remove it.
// it from the map.
if _, ok := m[keys[i]]; ok { if _, ok := m[keys[i]]; ok {
if needsDeletion(r, keys[i], cname, dname) {
// It the RR is masked by an CNAME or DNAME, we only
// delete it from the map, but don't copy it.
delete(m, keys[i])
continue
}
delete(m, keys[i]) delete(m, keys[i])
ret = append(ret, r) ret = append(ret, r)
} }
@ -74,70 +42,37 @@ func Dedup(rrs []RR) []RR {
} }
// normalizedString returns a normalized string from r. The TTL // normalizedString returns a normalized string from r. The TTL
// is removed and the domain name is lowercased. The returned integer // is removed and the domain name is lowercased. We go from this:
// is the index where the domain name ends + 1. // DomainName<TAB>TTL<TAB>CLASS<TAB>TYPE<TAB>RDATA to:
func normalizedString(r RR) (string, int) { // lowercasename<TAB>CLASS<TAB>TYPE...
func normalizedString(r RR) string {
// A string Go DNS makes has: domainname<TAB>TTL<TAB>... // A string Go DNS makes has: domainname<TAB>TTL<TAB>...
b := []byte(r.String()) b := []byte(r.String())
// find the first non-escaped tab, then another, so we capture // find the first non-escaped tab, then another, so we capture where the TTL lives.
// where the TTL lives.
esc := false esc := false
ttlStart, ttlEnd := 0, 0 ttlStart, ttlEnd := 0, 0
for i, c := range b { for i := 0; i < len(b) && ttlEnd == 0; i++ {
if c == '\\' { switch {
esc = true case b[i] == '\\':
continue esc = !esc
} case b[i] == '\t' && !esc:
if esc {
esc = false
continue
}
if c == '\t' {
if ttlStart == 0 { if ttlStart == 0 {
ttlStart = i ttlStart = i
continue continue
} }
if ttlEnd == 0 { if ttlEnd == 0 {
ttlEnd = i ttlEnd = i
break
} }
} case b[i] >= 'A' && b[i] <= 'Z' && !esc:
if c >= 'A' && c <= 'Z' { b[i] += 32
b[i] = c + 32 default:
esc = false
} }
} }
// remove TTL. // remove TTL.
copy(b[ttlStart:], b[ttlEnd:]) copy(b[ttlStart:], b[ttlEnd:])
cut := ttlEnd - ttlStart cut := ttlEnd - ttlStart
// ttlStart + 3 puts us on the start of the rdata return string(b[:len(b)-cut])
return string(b[:len(b)-cut]), ttlStart
}
// needsDeletion checks if the RR is masked by either a CNAME or a DNAME.
// If so it return true.
func needsDeletion(r RR, s string, cname, dname []string) bool {
// For CNAME we can do strings.HasPrefix with s.
// For DNAME we can do strings.Contains with s.
// Either signals a removal of this RR.
for _, c := range cname {
if strings.HasPrefix(s, c) {
if r.Header().Rrtype == TypeCNAME {
// don't delete yourself
continue
}
return true
}
}
for _, d := range dname {
if strings.Contains(s, d) {
if r.Header().Rrtype == TypeDNAME && strings.HasPrefix(s, d) {
// don't delete yourself
continue
}
return true
}
}
return false
} }

View File

@ -49,55 +49,17 @@ func TestDedup(t *testing.T) {
} }
} }
func TestDedupWithCNAMEDNAME(t *testing.T) { func BenchmarkDedup(b *testing.B) {
testcases := map[[4]RR][]string{ rrs := []RR{
[...]RR{ newRR(nil, "miEk.nl. 2000 IN A 127.0.0.1"),
newRR(t, "miek.Nl. CNAME a."), newRR(nil, "mieK.Nl. 1000 IN A 127.0.0.1"),
newRR(t, "miEk.nl. IN A 127.0.0.1"), newRR(nil, "Miek.nL. 500 IN A 127.0.0.1"),
newRR(t, "miek.Nl. IN A 127.0.0.1"),
newRR(t, "miek.de. IN A 127.0.0.1"),
}: []string{"miek.Nl.\t3600\tIN\tCNAME\ta.",
"miek.de.\t3600\tIN\tA\t127.0.0.1"},
[...]RR{
newRR(t, "Miek.nl. CNAME a."),
newRR(t, "mIek.nl. CNAME a."),
newRR(t, "miEk.nl. CNAME a."),
newRR(t, "mieK.nl. CNAME a."),
}: []string{"Miek.nl.\t3600\tIN\tCNAME\ta."},
[...]RR{
newRR(t, "miek.nl. CNAME a."),
newRR(t, "a.miek.nl. CNAME a."),
newRR(t, "a.miek.nl. CNAME a."),
newRR(t, "a.miek.nl. CNAME a."),
}: []string{"miek.nl.\t3600\tIN\tCNAME\ta.",
"a.miek.nl.\t3600\tIN\tCNAME\ta."},
[...]RR{
newRR(t, "miek.nl. DNAME a."),
newRR(t, "a.miek.nl. CNAME a."),
newRR(t, "b.miek.nl. IN A 127.0.0.1"),
newRR(t, "a.miek.de. IN A 127.0.0.1"),
}: []string{"miek.nl.\t3600\tIN\tDNAME\ta.",
"a.miek.de.\t3600\tIN\tA\t127.0.0.1"},
[...]RR{
newRR(t, "miek.nl. DNAME a."),
newRR(t, "a.miek.nl. DNAME a."),
newRR(t, "b.miek.nl. DNAME b."),
newRR(t, "a.b.miek.nl. DNAME a.b"),
}: []string{"miek.nl.\t3600\tIN\tDNAME\ta."},
} }
for i := 0; i < b.N; i++ {
for rr, expected := range testcases { Dedup(rrs)
out := Dedup([]RR{rr[0], rr[1], rr[2], rr[3]})
for i, o := range out {
if o.String() != expected[i] {
t.Fatalf("expected %v, got %v", expected[i], o.String())
}
}
} }
} }
// BenchMark test as well TODO(miek)
func TestNormalizedString(t *testing.T) { func TestNormalizedString(t *testing.T) {
tests := map[RR]string{ tests := map[RR]string{
newRR(t, "mIEk.Nl. 3600 IN A 127.0.0.1"): "miek.nl.\tIN\tA\t127.0.0.1", newRR(t, "mIEk.Nl. 3600 IN A 127.0.0.1"): "miek.nl.\tIN\tA\t127.0.0.1",
@ -105,9 +67,9 @@ func TestNormalizedString(t *testing.T) {
newRR(t, "m\\\tIeK.nl. 3600 in A 127.0.0.1"): "m\\tiek.nl.\tIN\tA\t127.0.0.1", newRR(t, "m\\\tIeK.nl. 3600 in A 127.0.0.1"): "m\\tiek.nl.\tIN\tA\t127.0.0.1",
} }
for tc, expected := range tests { for tc, expected := range tests {
a1, _ := normalizedString(tc) n := normalizedString(tc)
if a1 != expected { if n != expected {
t.Logf("expected %s, got %s", expected, a1) t.Logf("expected %s, got %s", expected, n)
t.Fail() t.Fail()
} }
} }