Improved color validation - now with fixes and a table driven test

server.go

@@ -10,6 +10,7 @@ import (
 	"math/big"
 	"net"
 	"path/filepath"
+	"regexp"
 	"strconv"
 	"sync"
 	"sync/atomic"
@@ -70,6 +71,10 @@ var (
 	// ErrServerShuttingDown indicates that the server is in the process of
 	// gracefully exiting.
 	ErrServerShuttingDown = errors.New("server is shutting down")
+
+	// validColorRegexp is a regexp that lets you check if a particular
+	// color string matches the standard hex color format #RRGGBB.
+	validColorRegexp = regexp.MustCompile("^#[A-Fa-f0-9]{6}$")
 )
 
 // server is the main server of the Lightning Network Daemon. The server houses
@@ -2821,8 +2826,10 @@ func (s *server) Peers() []*peer {
 // form "#RRGGBB", parses the hex color values, and returns a color.RGBA
 // struct of the same color.
 func parseHexColor(colorStr string) (color.RGBA, error) {
-	if len(colorStr) != 7 || colorStr[0] != '#' {
-		return color.RGBA{}, errors.New("Color must be in format #RRGGBB")
+	// Check if the hex color string is a valid color representation.
+	if !validColorRegexp.MatchString(colorStr) {
+		return color.RGBA{}, errors.New("Color must be specified " +
+			"using a hexadecimal value in the form #RRGGBB")
 	}
 
 	// Decode the hex color string to bytes.

server_test.go

@@ -5,32 +5,39 @@ package main
 import "testing"
 
 func TestParseHexColor(t *testing.T) {
-	empty := ""
-	color, err := parseHexColor(empty)
-	if err == nil {
-		t.Fatalf("Empty color string should return error, but did not")
+	var colorTestCases = []struct {
+		test  string
+		valid bool // If valid format
+		R     byte
+		G     byte
+		B     byte
+	}{
+		{"#123", false, 0, 0, 0},
+		{"#1234567", false, 0, 0, 0},
+		{"$123456", false, 0, 0, 0},
+		{"#12345+", false, 0, 0, 0},
+		{"#fFGG00", false, 0, 0, 0},
+		{"", false, 0, 0, 0},
+		{"#123456", true, 0x12, 0x34, 0x56},
+		{"#C0FfeE", true, 0xc0, 0xff, 0xee},
 	}
 
-	tooLong := "#1234567"
-	color, err = parseHexColor(tooLong)
-	if err == nil {
-		t.Fatalf("Invalid color string %s should return error, but did not",
-			tooLong)
-	}
+	// Perform the table driven tests.
+	for _, ct := range colorTestCases {
 
-	invalidFormat := "$123456"
-	color, err = parseHexColor(invalidFormat)
-	if err == nil {
-		t.Fatalf("Invalid color string %s should return error, but did not",
-			invalidFormat)
-	}
+		color, err := parseHexColor(ct.test)
+		if !ct.valid && err == nil {
+			t.Fatalf("Invalid color string: %s, should return "+
+				"error, but did not", ct.test)
+		}
 
-	valid := "#C0FfeE"
-	color, err = parseHexColor(valid)
-	if err != nil {
-		t.Fatalf("Color %s valid to parse: %s", valid, err)
-	}
-	if color.R != 0xc0 || color.G != 0xff || color.B != 0xee {
-		t.Fatalf("Color %s incorrectly parsed as %v", valid, color)
+		if ct.valid && err != nil {
+			t.Fatalf("Color %s valid to parse: %s", ct.test, err)
+		}
+
+		// Ensure that the string to hex decoding is working properly.
+		if color.R != ct.R || color.G != ct.G || color.B != ct.B {
+			t.Fatalf("Color %s incorrectly parsed as %v", ct.test, color)
+		}
 	}
 }