Skip to content

Commit 9ecfe4f

Browse files
ndeloofthaJeztah
authored andcommitted
move parsing key-value files to a separate package
Move the code for parsing key-value files, such as used for env-files and label-files to a separate package. This allows other projects (such as compose) to use the same parsing logic, but provide custom lookup functions for their situation (which is slightly different). The new package provides utilities for parsing key-value files for either a file or an io.Reader. Most tests for EnvFile were now testing functionality that's already tested in the new package, so were (re)moved. Co-authored-by: Nicolas De Loof <[email protected]> Signed-off-by: Nicolas De Loof <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>
1 parent 9dee12f commit 9ecfe4f

File tree

8 files changed

+283
-161
lines changed

8 files changed

+283
-161
lines changed

cli/command/container/opts_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -919,7 +919,7 @@ func TestParseEnvfileVariablesWithBOMUnicode(t *testing.T) {
919919
}
920920

921921
// UTF16 with BOM
922-
e := "contains invalid utf8 bytes at line"
922+
e := "invalid utf8 bytes at line"
923923
if _, _, _, err := parseRun([]string{"--env-file=testdata/utf16.env", "img", "cmd"}); err == nil || !strings.Contains(err.Error(), e) {
924924
t.Fatalf("Expected an error with message '%s', got %v", e, err)
925925
}

opts/envfile.go

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package opts
22

33
import (
44
"os"
5+
6+
"github.com/docker/cli/pkg/kvfile"
57
)
68

