Merge pull request #285 from GraveRaven/validVarName

journal: fix validation of key names

Author: Luca Bruno

journal/journal.go

@@ -119,8 +119,8 @@ func Print(priority Priority, format string, a ...interface{}) error {
 }
 
 func appendVariable(w io.Writer, name, value string) {
-	if !validVarName(name) {
-		journalError("variable name contains invalid character, ignoring")
+	if err := validVarName(name); err != nil {
+		journalError(err.Error())
 	}
 	if strings.ContainsRune(value, '\n') {
 		/* When the value contains a newline, we write:
@@ -137,16 +137,23 @@ func appendVariable(w io.Writer, name, value string) {
 	}
 }
 
-func validVarName(name string) bool {
-	/* The variable name must be in uppercase and consist only of characters,
-	 * numbers and underscores, and may not begin with an underscore. (from the docs)
-	 */
+// validVarName validates a variable name to make sure it journald will accept it.
+// The variable name must be in uppercase and consist only of characters,
+// numbers and underscores, and may not begin with an underscore. (from the docs)
+// https://www.freedesktop.org/software/systemd/man/sd_journal_print.html
+func validVarName(name string) error {
+	if name == "" {
+		return errors.New("Empty variable name")
+	} else if name[0] == '_' {
+		return errors.New("Variable name begins with an underscore")
+	}
 
-	valid := name[0] != '_'
 	for _, c := range name {
-		valid = valid && ('A' <= c && c <= 'Z') || ('0' <= c && c <= '9') || c == '_'
+		if !(('A' <= c && c <= 'Z') || ('0' <= c && c <= '9') || c == '_') {
+			return errors.New("Variable name contains invalid characters")
+		}
 	}
-	return valid
+	return nil
 }
 
 func isSocketSpaceError(err error) bool {

journal/journal_test.go

@@ -22,27 +22,30 @@ import (
 	"testing"
 )
 
-func TestValidaVarName(t *testing.T) {
-	testCases := []struct {
-		testcase string
-		valid    bool
-	}{
-		{
-			"TEST",
-			true,
-		},
-		{
-			"test",
-			false,
-		},
+func TestValidVarName(t *testing.T) {
+	validTestCases := []string{
+		"TEST",
+		"TE_ST",
+		"TEST_",
+		"0TEST0",
+	}
+	invalidTestCases := []string{
+		"test",
+		"_TEST",
+		"",
 	}
 
-	for _, tt := range testCases {
-		valid := validVarName(tt.testcase)
-		if valid != tt.valid {
-			t.Fatalf("expected %t, got %t", tt.valid, valid)
+	for _, tt := range validTestCases {
+		if err := validVarName(tt); err != nil {
+			t.Fatalf("\"%s\" should be a valid variable", tt)
+		}
+	}
+	for _, tt := range invalidTestCases {
+		if err := validVarName(tt); err == nil {
+			t.Fatalf("\"%s\" should be an invalid variable", tt)
 		}
 	}
+
 }
 
 func TestJournalSend(t *testing.T) {