Fix -expand-headers Help String

Also fix all other PR feedback.

cmd/grpcurl/grpcurl.go

@@ -59,12 +59,13 @@ var (
 	rpcHeaders    multiString
 	reflHeaders   multiString
 	expandHeaders = flags.Bool("expand-headers", false, prettify(`
-		If set any environmental variables contained contained in any additional, rpc,
-		or reflection header string will be substituted for by the value of the environmental
-		variable. For instance, for the header 'key: ${VALUE}' where VALUE="foo"
-		will be expanded to 'key: foo'. Any undefined environmental variables
-		will throw an error. Note that verbose mode shows that the headers are
-		being expanded to. Escaping '${' is not supported.`))
+		If set, headers may use '${NAME}' syntax to reference environment variables.
+		These will be expanded to the actual environment variable value before
+		sending to the server. For example, if there is an environment variable
+		defined like FOO=bar, then a header of 'key: ${FOO}' would expand to 'key: bar'.
+		This applies to -H, -rpc-header, and -reflect-header options. No other
+		expansion/escaping is performed. This can be used to supply
+		credentials/secrets without having to put them in command-line arguments.`))
 	authority = flags.String("authority", "", prettify(`
 		Value of :authority pseudo-header to be use with underlying HTTP/2
 		requests. It defaults to the given address.`))
@@ -324,15 +325,15 @@ func main() {
 		var err error
 		addlHeaders, err = grpcurl.ExpandHeaders(addlHeaders)
 		if err != nil {
-			fail(err, "Failed to expand additional headers, missing environmental variable")
+			fail(err, "Failed to expand additional headers")
 		}
 		rpcHeaders, err = grpcurl.ExpandHeaders(rpcHeaders)
 		if err != nil {
-			fail(err, "Failed to expand rpc headers, missing environmental variable")
+			fail(err, "Failed to expand rpc headers")
 		}
 		reflHeaders, err = grpcurl.ExpandHeaders(reflHeaders)
 		if err != nil {
-			fail(err, "Failed to expand reflection headers, missing environmental variable")
+			fail(err, "Failed to expand reflection headers")
 		}
 	}
 

grpcurl.go

@@ -165,30 +165,30 @@ func MetadataFromHeaders(headers []string) metadata.MD {
 
 var envVarRegex = regexp.MustCompile(`\${\w+}`)
 
-// ExpandHeaders expands environmental variables contained in the header string.
-// If no corresponding environmental variable is found an error is thrown.
+// ExpandHeaders expands environment variables contained in the header string.
+// If no corresponding environment variable is found an error is returned.
 // TODO: Add escaping for `${`
 func ExpandHeaders(headers []string) ([]string, error) {
 	expandedHeaders := make([]string, len(headers))
 	for idx, header := range headers {
-		if header != "" {
-			results := envVarRegex.FindAllString(header, -1)
-			if results != nil {
-				expandedHeader := header
-				for _, result := range results {
-					envVarName := result[2 : len(result)-1]
-					envVarValue := os.Getenv(envVarName)
-					replacementValue := envVarValue
-					if len(envVarValue) == 0 {
-						return nil, fmt.Errorf("environmental variable '%s' is not set", envVarName)
-					}
-					expandedHeader = strings.Replace(expandedHeader, result, replacementValue, -1)
-				}
-				expandedHeaders[idx] = expandedHeader
-			} else {
-				expandedHeaders[idx] = headers[idx]
-			}
+		if header == "" {
+			continue
 		}
+		results := envVarRegex.FindAllString(header, -1)
+		if len(results) == 0 {
+			expandedHeaders[idx] = headers[idx]
+			continue
+		}
+		expandedHeader := header
+		for _, result := range results {
+			envVarName := result[2 : len(result)-1] // strip leading `${` and trailing `}`
+			envVarValue, ok := os.LookupEnv(envVarName)
+			if !ok {
+				return nil, fmt.Errorf("header %q refers to missing environment variable %q", header, envVarName)
+			}
+			expandedHeader = strings.Replace(expandedHeader, result, envVarValue, -1)
+		}
+		expandedHeaders[idx] = expandedHeader
 	}
 	return expandedHeaders, nil
 }

grpcurl_test.go

@@ -301,13 +301,14 @@ func TestGetAllFiles(t *testing.T) {
 }
 
 func TestExpandHeaders(t *testing.T) {
-	inHeaders := []string{"", "key1: ${value}", "key2: bar", "key3: ${woo", "key4: woo}", "key5: ${TEST}",
-		"key6: ${TEST_VAR}", "${TEST}: ${TEST_VAR}"}
+	inHeaders := []string{"key1: ${value}", "key2: bar", "key3: ${woo", "key4: woo}", "key5: ${TEST}",
+		"key6: ${TEST_VAR}", "${TEST}: ${TEST_VAR}", "key8: ${EMPTY}"}
 	os.Setenv("value", "value")
 	os.Setenv("TEST", "value5")
 	os.Setenv("TEST_VAR", "value6")
-	expectedHeaders := map[string]bool{"": true, "key1: value": true, "key2: bar": true, "key3: ${woo": true, "key4: woo}": true,
-		"key5: value5": true, "key6: value6": true, "value5: value6": true}
+	os.Setenv("EMPTY", "")
+	expectedHeaders := map[string]bool{"key1: value": true, "key2: bar": true, "key3: ${woo": true, "key4: woo}": true,
+		"key5: value5": true, "key6: value6": true, "value5: value6": true, "key8: ": true}
 
 	outHeaders, err := ExpandHeaders(inHeaders)
 	if err != nil {
@@ -315,14 +316,14 @@ func TestExpandHeaders(t *testing.T) {
 	}
 	for _, expandedHeader := range outHeaders {
 		if _, ok := expectedHeaders[expandedHeader]; !ok {
-			t.Errorf("The ExpandHeaders function has generated an unexpected header. Recieved unexpected header %s", expandedHeader)
+			t.Errorf("The ExpandHeaders function has returned an unexpected header. Received unexpected header %s", expandedHeader)
 		}
 	}
 
 	badHeaders := []string{"key: ${DNE}"}
 	_, err = ExpandHeaders(badHeaders)
 	if err == nil {
-		t.Errorf("The ExpandHeaders function should generate an error for missing environmental variables %q", badHeaders)
+		t.Errorf("The ExpandHeaders function should return an error for missing environment variables %q", badHeaders)
 	}
 }