git » dnss » commit eafe242

Documentation and style improvements

author Alberto Bertogli
2017-11-19 13:34:38 UTC
committer Alberto Bertogli
2017-11-19 13:34:38 UTC
parent 83905773845a7659c2647a0085a5c390e552322d

Documentation and style improvements

This patch improves internal documentation, and fixes some style issues
(mostly detected by the linter).

There are no functional or logic changes.

internal/dnsjson/dnsjson.go +10 -4
internal/dnstohttps/resolver.go +6 -2
internal/dnstohttps/server.go +15 -8
internal/httpstodns/parser_test.go +9 -9
internal/httpstodns/server.go +32 -23
internal/util/trace.go +4 -0

diff --git a/internal/dnsjson/dnsjson.go b/internal/dnsjson/dnsjson.go
index 820ea9f..a6a0bc3 100644
--- a/internal/dnsjson/dnsjson.go
+++ b/internal/dnsjson/dnsjson.go
@@ -3,6 +3,9 @@
 // Matches the API implemented by https://dns.google.com/.
 package dnsjson
 
+// Response is the highest level struct in the DNS JSON response.
+// Note the fields must match the JSON API specified at
+// https://developers.google.com/speed/public-dns/docs/dns-over-https#dns_response_in_json/.
 type Response struct {
 	Status   int  // Standard DNS response code (32 bit integer).
 	TC       bool // Whether the response is truncated
@@ -14,9 +17,12 @@ type Response struct {
 	Answer   []RR // Answer to the question.
 }
 
+// RR represents a JSON-encoded DNS RR.
+// Note the fields must match the JSON API specified at
+// https://developers.google.com/speed/public-dns/docs/dns-over-https#dns_response_in_json/.
 type RR struct {
-	Name string `json:name`
-	Type uint16 `json:type`
-	TTL  uint32 `json:TTL`
-	Data string `json:data`
+	Name string // FQDN for the RR.
+	Type uint16 // DNS RR type.
+	TTL  uint32 // Record's time to live, in seconds.
+	Data string // Data for the record (e.g. for A it's the IP address).
 }
diff --git a/internal/dnstohttps/resolver.go b/internal/dnstohttps/resolver.go
index 5309521..b0376d9 100644
--- a/internal/dnstohttps/resolver.go
+++ b/internal/dnstohttps/resolver.go
@@ -21,7 +21,7 @@ import (
 	"bytes"
 )
 
-// Interface for DNS resolvers that can answer queries.
+// Resolver is the interface for DNS resolvers that can answer queries.
 type Resolver interface {
 	// Initialize the resolver.
 	Init() error
@@ -59,6 +59,8 @@ func loadCertPool(caFile string) (*x509.CertPool, error) {
 	return pool, nil
 }
 
+// NewHTTPSResolver creates a new resolver which uses the given upstream URL
+// to resolve queries.
 func NewHTTPSResolver(upstream, caFile string) *httpsResolver {
 	return &httpsResolver{
 		Upstream: upstream,
@@ -168,7 +170,7 @@ func (r *httpsResolver) Query(req *dns.Msg, tr trace.Trace) (*dns.Msg, error) {
 			CheckingDisabled:   jr.CD,
 		},
 		Question: []dns.Question{
-			dns.Question{
+			{
 				Name:   jr.Question[0].Name,
 				Qtype:  jr.Question[0].Type,
 				Qclass: dns.ClassINET,
@@ -208,6 +210,8 @@ type cachingResolver struct {
 	mu *sync.RWMutex
 }
 
+// NewCachingResolver returns a new resolver which implements a cache on top
+// of the given one.
 func NewCachingResolver(back Resolver) *cachingResolver {
 	return &cachingResolver{
 		back:   back,
diff --git a/internal/dnstohttps/server.go b/internal/dnstohttps/server.go
index 289f40e..3df711a 100644
--- a/internal/dnstohttps/server.go
+++ b/internal/dnstohttps/server.go
@@ -21,11 +21,11 @@ import (
 // newID is a channel used to generate new request IDs.
 // There is a goroutine created at init() time that will get IDs randomly, to
 // help prevent guesses.
-var newId chan uint16
+var newID chan uint16
 
 func init() {
 	// Buffer 100 numbers to avoid blocking on crypto rand.
-	newId = make(chan uint16, 100)
+	newID = make(chan uint16, 100)
 
 	go func() {
 		var id uint16
@@ -37,12 +37,14 @@ func init() {
 				panic(fmt.Sprintf("error creating id: %v", err))
 			}
 
-			newId <- id
+			newID <- id
 		}
 
 	}()
 }
 
+// Server implements a DNS proxy, which will (mostly) use the given resolver
+// to resolve queries.
 type Server struct {
 	Addr        string
 	unqUpstream string
@@ -52,6 +54,8 @@ type Server struct {
 	fallbackUpstream string
 }
 
+// New *Server, which will listen on addr, use resolver as the backend
+// resolver, and use unqUpstream to resolve unqualified queries.
 func New(addr string, resolver Resolver, unqUpstream string) *Server {
 	return &Server{
 		Addr:            addr,
@@ -61,6 +65,7 @@ func New(addr string, resolver Resolver, unqUpstream string) *Server {
 	}
 }
 
+// SetFallback upstream server for the given domains.
 func (s *Server) SetFallback(upstream string, domains []string) {
 	s.fallbackUpstream = upstream
 	for _, d := range domains {
@@ -68,6 +73,7 @@ func (s *Server) SetFallback(upstream string, domains []string) {
 	}
 }
 
+// Handler for the incoming DNS queries.
 func (s *Server) Handler(w dns.ResponseWriter, r *dns.Msg) {
 	tr := trace.New("dnstohttp", "Handler")
 	defer tr.Finish()
@@ -121,9 +127,9 @@ func (s *Server) Handler(w dns.ResponseWriter, r *dns.Msg) {
 	// Create our own IDs, in case different users pick the same id and we
 	// pass that upstream.
 	oldid := r.Id
-	r.Id = <-newId
+	r.Id = <-newID
 
-	from_up, err := s.resolver.Query(r, tr)
+	fromUp, err := s.resolver.Query(r, tr)
 	if err != nil {
 		glog.Infof(err.Error())
 		tr.LazyPrintf(err.Error())
@@ -131,12 +137,13 @@ func (s *Server) Handler(w dns.ResponseWriter, r *dns.Msg) {
 		return
 	}
 
-	util.TraceAnswer(tr, from_up)
+	util.TraceAnswer(tr, fromUp)
 
-	from_up.Id = oldid
-	w.WriteMsg(from_up)
+	fromUp.Id = oldid
+	w.WriteMsg(fromUp)
 }
 
+// ListenAndServe launches the DNS proxy.
 func (s *Server) ListenAndServe() {
 	err := s.resolver.Init()
 	if err != nil {
diff --git a/internal/httpstodns/parser_test.go b/internal/httpstodns/parser_test.go
index 52e8ce8..746e774 100644
--- a/internal/httpstodns/parser_test.go
+++ b/internal/httpstodns/parser_test.go
@@ -79,15 +79,15 @@ func Test(t *testing.T) {
 		raw string
 		err error
 	}{
-		{"", emptyNameErr},
-		{"name=" + longName, nameTooLongErr},
-		{"name=x;type=0", intOutOfRangeErr},
-		{"name=x;type=-1", intOutOfRangeErr},
-		{"name=x;type=65536", unknownType},
-		{"name=x;type=merienda", unknownType},
-		{"name=x;cd=lala", invalidCD},
-		{"name=x;edns_client_subnet=lala", invalidSubnetErr},
-		{"name=x;edns_client_subnet=1.2.3.4", invalidSubnetErr},
+		{"", errEmptyName},
+		{"name=" + longName, errNameTooLong},
+		{"name=x;type=0", errIntOutOfRange},
+		{"name=x;type=-1", errIntOutOfRange},
+		{"name=x;type=65536", errUnknownType},
+		{"name=x;type=merienda", errUnknownType},
+		{"name=x;cd=lala", errInvalidCD},
+		{"name=x;edns_client_subnet=lala", errInvalidSubnet},
+		{"name=x;edns_client_subnet=1.2.3.4", errInvalidSubnet},
 	}
 	for _, c := range errCases {
 		_, err := parseQuery(makeURL(t, c.raw))
diff --git a/internal/httpstodns/server.go b/internal/httpstodns/server.go
index c5d4ca3..6c75ae1 100644
--- a/internal/httpstodns/server.go
+++ b/internal/httpstodns/server.go
@@ -18,6 +18,8 @@ import (
 	"golang.org/x/net/trace"
 )
 
+// Server is an HTTPS server that implements DNS over HTTPS, as specified in
+// https://developers.google.com/speed/public-dns/docs/dns-over-https#api_specification.
 type Server struct {
 	Addr     string
 	Upstream string
@@ -25,8 +27,11 @@ type Server struct {
 	KeyFile  string
 }
 
+// InsecureForTesting = true will make Server.ListenAndServe will not use TLS.
+// This is only useful for integration testing purposes.
 var InsecureForTesting = false
 
+// ListenAndServe starts the HTTPS server.
 func (s *Server) ListenAndServe() {
 	mux := http.NewServeMux()
 	mux.HandleFunc("/resolve", s.Resolve)
@@ -45,6 +50,10 @@ func (s *Server) ListenAndServe() {
 	glog.Fatalf("HTTPS exiting: %s", err)
 }
 
+// Resolve "DNS over HTTPS" requests, and returns responses as specified in
+// https://developers.google.com/speed/public-dns/docs/dns-over-https#api_specification.
+// It implements an http.HandlerFunc so it can be used with any standard Go
+// HTTP server.
 func (s *Server) Resolve(w http.ResponseWriter, req *http.Request) {
 	tr := trace.New("httpstodns", "/resolve")
 	defer tr.Finish()
@@ -88,32 +97,32 @@ func (s *Server) Resolve(w http.ResponseWriter, req *http.Request) {
 	util.TraceQuestion(tr, r.Question)
 
 	// Do the DNS request, get the reply.
-	from_up, err := dns.Exchange(r, s.Upstream)
+	fromUp, err := dns.Exchange(r, s.Upstream)
 	if err != nil {
 		err = util.TraceErrorf(tr, "dns exchange error: %v", err)
 		http.Error(w, err.Error(), http.StatusFailedDependency)
 		return
 	}
 
-	if from_up == nil {
+	if fromUp == nil {
 		err = util.TraceErrorf(tr, "no response from upstream")
 		http.Error(w, err.Error(), http.StatusRequestTimeout)
 		return
 	}
 
-	util.TraceAnswer(tr, from_up)
+	util.TraceAnswer(tr, fromUp)
 
 	// Convert the reply to json, and write it back.
 	jr := &dnsjson.Response{
-		Status: from_up.Rcode,
-		TC:     from_up.Truncated,
-		RD:     from_up.RecursionDesired,
-		RA:     from_up.RecursionAvailable,
-		AD:     from_up.AuthenticatedData,
-		CD:     from_up.CheckingDisabled,
+		Status: fromUp.Rcode,
+		TC:     fromUp.Truncated,
+		RD:     fromUp.RecursionDesired,
+		RA:     fromUp.RecursionAvailable,
+		AD:     fromUp.AuthenticatedData,
+		CD:     fromUp.CheckingDisabled,
 	}
 
-	for _, q := range from_up.Question {
+	for _, q := range fromUp.Question {
 		rr := dnsjson.RR{
 			Name: q.Name,
 			Type: q.Qtype,
@@ -121,7 +130,7 @@ func (s *Server) Resolve(w http.ResponseWriter, req *http.Request) {
 		jr.Question = append(jr.Question, rr)
 	}
 
-	for _, a := range from_up.Answer {
+	for _, a := range fromUp.Answer {
 		hdr := a.Header()
 		ja := dnsjson.RR{
 			Name: hdr.Name,
@@ -158,12 +167,12 @@ func (q query) String() string {
 }
 
 var (
-	emptyNameErr     = fmt.Errorf("empty name")
-	nameTooLongErr   = fmt.Errorf("name too long")
-	invalidSubnetErr = fmt.Errorf("invalid edns_client_subnet")
-	intOutOfRangeErr = fmt.Errorf("invalid type (int out of range)")
-	unknownType      = fmt.Errorf("invalid type (unknown string type)")
-	invalidCD        = fmt.Errorf("invalid cd value")
+	errEmptyName     = fmt.Errorf("empty name")
+	errNameTooLong   = fmt.Errorf("name too long")
+	errInvalidSubnet = fmt.Errorf("invalid edns_client_subnet")
+	errIntOutOfRange = fmt.Errorf("invalid type (int out of range)")
+	errUnknownType   = fmt.Errorf("invalid type (unknown string type)")
+	errInvalidCD     = fmt.Errorf("invalid cd value")
 )
 
 func parseQuery(u *url.URL) (query, error) {
@@ -187,10 +196,10 @@ func parseQuery(u *url.URL) (query, error) {
 	var err error
 
 	if q.name, ok = vs["name"]; !ok || q.name == "" {
-		return q, emptyNameErr
+		return q, errEmptyName
 	}
 	if len(q.name) > 253 {
-		return q, nameTooLongErr
+		return q, errNameTooLong
 	}
 
 	if _, ok = vs["type"]; ok {
@@ -210,7 +219,7 @@ func parseQuery(u *url.URL) (query, error) {
 	if clientSubnet, ok := vs["edns_client_subnet"]; ok {
 		_, q.clientSubnet, err = net.ParseCIDR(clientSubnet)
 		if err != nil {
-			return q, invalidSubnetErr
+			return q, errInvalidSubnet
 		}
 	}
 
@@ -226,12 +235,12 @@ func stringToRRType(s string) (uint16, error) {
 		if 1 <= i && i <= 65535 {
 			return uint16(i), nil
 		}
-		return 0, intOutOfRangeErr
+		return 0, errIntOutOfRange
 	}
 
 	rrType, ok := dns.StringToType[strings.ToUpper(s)]
 	if !ok {
-		return 0, unknownType
+		return 0, errUnknownType
 	}
 	return rrType, nil
 }
@@ -246,5 +255,5 @@ func stringToBool(s string) (bool, error) {
 		return false, nil
 	}
 
-	return false, invalidCD
+	return false, errInvalidCD
 }
diff --git a/internal/util/trace.go b/internal/util/trace.go
index 167265d..7cfc8f6 100644
--- a/internal/util/trace.go
+++ b/internal/util/trace.go
@@ -9,6 +9,7 @@ import (
 	"golang.org/x/net/trace"
 )
 
+// TraceQuestion adds the given question to the trace.
 func TraceQuestion(tr trace.Trace, qs []dns.Question) {
 	if !glog.V(3) {
 		return
@@ -26,6 +27,7 @@ func questionsToString(qs []dns.Question) string {
 	return "Q: " + strings.Join(s, " ; ")
 }
 
+// TraceAnswer adds the given DNS answer to the trace.
 func TraceAnswer(tr trace.Trace, m *dns.Msg) {
 	if !glog.V(3) {
 		return
@@ -37,12 +39,14 @@ func TraceAnswer(tr trace.Trace, m *dns.Msg) {
 	}
 }
 
+// TraceError adds the given error to the trace.
 func TraceError(tr trace.Trace, err error) {
 	glog.Info(err.Error())
 	tr.LazyPrintf(err.Error())
 	tr.SetError()
 }
 
+// TraceErrorf adds an error message to the trace.
 func TraceErrorf(tr trace.Trace, format string, a ...interface{}) error {
 	err := fmt.Errorf(format, a...)
 	TraceError(tr, err)