caddyfile: fix replacing variables on imported files (#2970)

* fix replacing variables on imported files

* refactored replaceEnvVars to ensure it is always called

* Use byte slices for easier use

Co-authored-by: Matt Holt <mholt@users.noreply.github.com>

caddyconfig/caddyfile/dispenser_test.go

@@ -308,7 +308,7 @@ func TestDispenser_ArgErr_Err(t *testing.T) {
 }
 
 func newTestDispenser(input string) *Dispenser {
-	tokens, err := allTokens("Testfile", strings.NewReader(input))
+	tokens, err := allTokens("Testfile", []byte(input))
 	if err != nil && err != io.EOF {
 		log.Fatalf("getting all tokens from input: %v", err)
 	}

caddyconfig/caddyfile/parse.go

@@ -16,7 +16,7 @@ package caddyfile
 
 import (
 	"bytes"
-	"io"
+	"io/ioutil"
 	"log"
 	"os"
 	"path/filepath"
@@ -33,8 +33,7 @@ import (
 // Environment variables in {$ENVIRONMENT_VARIABLE} notation
 // will be replaced before parsing begins.
 func Parse(filename string, input []byte) ([]ServerBlock, error) {
-	input = replaceEnvVars(input)
+	tokens, err := allTokens(filename, input)
-	tokens, err := allTokens(filename, bytes.NewReader(input))
 	if err != nil {
 		return nil, err
 	}
@@ -43,7 +42,7 @@ func Parse(filename string, input []byte) ([]ServerBlock, error) {
 }
 
 // replaceEnvVars replaces all occurrences of environment variables.
-func replaceEnvVars(input []byte) []byte {
+func replaceEnvVars(input []byte) ([]byte, error) {
 	var offset int
 	for {
 		begin := bytes.Index(input[offset:], spanOpen)
@@ -74,15 +73,19 @@ func replaceEnvVars(input []byte) []byte {
 		// continue at the end of the replacement
 		offset = begin + len(envVarValue)
 	}
-	return input
+	return input, nil
 }
 
 // allTokens lexes the entire input, but does not parse it.
 // It returns all the tokens from the input, unstructured
 // and in order.
-func allTokens(filename string, input io.Reader) ([]Token, error) {
+func allTokens(filename string, input []byte) ([]Token, error) {
+	input, err := replaceEnvVars(input)
+	if err != nil {
+		return nil, err
+	}
 	l := new(lexer)
-	err := l.load(input)
+	err = l.load(bytes.NewReader(input))
 	if err != nil {
 		return nil, err
 	}
@@ -368,7 +371,12 @@ func (p *parser) doSingleImport(importFile string) ([]Token, error) {
 		return nil, p.Errf("Could not import %s: is a directory", importFile)
 	}
 
-	importedTokens, err := allTokens(importFile, file)
+	input, err := ioutil.ReadAll(file)
+	if err != nil {
+		return nil, p.Errf("Could not read imported file %s: %v", importFile, err)
+	}
+
+	importedTokens, err := allTokens(importFile, input)
 	if err != nil {
 		return nil, p.Errf("Could not read tokens while importing %s: %v", importFile, err)
 	}

caddyconfig/caddyfile/parse_test.go

@@ -19,12 +19,11 @@ import (
 	"io/ioutil"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 )
 
 func TestAllTokens(t *testing.T) {
-	input := strings.NewReader("a b c\nd e")
+	input := []byte("a b c\nd e")
 	expected := []string{"a", "b", "c", "d", "e"}
 	tokens, err := allTokens("TestAllTokens", input)
 
@@ -545,7 +544,10 @@ func TestEnvironmentReplacement(t *testing.T) {
 			expect: "}{$",
 		},
 	} {
-		actual := replaceEnvVars([]byte(test.input))
+		actual, err := replaceEnvVars([]byte(test.input))
+		if err != nil {
+			t.Fatal(err)
+		}
 		if !bytes.Equal(actual, []byte(test.expect)) {
 			t.Errorf("Test %d: Expected: '%s' but got '%s'", i, test.expect, actual)
 		}