From 4493e38f6281e7a29a1e34fada861f8ccae1feea Mon Sep 17 00:00:00 2001
From: krashanoff <lkrashanoff2@gmail.com>
Date: Fri, 3 Jul 2020 00:33:32 -0700
Subject: [PATCH] Remove unnecessary export. Clarify comments. Address
 golangci-lint warnings.

Closes #972
---
 js/runner_test.go   |  2 +-
 lib/options.go      | 27 ++++++++++++++-------------
 lib/options_test.go |  3 ++-
 3 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/js/runner_test.go b/js/runner_test.go
index a3e58d609dd6..a62f683cd23f 100644
--- a/js/runner_test.go
+++ b/js/runner_test.go
@@ -828,7 +828,7 @@ func TestVUIntegrationBlockHostnamesOption(t *testing.T) {
 	}
 
 	hostnames := lib.HostnameTrie{}
-	if err := hostnames.Insert("*.io"); !assert.NoError(t, err) {
+	if insertErr := hostnames.Insert("*.io"); !assert.NoError(t, insertErr) {
 		return
 	}
 	require.NoError(t, r1.SetOptions(lib.Options{
diff --git a/lib/options.go b/lib/options.go
index 09e45556a4b1..b8e1173a2fd6 100644
--- a/lib/options.go
+++ b/lib/options.go
@@ -193,21 +193,20 @@ func ParseCIDR(s string) (*IPNet, error) {
 // for wildcards exclusively at the start of the pattern. Items may only
 // be inserted and searched. Internationalized hostnames are valid.
 type HostnameTrie struct {
-	r        rune
 	children []*HostnameTrie
+	r        rune
 	terminal bool // end of a valid match
 }
 
-// describes a valid hostname pattern to block by. Global var to avoid
-// compilation penalty each call to ValidHostname.
-var validHostnamePattern *regexp.Regexp = regexp.MustCompile("^\\*?(\\pL|[0-9\\.])*")
+// Regex description of hostname pattern to enforce blocks by. Global var
+// to avoid compilation penalty at runtime.
+// Matches against strings composed entirely of letters, numbers, or '.'s
+// with an optional wildcard at the start.
+var legalHostnamePattern *regexp.Regexp = regexp.MustCompile("^\\*?(\\pL|[0-9\\.])*")
 
-// ValidHostname returns whether the provided hostname pattern
-// has an optional wildcard at the start, and is composed entirely
-// of letters, numbers, or '.'s.
-func ValidHostname(s string) error {
-	if len(validHostnamePattern.FindString(s)) != len(s) {
-		return fmt.Errorf("invalid hostname pattern %s", s)
+func legalHostname(s string) error {
+	if len(legalHostnamePattern.FindString(s)) != len(s) {
+		return errors.Errorf("invalid hostname pattern %s", s)
 	}
 	return nil
 }
@@ -238,13 +237,14 @@ func (t *HostnameTrie) UnmarshalText(b []byte) error {
 	return nil
 }
 
-// Insert a string into the given HostnameTrie.
+// Insert a hostname pattern into the given HostnameTrie. Returns an error
+// if hostname pattern is illegal.
 func (t *HostnameTrie) Insert(s string) error {
 	if len(s) == 0 {
 		return nil
 	}
 
-	if err := ValidHostname(s); err != nil {
+	if err := legalHostname(s); err != nil {
 		return err
 	}
 
@@ -256,7 +256,7 @@ func (t *HostnameTrie) Insert(s string) error {
 		}
 	}
 
-	n := &HostnameTrie{rStr[last], nil, len(rStr) == 1}
+	n := &HostnameTrie{nil, rStr[last], len(rStr) == 1}
 	t.children = append(t.children, n)
 	return n.Insert(string(rStr[:last]))
 }
@@ -272,6 +272,7 @@ func (t *HostnameTrie) Contains(s string) (bool, string) {
 	return false, ""
 }
 
+// recursively traverse HostnameTrie children searching for a match.
 func (t *HostnameTrie) childContains(s string, match string) (bool, string) {
 	if len(s) == 0 {
 		return false, ""
diff --git a/lib/options_test.go b/lib/options_test.go
index 99c27137e733..765fa63b162f 100644
--- a/lib/options_test.go
+++ b/lib/options_test.go
@@ -320,7 +320,8 @@ func TestOptions(t *testing.T) {
 	})
 	t.Run("BlockedHostnames", func(t *testing.T) {
 		hostnames := HostnameTrie{}
-		hostnames.Insert("*")
+		err := hostnames.Insert("*")
+		assert.Nil(t, err)
 		opts := Options{}.Apply(Options{
 			BlockedHostnames: &hostnames,
 		})