net: avoid string operation and make valid domain names explicit

Having a trailing dot in the string doesn't really simplify
the checking loop in isDomainName. Avoid this unnecessary allocation.
Also make the valid domain names more explicit by adding some more
test cases.

benchmark            old ns/op    new ns/op    delta
BenchmarkDNSNames       2420.0        983.0  -59.38%

benchmark           old allocs   new allocs    delta
BenchmarkDNSNames           12            0  -100.00%

benchmark            old bytes    new bytes    delta
BenchmarkDNSNames          336            0  -100.00%

R=golang-dev, bradfitz
CC=golang-dev
https://golang.org/cl/12662043

diff --git a/src/pkg/net/dnsclient.go b/src/pkg/net/dnsclient.go
index 76b192645aafc2442f34e7d7b4044fa7d54f34ed..01db4372945c230f7df3a4f6d72bb7bae353e321 100644 (file)
--- a/src/pkg/net/dnsclient.go
+++ b/src/pkg/net/dnsclient.go
@@ -122,12 +122,9 @@ func isDomainName(s string) bool {
        if len(s) > 255 {
                return false
        }
-       if s[len(s)-1] != '.' { // simplify checking loop: make name end in dot
-               s += "."
-       }
 
        last := byte('.')
-       ok := false // ok once we've seen a letter
+       ok := false // Ok once we've seen a letter.
        partlen := 0
        for i := 0; i < len(s); i++ {
                c := s[i]
@@ -141,13 +138,13 @@ func isDomainName(s string) bool {
                        // fine
                        partlen++
                case c == '-':
-                       // byte before dash cannot be dot
+                       // Byte before dash cannot be dot.
                        if last == '.' {
                                return false
                        }
                        partlen++
                case c == '.':
-                       // byte before dot cannot be dot, dash
+                       // Byte before dot cannot be dot, dash.
                        if last == '.' || last == '-' {
                                return false
                        }
@@ -158,6 +155,9 @@ func isDomainName(s string) bool {
                }
                last = c
        }
+       if last == '-' || partlen > 63 {
+               return false
+       }
 
        return ok
 }

diff --git a/src/pkg/net/dnsname_test.go b/src/pkg/net/dnsname_test.go
index 70df693f789a8f0fe4c91a4222166738b3ba506d..57dd25fe4c6dec1752858c13c926ed1bc228a74a 100644 (file)
--- a/src/pkg/net/dnsname_test.go
+++ b/src/pkg/net/dnsname_test.go
@@ -5,6 +5,7 @@
 package net
 
 import (
+       "strings"
        "testing"
 )
 
@@ -16,7 +17,6 @@ type testCase struct {
 var tests = []testCase{
        // RFC2181, section 11.
        {"_xmpp-server._tcp.google.com", true},
-       {"_xmpp-server._tcp.google.com", true},
        {"foo.com", true},
        {"1foo.com", true},
        {"26.0.0.73.com", true},
@@ -24,6 +24,10 @@ var tests = []testCase{
        {"fo1o.com", true},
        {"foo1.com", true},
        {"a.b..com", false},
+       {"a.b-.com", false},
+       {"a.b.com-", false},
+       {"a.b..", false},
+       {"b.com.", true},
 }
 
 func getTestCases(ch chan<- testCase) {
@@ -63,3 +67,17 @@ func TestDNSNames(t *testing.T) {
                }
        }
 }
+
+func BenchmarkDNSNames(b *testing.B) {
+       benchmarks := append(tests, []testCase{
+               {strings.Repeat("a", 63), true},
+               {strings.Repeat("a", 64), false},
+       }...)
+       for n := 0; n < b.N; n++ {
+               for _, tc := range benchmarks {
+                       if isDomainName(tc.name) != tc.result {
+                               b.Errorf("isDomainName(%q) = %v; want %v", tc.name, !tc.result, tc.result)
+                       }
+               }
+       }
+}