Require braces to be present for environment variable expansion

CHANGELOG.asciidoc

@@ -18,6 +18,7 @@ https://github.com/elastic/beats/compare/v1.2.0...master[Check the HEAD diff]
 - Move event preprocessor applying GeoIP to packetbeat {pull}772[772]
 - Add include_fields and drop_fields as part of generic filtering {pull}1120[1120]
 - The method signature of HandleFlags() was changed to allow returning an error {pull}1249[1249]
+- Require braces for environment variable expansion in config files {pull}1304[1304]
 
 *Packetbeat*
 - Rename output fields in the dns package. Former flag `recursion_allowed` becomes `recursion_available`. {pull}803[803]

libbeat/cfgfile/cfgfile.go

@@ -31,7 +31,9 @@ func ChangeDefaultCfgfileFlag(beatName string) error {
 
 	path, err := filepath.Abs(filepath.Dir(os.Args[0]))
 	if err != nil {
-		return fmt.Errorf("Failed to set default config file location because the absolute path to %s could not be obtained. %v", os.Args[0], err)
+		return fmt.Errorf("Failed to set default config file location because "+
+			"the absolute path to %s could not be obtained. %v",
+			os.Args[0], err)
 	}
 
 	cliflag.DefValue = filepath.Join(path, beatName+".yml")
@@ -80,12 +82,12 @@ func IsTestConfig() bool {
 	return *testConfig
 }
 
-// expandEnv replaces ${var} or $var in config according to the values of the
-// current environment variables. The replacement is case-sensitive. References
-// to undefined variables are replaced by the empty string. A default value
-// can be given by using the form ${var:default value}.
+// expandEnv replaces ${var} in config according to the values of the current
+// environment variables. The replacement is case-sensitive. References to
+// undefined variables are replaced by the empty string. A default value can be
+// given by using the form ${var:default value}.
 func expandEnv(config []byte) []byte {
-	return []byte(os.Expand(string(config), func(key string) string {
+	return []byte(expand(string(config), func(key string) string {
 		keyAndDefault := strings.SplitN(key, ":", 2)
 		key = keyAndDefault[0]
 
@@ -103,3 +105,65 @@ func expandEnv(config []byte) []byte {
 		return v
 	}))
 }
+
+// The following methods were copied from the os package of the stdlib. The
+// expand method was modified to only expand variables defined with braces and
+// ignore $var.
+
+// Expand replaces ${var} in the string based on the mapping function.
+func expand(s string, mapping func(string) string) string {
+	buf := make([]byte, 0, 2*len(s))
+	// ${} is all ASCII, so bytes are fine for this operation.
+	i := 0
+	for j := 0; j < len(s); j++ {
+		if s[j] == '$' && j+2 < len(s) && s[j+1] == '{' {
+			buf = append(buf, s[i:j]...)
+			name, w := getShellName(s[j+1:])
+			buf = append(buf, mapping(name)...)
+			j += w
+			i = j + 1
+		}
+	}
+	return string(buf) + s[i:]
+}
+
+// isShellSpecialVar reports whether the character identifies a special
+// shell variable such as $*.
+func isShellSpecialVar(c uint8) bool {
+	switch c {
+	case '*', '#', '$', '@', '!', '?', '0', '1', '2', '3', '4', '5', '6', '7', '8', '9':
+		return true
+	}
+	return false
+}
+
+// isAlphaNum reports whether the byte is an ASCII letter, number, or underscore
+func isAlphaNum(c uint8) bool {
+	return c == '_' || '0' <= c && c <= '9' || 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z'
+}
+
+// getShellName returns the name that begins the string and the number of bytes
+// consumed to extract it.  If the name is enclosed in {}, it's part of a ${}
+// expansion and two more bytes are needed than the length of the name.
+func getShellName(s string) (string, int) {
+	switch {
+	case s[0] == '{':
+		if len(s) > 2 && isShellSpecialVar(s[1]) && s[2] == '}' {
+			return s[1:2], 3
+		}
+		// Scan to closing brace
+		for i := 1; i < len(s); i++ {
+			if s[i] == '}' {
+				return s[1:i], i + 1
+			}
+		}
+		return "", 1 // Bad syntax; just eat the brace.
+	case isShellSpecialVar(s[0]):
+		return s[0:1], 1
+	}
+	// Scan alphanumerics.
+	var i int
+	for i = 0; i < len(s) && isAlphaNum(s[i]); i++ {
+	}
+	return s[:i], i
+}

libbeat/cfgfile/cfgfile_test.go

@@ -46,21 +46,31 @@ func TestExpandEnv(t *testing.T) {
 		in  string
 		out string
 	}{
-		// Environment variables can be specified as ${env} or $env.
-		{"x$y", "xy"},
-		{"x${y}", "xy"},
+		// Environment variables can be specified as ${env} only.
+		{"${y}", "y"},
+		{"$y", "$y"},
 
-		// Environment variables are case-sensitive. Neither are replaced.
-		{"x$Y", "x"},
-		{"x${Y}", "x"},
+		// Environment variables are case-sensitive.
+		{"${Y}", ""},
 
-		// Defaults can only be specified when using braces.
+		// Defaults can be specified.
 		{"x${Z:D}", "xD"},
 		{"x${Z:A B C D}", "xA B C D"}, // Spaces are allowed in the default.
 		{"x${Z:}", "x"},
 
-		// Defaults don't work unless braces are used.
-		{"x$y:D", "xy:D"},
+		// Un-matched braces are swallowed by the Go os.Expand function.
+		{"x${Y ${Z:Z}", "xZ"},
+
+		// Special environment variables are not replaced.
+		{"$*", "$*"},
+		{"${*}", ""},
+		{"$@", "$@"},
+		{"${@}", ""},
+		{"$1", "$1"},
+		{"${1}", ""},
+
+		{"", ""},
+		{"$$", "$$"},
 	}
 
 	for _, test := range tests {