79
// ParseEnvFile reads a file with environment variables enumerated by lines
@@ -18,5 +20,5 @@ import (
1820
// environment variables, that's why we just strip leading whitespace and
1921
// nothing more.
2022
func ParseEnvFile(filename string) ([]string, error) {
21-
return parseKeyValueFile(filename, os.LookupEnv)
23+
return kvfile.Parse(filename, os.LookupEnv)
2224
}

opts/envfile_test.go

-88
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
package opts
22

33
import (
4-
"bufio"
54
"os"
65
"path/filepath"
7-
"strings"
86
"testing"
97

108
"gotest.tools/v3/assert"
@@ -19,86 +17,12 @@ func tmpFileWithContent(t *testing.T, content string) string {
1917
return fileName
2018
}
2119

22-
// Test ParseEnvFile for a file with a few well formatted lines
23-
func TestParseEnvFileGoodFile(t *testing.T) {
24-
content := `foo=bar
25-
baz=quux
26-
# comment
27-
28-
_foobar=foobaz
29-
with.dots=working
30-
and_underscore=working too
31-
`
32-
// Adding a newline + a line with pure whitespace.
33-
// This is being done like this instead of the block above
34-
// because it's common for editors to trim trailing whitespace
35-
// from lines, which becomes annoying since that's the
36-
// exact thing we need to test.
37-
content += "\n \t "
38-
tmpFile := tmpFileWithContent(t, content)
39-
40-
lines, err := ParseEnvFile(tmpFile)
41-
assert.NilError(t, err)
42-
43-
expectedLines := []string{
44-
"foo=bar",
45-
"baz=quux",
46-
"_foobar=foobaz",
47-
"with.dots=working",
48-
"and_underscore=working too",
49-
}
50-
51-
assert.Check(t, is.DeepEqual(lines, expectedLines))
52-
}
53-
54-
// Test ParseEnvFile for an empty file
55-
func TestParseEnvFileEmptyFile(t *testing.T) {
56-
tmpFile := tmpFileWithContent(t, "")
57-
58-
lines, err := ParseEnvFile(tmpFile)
59-
assert.NilError(t, err)
60-
assert.Check(t, is.Len(lines, 0))
61-
}
62-
6320
// Test ParseEnvFile for a non existent file
6421
func TestParseEnvFileNonExistentFile(t *testing.T) {
6522
_, err := ParseEnvFile("no_such_file")
6623
assert.Check(t, is.ErrorType(err, os.IsNotExist))
6724
}
6825

69-
// Test ParseEnvFile for a badly formatted file
70-
func TestParseEnvFileBadlyFormattedFile(t *testing.T) {
71-
content := `foo=bar
72-
f =quux
73-
`
74-
tmpFile := tmpFileWithContent(t, content)
75-
76-
_, err := ParseEnvFile(tmpFile)
77-
const expectedMessage = "variable 'f ' contains whitespaces"
78-
assert.Check(t, is.ErrorContains(err, expectedMessage))
79-
}
80-
81-
// Test ParseEnvFile for a file with a line exceeding bufio.MaxScanTokenSize
82-
func TestParseEnvFileLineTooLongFile(t *testing.T) {
83-
content := "foo=" + strings.Repeat("a", bufio.MaxScanTokenSize+42)
84-
tmpFile := tmpFileWithContent(t, content)
85-
86-
_, err := ParseEnvFile(tmpFile)
87-
const expectedMessage = "bufio.Scanner: token too long"
88-
assert.Check(t, is.ErrorContains(err, expectedMessage))
89-
}
90-
91-
// ParseEnvFile with a random file, pass through
92-
func TestParseEnvFileRandomFile(t *testing.T) {
93-
content := `first line
94-
another invalid line`
95-
tmpFile := tmpFileWithContent(t, content)
96-
97-
_, err := ParseEnvFile(tmpFile)
98-
const expectedMessage = "variable 'first line' contains whitespaces"
99-
assert.Check(t, is.ErrorContains(err, expectedMessage))
100-
}
101-
10226
// ParseEnvFile with environment variable import definitions
10327
func TestParseEnvVariableDefinitionsFile(t *testing.T) {
10428
content := `# comment=
@@ -114,15 +38,3 @@ DEFINED_VAR
11438
expectedLines := []string{"DEFINED_VAR=defined-value"}
11539
assert.Check(t, is.DeepEqual(variables, expectedLines))
11640
}
117-
118-
// ParseEnvFile with empty variable name
119-
func TestParseEnvVariableWithNoNameFile(t *testing.T) {
120-
content := `# comment=
121-
=blank variable names are an error case
122-
`
123-
tmpFile := tmpFileWithContent(t, content)
124-
125-
_, err := ParseEnvFile(tmpFile)
126-
const expectedMessage = "no variable name on line '=blank variable names are an error case'"
127-
assert.Check(t, is.ErrorContains(err, expectedMessage))
128-
}

opts/file.go

-70
This file was deleted.

opts/opts.go

+2
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,8 @@ func validateDomain(val string) (string, error) {
266266
return "", fmt.Errorf("%s is not a valid domain", val)
267267
}
268268

269+
const whiteSpaces = " \t"
270+
269271
// ValidateLabel validates that the specified string is a valid label, and returns it.
270272
//
271273
// Labels are in the form of key=value; key must be a non-empty string, and not

opts/parse.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"strconv"
77
"strings"
88

9+
"github.com/docker/cli/pkg/kvfile"
910
"github.com/docker/docker/api/types/container"
1011
)
1112

@@ -25,7 +26,7 @@ func ReadKVEnvStrings(files []string, override []string) ([]string, error) {
2526
func readKVStrings(files []string, override []string, emptyFn func(string) (string, bool)) ([]string, error) {
2627
var variables []string
2728
for _, ef := range files {
28-
parsedVars, err := parseKeyValueFile(ef, emptyFn)
29+
parsedVars, err := kvfile.Parse(ef, emptyFn)
2930
if err != nil {
3031
return nil, err
3132
}

pkg/kvfile/kvfile.go

+130
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
// Package kvfile provides utilities to parse line-delimited key/value files
2+
// such as used for label-files and env-files.
3+
//
4+
// # File format
5+
//
6+
// key/value files use the following syntax:
7+
//
8+
// - File must be valid UTF-8.
9+
// - BOM headers are removed.
10+
// - Leading whitespace is removed for each line.
11+
// - Lines starting with "#" are ignored.
12+
// - Empty lines are ignored.
13+
// - Key/Value pairs are provided as "KEY[=<VALUE>]".
14+
// - Maximum line-length is limited to [bufio.MaxScanTokenSize].
15+
//
16+
// # Interpolation, substitution, and escaping
17+
//
18+
// Both keys and values are used as-is; no interpolation, substitution or
19+
// escaping is supported, and quotes are considered part of the key or value.
20+
// Whitespace in values (including leading and trailing) is preserved. Given
21+
// that the file format is line-delimited, neither key, nor value, can contain
22+
// newlines.
23+
//
24+
// # Key/Value pairs
25+
//
26+
// Key/Value pairs take the following format:
27+
//
28+
// KEY[=<VALUE>]
29+
//
30+
// KEY is required and may not contain whitespaces or NUL characters. Any
31+
// other character (except for the "=" delimiter) are accepted, but it is
32+
// recommended to use a subset of the POSIX portable character set, as
33+
// outlined in [Environment Variables].
34+
//
35+
// VALUE is optional, but may be empty. If no value is provided (i.e., no
36+
// equal sign ("=") is present), the KEY is omitted in the result, but some
37+
// functions accept a lookup-function to provide a default value for the
38+
// given key.
39+
//
40+
// [Environment Variables]: https://pubs.opengroup.org/onlinepubs/7908799/xbd/envvar.html
41+
package kvfile
42+
43+
import (
44+
"bufio"
45+
"bytes"
46+
"fmt"
47+
"io"
48+
"os"
49+
"strings"
50+
"unicode"
51+
"unicode/utf8"
52+
)
53+
54+
// Parse parses a line-delimited key/value pairs separated by equal sign.
55+
// It accepts a lookupFn to lookup default values for keys that do not define
56+
// a value. An error is produced if parsing failed, the content contains invalid
57+
// UTF-8 characters, or a key contains whitespaces.
58+
func Parse(filename string, lookupFn func(key string) (value string, found bool)) ([]string, error) {
59+
fh, err := os.Open(filename)
60+
if err != nil {
61+
return []string{}, err
62+
}
63+
out, err := parseKeyValueFile(fh, lookupFn)
64+
_ = fh.Close()
65+
if err != nil {
66+
return []string{}, fmt.Errorf("invalid env file (%s): %v", filename, err)
67+
}
68+
return out, nil
69+
}
70+
71+
// ParseFromReader parses a line-delimited key/value pairs separated by equal sign.
72+
// It accepts a lookupFn to lookup default values for keys that do not define
73+
// a value. An error is produced if parsing failed, the content contains invalid
74+
// UTF-8 characters, or a key contains whitespaces.
75+
func ParseFromReader(r io.Reader, lookupFn func(key string) (value string, found bool)) ([]string, error) {
76+
return parseKeyValueFile(r, lookupFn)
77+
}
78+
79+
const whiteSpaces = " \t"
80+
81+
func parseKeyValueFile(r io.Reader, lookupFn func(string) (string, bool)) ([]string, error) {
82+
lines := []string{}
83+
scanner := bufio.NewScanner(r)
84+
utf8bom := []byte{0xEF, 0xBB, 0xBF}
85+
for currentLine := 1; scanner.Scan(); currentLine++ {
86+
scannedBytes := scanner.Bytes()
87+
if !utf8.Valid(scannedBytes) {
88+
return []string{}, fmt.Errorf("invalid utf8 bytes at line %d: %v", currentLine, scannedBytes)
89+
}
90+
// We trim UTF8 BOM
91+
if currentLine == 1 {
92+
scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom)
93+
}
94+
// trim the line from all leading whitespace first. trailing whitespace
95+
// is part of the value, and is kept unmodified.
96+
line := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace)
97+
98+
if len(line) == 0 || line[0] == '#' {
99+
// skip empty lines and comments (lines starting with '#')
100+
continue
101+
}
102+
103+
key, _, hasValue := strings.Cut(line, "=")
104+
if len(key) == 0 {
105+
return []string{}, fmt.Errorf("no variable name on line '%s'", line)
106+
}
107+
108+
// leading whitespace was already removed from the line, but
109+
// variables are not allowed to contain whitespace or have
110+
// trailing whitespace.
111+
if strings.ContainsAny(key, whiteSpaces) {
112+
return []string{}, fmt.Errorf("variable '%s' contains whitespaces", key)
113+
}
114+
115+
if hasValue {
116+
// key/value pair is valid and has a value; add the line as-is.
117+
lines = append(lines, line)
118+
continue
119+
}
120+
121+
if lookupFn != nil {
122+
// No value given; try to look up the value. The value may be
123+
// empty but if no value is found, the key is omitted.
124+
if value, found := lookupFn(line); found {
125+
lines = append(lines, key+"="+value)
126+
}
127+
}
128+
}
129+
return lines, scanner.Err()
130+
}

0 commit comments

Comments
 (0)