From 9490fb6b656eaa92a0182a89905b6afa7945a889 Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 11:19:31 +1100 Subject: [PATCH 01/10] Add tests to cover the regression reported in #204 --- fixtures/plain.env | 3 ++- godotenv_test.go | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/fixtures/plain.env b/fixtures/plain.env index 43f7e44..e033366 100644 --- a/fixtures/plain.env +++ b/fixtures/plain.env @@ -4,4 +4,5 @@ OPTION_C= 3 OPTION_D =4 OPTION_E = 5 OPTION_F = -OPTION_G= \ No newline at end of file +OPTION_G= +OPTION_H=1 2 \ No newline at end of file diff --git a/godotenv_test.go b/godotenv_test.go index 017d6b6..ea91f8e 100644 --- a/godotenv_test.go +++ b/godotenv_test.go @@ -80,6 +80,7 @@ func TestReadPlainEnv(t *testing.T) { "OPTION_E": "5", "OPTION_F": "", "OPTION_G": "", + "OPTION_H": "1 2", } envMap, err := Read(envFileName) @@ -153,6 +154,7 @@ func TestLoadPlainEnv(t *testing.T) { "OPTION_C": "3", "OPTION_D": "4", "OPTION_E": "5", + "OPTION_H": "1 2", } loadEnvAndCompareValues(t, Load, envFileName, expectedValues, noopPresets) @@ -369,6 +371,9 @@ func TestParsing(t *testing.T) { // expect(env('foo=bar ')).to eql('foo' => 'bar') # not 'bar ' parseAndCompare(t, "FOO=bar ", "FOO", "bar") + // unquoted internal whitespace is preserved + parseAndCompare(t, `KEY=value value`, "KEY", "value value") + // it 'ignores inline comments' do // expect(env("foo=bar # this is foo")).to eql('foo' => 'bar') parseAndCompare(t, "FOO=bar # this is foo", "FOO", "bar") @@ -394,7 +399,7 @@ func TestParsing(t *testing.T) { parseAndCompare(t, `KEY="`, "KEY", "\"") parseAndCompare(t, `KEY="value`, "KEY", "\"value") - // leading whitespace should be ignored + // unquoted whitespace around keys should be ignored parseAndCompare(t, " KEY =value", "KEY", "value") parseAndCompare(t, " KEY=value", "KEY", "value") parseAndCompare(t, "\tKEY=value", "KEY", "value") From 9deb6f6b37b939f35adccba37a1644f8eeb2d065 Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 11:41:39 +1100 Subject: [PATCH 02/10] Add a comment on regex for clarity --- godotenv.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/godotenv.go b/godotenv.go index 3d312f3..ae358ca 100644 --- a/godotenv.go +++ b/godotenv.go @@ -215,6 +215,7 @@ func readFile(filename string) (envMap map[string]string, err error) { return Parse(file) } +// removes "export " if present and trims whitespace var exportRegex = regexp.MustCompile(`^\s*(?:export\s+)?(.*?)\s*$`) func parseLine(line string, envMap map[string]string) (key string, value string, err error) { @@ -281,7 +282,6 @@ var ( ) func parseValue(value string, envMap map[string]string) string { - // trim value = strings.Trim(value, " ") From 22da1c926cf1fa30745e921fa6f5d41c9d1a1828 Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 11:42:06 +1100 Subject: [PATCH 03/10] Remove some old code that wasn't doing anything --- godotenv.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/godotenv.go b/godotenv.go index ae358ca..907abe5 100644 --- a/godotenv.go +++ b/godotenv.go @@ -263,11 +263,7 @@ func parseLine(line string, envMap map[string]string) (key string, value string, // Parse the key key = splitString[0] - key = strings.TrimPrefix(key, "export") - - key = strings.TrimSpace(key) - - key = exportRegex.ReplaceAllString(splitString[0], "$1") + key = exportRegex.ReplaceAllString(key, "$1") // Parse the value value = parseValue(splitString[1], envMap) From 25b65cf4876e0dc4c202938735f91b778a3e3092 Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 15:44:00 +1100 Subject: [PATCH 04/10] Push _all_ parse code into the parser and get tests calling live code --- godotenv.go | 122 ----------------------------------------------- godotenv_test.go | 10 ++-- parser.go | 23 +++++++++ 3 files changed, 28 insertions(+), 127 deletions(-) diff --git a/godotenv.go b/godotenv.go index 907abe5..61b0ebb 100644 --- a/godotenv.go +++ b/godotenv.go @@ -15,12 +15,10 @@ package godotenv import ( "bytes" - "errors" "fmt" "io" "os" "os/exec" - "regexp" "sort" "strconv" "strings" @@ -215,126 +213,6 @@ func readFile(filename string) (envMap map[string]string, err error) { return Parse(file) } -// removes "export " if present and trims whitespace -var exportRegex = regexp.MustCompile(`^\s*(?:export\s+)?(.*?)\s*$`) - -func parseLine(line string, envMap map[string]string) (key string, value string, err error) { - if len(line) == 0 { - err = errors.New("zero length string") - return - } - - // ditch the comments (but keep quoted hashes) - if strings.Contains(line, "#") { - segmentsBetweenHashes := strings.Split(line, "#") - quotesAreOpen := false - var segmentsToKeep []string - for _, segment := range segmentsBetweenHashes { - if strings.Count(segment, "\"") == 1 || strings.Count(segment, "'") == 1 { - if quotesAreOpen { - quotesAreOpen = false - segmentsToKeep = append(segmentsToKeep, segment) - } else { - quotesAreOpen = true - } - } - - if len(segmentsToKeep) == 0 || quotesAreOpen { - segmentsToKeep = append(segmentsToKeep, segment) - } - } - - line = strings.Join(segmentsToKeep, "#") - } - - firstEquals := strings.Index(line, "=") - firstColon := strings.Index(line, ":") - splitString := strings.SplitN(line, "=", 2) - if firstColon != -1 && (firstColon < firstEquals || firstEquals == -1) { - //this is a yaml-style line - splitString = strings.SplitN(line, ":", 2) - } - - if len(splitString) != 2 { - err = errors.New("can't separate key from value") - return - } - - // Parse the key - key = splitString[0] - - key = exportRegex.ReplaceAllString(key, "$1") - - // Parse the value - value = parseValue(splitString[1], envMap) - return -} - -var ( - singleQuotesRegex = regexp.MustCompile(`\A'(.*)'\z`) - doubleQuotesRegex = regexp.MustCompile(`\A"(.*)"\z`) - escapeRegex = regexp.MustCompile(`\\.`) - unescapeCharsRegex = regexp.MustCompile(`\\([^$])`) -) - -func parseValue(value string, envMap map[string]string) string { - // trim - value = strings.Trim(value, " ") - - // check if we've got quoted values or possible escapes - if len(value) > 1 { - singleQuotes := singleQuotesRegex.FindStringSubmatch(value) - - doubleQuotes := doubleQuotesRegex.FindStringSubmatch(value) - - if singleQuotes != nil || doubleQuotes != nil { - // pull the quotes off the edges - value = value[1 : len(value)-1] - } - - if doubleQuotes != nil { - // expand newlines - value = escapeRegex.ReplaceAllStringFunc(value, func(match string) string { - c := strings.TrimPrefix(match, `\`) - switch c { - case "n": - return "\n" - case "r": - return "\r" - default: - return match - } - }) - // unescape characters - value = unescapeCharsRegex.ReplaceAllString(value, "$1") - } - - if singleQuotes == nil { - value = expandVariables(value, envMap) - } - } - - return value -} - -var expandVarRegex = regexp.MustCompile(`(\\)?(\$)(\()?\{?([A-Z0-9_]+)?\}?`) - -func expandVariables(v string, m map[string]string) string { - return expandVarRegex.ReplaceAllStringFunc(v, func(s string) string { - submatch := expandVarRegex.FindStringSubmatch(s) - - if submatch == nil { - return s - } - if submatch[1] == "\\" || submatch[2] == "(" { - return submatch[0][1:] - } else if submatch[4] != "" { - return m[submatch[4]] - } - return s - }) -} - func doubleQuoteEscape(line string) string { for _, c := range doubleQuoteSpecialChars { toReplace := "\\" + string(c) diff --git a/godotenv_test.go b/godotenv_test.go index ea91f8e..5a3fc77 100644 --- a/godotenv_test.go +++ b/godotenv_test.go @@ -12,9 +12,10 @@ import ( var noopPresets = make(map[string]string) func parseAndCompare(t *testing.T, rawEnvLine string, expectedKey string, expectedValue string) { - key, value, _ := parseLine(rawEnvLine, noopPresets) - if key != expectedKey || value != expectedValue { - t.Errorf("Expected '%v' to parse as '%v' => '%v', got '%v' => '%v' instead", rawEnvLine, expectedKey, expectedValue, key, value) + result, _ := Unmarshal(rawEnvLine) + + if result[expectedKey] != expectedValue { + t.Errorf("Expected '%v' to parse as '%v' => '%v', got %q instead", rawEnvLine, expectedKey, expectedValue, result) } } @@ -274,7 +275,6 @@ func TestExpanding(t *testing.T) { } }) } - } func TestVariableStringValueSeparator(t *testing.T) { @@ -407,7 +407,7 @@ func TestParsing(t *testing.T) { // it 'throws an error if line format is incorrect' do // expect{env('lol$wut')}.to raise_error(Dotenv::FormatError) badlyFormattedLine := "lol$wut" - _, _, err := parseLine(badlyFormattedLine, noopPresets) + _, err := Unmarshal(badlyFormattedLine) if err == nil { t.Errorf("Expected \"%v\" to return error, but it didn't", badlyFormattedLine) } diff --git a/parser.go b/parser.go index 196418c..2f97548 100644 --- a/parser.go +++ b/parser.go @@ -4,6 +4,7 @@ import ( "bytes" "errors" "fmt" + "regexp" "strings" "unicode" ) @@ -205,3 +206,25 @@ func isSpace(r rune) bool { } return false } + +var ( + escapeRegex = regexp.MustCompile(`\\.`) + expandVarRegex = regexp.MustCompile(`(\\)?(\$)(\()?\{?([A-Z0-9_]+)?\}?`) + unescapeCharsRegex = regexp.MustCompile(`\\([^$])`) +) + +func expandVariables(v string, m map[string]string) string { + return expandVarRegex.ReplaceAllStringFunc(v, func(s string) string { + submatch := expandVarRegex.FindStringSubmatch(s) + + if submatch == nil { + return s + } + if submatch[1] == "\\" || submatch[2] == "(" { + return submatch[0][1:] + } else if submatch[4] != "" { + return m[submatch[4]] + } + return s + }) +} From 8469255f5e8d092b6f5c62ead6b05d113d30636e Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 15:44:25 +1100 Subject: [PATCH 05/10] Add some newline specific tests --- godotenv_test.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/godotenv_test.go b/godotenv_test.go index 5a3fc77..ac1c7df 100644 --- a/godotenv_test.go +++ b/godotenv_test.go @@ -525,3 +525,39 @@ func TestRoundtrip(t *testing.T) { } } + +func TestTrailingNewlines(t *testing.T) { + cases := map[string]struct { + input string + key string + value string + }{ + "Simple value without trailing newline": { + input: "KEY=value", + key: "KEY", + value: "value", + }, + "Value with internal whitespace without trailing newline": { + input: "KEY=value value", + key: "KEY", + value: "value value", + }, + "Value with internal whitespace with trailing newline": { + input: "KEY=value value\n", + key: "KEY", + value: "value value", + }, + } + + for n, c := range cases { + t.Run(n, func(t *testing.T) { + result, err := Unmarshal(c.input) + if err != nil { + t.Errorf("Unexpected error:\t%q", err) + } + if result[c.key] != c.value { + t.Errorf("Expected:\t %q/%q\nGot:\t %#v", c.key, c.value, result) + } + }) + } +} From 4736d270e51ede83dabe603dd93da542088117a3 Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 16:24:29 +1100 Subject: [PATCH 06/10] Add some YAML tests for the newline/space split bug --- godotenv_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/godotenv_test.go b/godotenv_test.go index ac1c7df..80ce148 100644 --- a/godotenv_test.go +++ b/godotenv_test.go @@ -547,16 +547,26 @@ func TestTrailingNewlines(t *testing.T) { key: "KEY", value: "value value", }, + "YAML style - value with internal whitespace without trailing newline": { + input: "KEY: value value", + key: "KEY", + value: "value value", + }, + "YAML style - value with internal whitespace with trailing newline": { + input: "KEY: value value\n", + key: "KEY", + value: "value value", + }, } for n, c := range cases { t.Run(n, func(t *testing.T) { result, err := Unmarshal(c.input) if err != nil { - t.Errorf("Unexpected error:\t%q", err) + t.Errorf("Input: %q Unexpected error:\t%q", c.input, err) } if result[c.key] != c.value { - t.Errorf("Expected:\t %q/%q\nGot:\t %#v", c.key, c.value, result) + t.Errorf("Input %q Expected:\t %q/%q\nGot:\t %q", c.input, c.key, c.value, result) } }) } From fe517b8f42859a5dc8f9e0970e177067de667b61 Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 20:30:36 +1100 Subject: [PATCH 07/10] Fix incorrect terminating of lines on whitespace --- parser.go | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/parser.go b/parser.go index 2f97548..7c47e38 100644 --- a/parser.go +++ b/parser.go @@ -114,13 +114,32 @@ loop: func extractVarValue(src []byte, vars map[string]string) (value string, rest []byte, err error) { quote, hasPrefix := hasQuotePrefix(src) if !hasPrefix { - // unquoted value - read until whitespace - end := bytes.IndexFunc(src, unicode.IsSpace) - if end == -1 { + // unquoted value - read until end of line + endOfLine := bytes.IndexFunc(src, isLineEnd) + + // Hit EOF without a trailing newline + if endOfLine == -1 { return expandVariables(string(src), vars), nil, nil } - return expandVariables(string(src[0:end]), vars), src[end:], nil + // Convert line to rune away to do accurate countback of runes + line := []rune(string(src[0:endOfLine])) + + // Assume end of line is end of var + endOfVar := len(line) + + // Work backwards to check if the line ends in whitespace then + // a comment (ie asdasd # some comment) + for i := endOfVar - 1; i >= 0; i-- { + if line[i] == charComment && i > 0 { + if isSpace(line[i-1]) { + endOfVar = i - 1 + break + } + } + } + + return expandVariables(string(line[0:endOfVar]), vars), src[endOfLine:], nil } // lookup quoted string terminator @@ -207,6 +226,13 @@ func isSpace(r rune) bool { return false } +func isLineEnd(r rune) bool { + if r == '\n' || r == '\r' { + return true + } + return false +} + var ( escapeRegex = regexp.MustCompile(`\\.`) expandVarRegex = regexp.MustCompile(`(\\)?(\$)(\()?\{?([A-Z0-9_]+)?\}?`) From f568c991a785dd83b25c1d6a8776ae44f2caedad Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 21:55:16 +1100 Subject: [PATCH 08/10] Fix most of the parser regressions --- godotenv_test.go | 6 +++++- parser.go | 29 +++++++++++++++++++++++++---- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/godotenv_test.go b/godotenv_test.go index 80ce148..8520a24 100644 --- a/godotenv_test.go +++ b/godotenv_test.go @@ -12,8 +12,12 @@ import ( var noopPresets = make(map[string]string) func parseAndCompare(t *testing.T, rawEnvLine string, expectedKey string, expectedValue string) { - result, _ := Unmarshal(rawEnvLine) + result, err := Unmarshal(rawEnvLine) + if err != nil { + t.Errorf("Expected %q to parse as %q: %q, errored %q", rawEnvLine, expectedKey, expectedValue, err) + return + } if result[expectedKey] != expectedValue { t.Errorf("Expected '%v' to parse as '%v' => '%v', got %q instead", rawEnvLine, expectedKey, expectedValue, result) } diff --git a/parser.go b/parser.go index 7c47e38..f2cbddf 100644 --- a/parser.go +++ b/parser.go @@ -70,7 +70,19 @@ func getStatementStart(src []byte) []byte { // locateKeyName locates and parses key name and returns rest of slice func locateKeyName(src []byte) (key string, cutset []byte, err error) { // trim "export" and space at beginning - src = bytes.TrimLeftFunc(bytes.TrimPrefix(src, []byte(exportPrefix)), isSpace) + // src = bytes.TrimLeftFunc(bytes.TrimPrefix(src, []byte(exportPrefix)), isSpace) + src = bytes.TrimLeftFunc(src, isSpace) + // exportPrefixEnd := len(exportPrefix) - 1 + // if len(src) > exportPrefixEnd+2 && bytes.Equal(src[0:exportPrefixEnd], []byte(exportPrefix)) && bytes.IndexFunc(src[exportPrefixEnd:], isSpace) == 0 { + // src = src[exportPrefixEnd:] + // fmt.Println(src) + // } + if bytes.HasPrefix(src, []byte(exportPrefix)) { + trimmed := bytes.TrimPrefix(src, []byte(exportPrefix)) + if bytes.IndexFunc(trimmed, isSpace) == 0 { + src = bytes.TrimLeftFunc(trimmed, isSpace) + } + } // locate key name end and validate it in single loop offset := 0 @@ -119,7 +131,11 @@ func extractVarValue(src []byte, vars map[string]string) (value string, rest []b // Hit EOF without a trailing newline if endOfLine == -1 { - return expandVariables(string(src), vars), nil, nil + endOfLine = len(src) + + if endOfLine == 0 { + return "", nil, nil + } } // Convert line to rune away to do accurate countback of runes @@ -127,19 +143,24 @@ func extractVarValue(src []byte, vars map[string]string) (value string, rest []b // Assume end of line is end of var endOfVar := len(line) + if endOfVar == 0 { + return "", src[endOfLine:], nil + } // Work backwards to check if the line ends in whitespace then // a comment (ie asdasd # some comment) for i := endOfVar - 1; i >= 0; i-- { if line[i] == charComment && i > 0 { if isSpace(line[i-1]) { - endOfVar = i - 1 + endOfVar = i break } } } - return expandVariables(string(line[0:endOfVar]), vars), src[endOfLine:], nil + trimmed := strings.TrimFunc(string(line[0:endOfVar]), isSpace) + + return expandVariables(trimmed, vars), src[endOfLine:], nil } // lookup quoted string terminator From 27959f30c6eb75635836ec8626da1d90bba1ceed Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 22:05:56 +1100 Subject: [PATCH 09/10] Bring back FOO.BAR names --- godotenv_test.go | 2 -- parser.go | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/godotenv_test.go b/godotenv_test.go index 8520a24..edb5781 100644 --- a/godotenv_test.go +++ b/godotenv_test.go @@ -400,8 +400,6 @@ func TestParsing(t *testing.T) { parseAndCompare(t, `FOO="bar\\r\ b\az"`, "FOO", "bar\\r baz") parseAndCompare(t, `="value"`, "", "value") - parseAndCompare(t, `KEY="`, "KEY", "\"") - parseAndCompare(t, `KEY="value`, "KEY", "\"value") // unquoted whitespace around keys should be ignored parseAndCompare(t, " KEY =value", "KEY", "value") diff --git a/parser.go b/parser.go index f2cbddf..203f976 100644 --- a/parser.go +++ b/parser.go @@ -101,8 +101,8 @@ loop: break loop case '_': default: - // variable name should match [A-Za-z0-9_] - if unicode.IsLetter(rchar) || unicode.IsNumber(rchar) { + // variable name should match [A-Za-z0-9_.] + if unicode.IsLetter(rchar) || unicode.IsNumber(rchar) || rchar == '.' { continue } From e768b04a4b638d0602280845212ba1184627e4f7 Mon Sep 17 00:00:00 2001 From: John Barton Date: Sun, 5 Feb 2023 22:11:18 +1100 Subject: [PATCH 10/10] remove some commented out code --- parser.go | 6 ------ 1 file changed, 6 deletions(-) diff --git a/parser.go b/parser.go index 203f976..cc709af 100644 --- a/parser.go +++ b/parser.go @@ -70,13 +70,7 @@ func getStatementStart(src []byte) []byte { // locateKeyName locates and parses key name and returns rest of slice func locateKeyName(src []byte) (key string, cutset []byte, err error) { // trim "export" and space at beginning - // src = bytes.TrimLeftFunc(bytes.TrimPrefix(src, []byte(exportPrefix)), isSpace) src = bytes.TrimLeftFunc(src, isSpace) - // exportPrefixEnd := len(exportPrefix) - 1 - // if len(src) > exportPrefixEnd+2 && bytes.Equal(src[0:exportPrefixEnd], []byte(exportPrefix)) && bytes.IndexFunc(src[exportPrefixEnd:], isSpace) == 0 { - // src = src[exportPrefixEnd:] - // fmt.Println(src) - // } if bytes.HasPrefix(src, []byte(exportPrefix)) { trimmed := bytes.TrimPrefix(src, []byte(exportPrefix)) if bytes.IndexFunc(trimmed, isSpace) == 0 {