Skip to content

Commit

Permalink
Merge pull request #5502 from thaJeztah/separate_kvfile
Browse files Browse the repository at this point in the history
move parsing key-value files to a separate package
  • Loading branch information
thaJeztah authored Oct 4, 2024
2 parents 9025c93 + 9ecfe4f commit ff3f94a
Show file tree
Hide file tree
Showing 8 changed files with 283 additions and 161 deletions.
2 changes: 1 addition & 1 deletion cli/command/container/opts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -919,7 +919,7 @@ func TestParseEnvfileVariablesWithBOMUnicode(t *testing.T) {
}

// UTF16 with BOM
e := "contains invalid utf8 bytes at line"
e := "invalid utf8 bytes at line"
if _, _, _, err := parseRun([]string{"--env-file=testdata/utf16.env", "img", "cmd"}); err == nil || !strings.Contains(err.Error(), e) {
t.Fatalf("Expected an error with message '%s', got %v", e, err)
}
Expand Down
4 changes: 3 additions & 1 deletion opts/envfile.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package opts

import (
"os"

"github.com/docker/cli/pkg/kvfile"
)

// ParseEnvFile reads a file with environment variables enumerated by lines
Expand All @@ -18,5 +20,5 @@ import (
// environment variables, that's why we just strip leading whitespace and
// nothing more.
func ParseEnvFile(filename string) ([]string, error) {
return parseKeyValueFile(filename, os.LookupEnv)
return kvfile.Parse(filename, os.LookupEnv)
}
88 changes: 0 additions & 88 deletions opts/envfile_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package opts

import (
"bufio"
"os"
"path/filepath"
"strings"
"testing"

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

// Test ParseEnvFile for a file with a few well formatted lines
func TestParseEnvFileGoodFile(t *testing.T) {
content := `foo=bar
baz=quux
# comment
_foobar=foobaz
with.dots=working
and_underscore=working too
`
// Adding a newline + a line with pure whitespace.
// This is being done like this instead of the block above
// because it's common for editors to trim trailing whitespace
// from lines, which becomes annoying since that's the
// exact thing we need to test.
content += "\n \t "
tmpFile := tmpFileWithContent(t, content)

lines, err := ParseEnvFile(tmpFile)
assert.NilError(t, err)

expectedLines := []string{
"foo=bar",
"baz=quux",
"_foobar=foobaz",
"with.dots=working",
"and_underscore=working too",
}

assert.Check(t, is.DeepEqual(lines, expectedLines))
}

// Test ParseEnvFile for an empty file
func TestParseEnvFileEmptyFile(t *testing.T) {
tmpFile := tmpFileWithContent(t, "")

lines, err := ParseEnvFile(tmpFile)
assert.NilError(t, err)
assert.Check(t, is.Len(lines, 0))
}

// Test ParseEnvFile for a non existent file
func TestParseEnvFileNonExistentFile(t *testing.T) {
_, err := ParseEnvFile("no_such_file")
assert.Check(t, is.ErrorType(err, os.IsNotExist))
}

// Test ParseEnvFile for a badly formatted file
func TestParseEnvFileBadlyFormattedFile(t *testing.T) {
content := `foo=bar
f =quux
`
tmpFile := tmpFileWithContent(t, content)

_, err := ParseEnvFile(tmpFile)
const expectedMessage = "variable 'f ' contains whitespaces"
assert.Check(t, is.ErrorContains(err, expectedMessage))
}

// Test ParseEnvFile for a file with a line exceeding bufio.MaxScanTokenSize
func TestParseEnvFileLineTooLongFile(t *testing.T) {
content := "foo=" + strings.Repeat("a", bufio.MaxScanTokenSize+42)
tmpFile := tmpFileWithContent(t, content)

_, err := ParseEnvFile(tmpFile)
const expectedMessage = "bufio.Scanner: token too long"
assert.Check(t, is.ErrorContains(err, expectedMessage))
}

// ParseEnvFile with a random file, pass through
func TestParseEnvFileRandomFile(t *testing.T) {
content := `first line
another invalid line`
tmpFile := tmpFileWithContent(t, content)

_, err := ParseEnvFile(tmpFile)
const expectedMessage = "variable 'first line' contains whitespaces"
assert.Check(t, is.ErrorContains(err, expectedMessage))
}

// ParseEnvFile with environment variable import definitions
func TestParseEnvVariableDefinitionsFile(t *testing.T) {
content := `# comment=
Expand All @@ -114,15 +38,3 @@ DEFINED_VAR
expectedLines := []string{"DEFINED_VAR=defined-value"}
assert.Check(t, is.DeepEqual(variables, expectedLines))
}

// ParseEnvFile with empty variable name
func TestParseEnvVariableWithNoNameFile(t *testing.T) {
content := `# comment=
=blank variable names are an error case
`
tmpFile := tmpFileWithContent(t, content)

_, err := ParseEnvFile(tmpFile)
const expectedMessage = "no variable name on line '=blank variable names are an error case'"
assert.Check(t, is.ErrorContains(err, expectedMessage))
}
70 changes: 0 additions & 70 deletions opts/file.go

This file was deleted.

2 changes: 2 additions & 0 deletions opts/opts.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,8 @@ func validateDomain(val string) (string, error) {
return "", fmt.Errorf("%s is not a valid domain", val)
}

const whiteSpaces = " \t"

// ValidateLabel validates that the specified string is a valid label, and returns it.
//
// Labels are in the form of key=value; key must be a non-empty string, and not
Expand Down
3 changes: 2 additions & 1 deletion opts/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strconv"
"strings"

"github.com/docker/cli/pkg/kvfile"
"github.com/docker/docker/api/types/container"
)

Expand All @@ -25,7 +26,7 @@ func ReadKVEnvStrings(files []string, override []string) ([]string, error) {
func readKVStrings(files []string, override []string, emptyFn func(string) (string, bool)) ([]string, error) {
var variables []string
for _, ef := range files {
parsedVars, err := parseKeyValueFile(ef, emptyFn)
parsedVars, err := kvfile.Parse(ef, emptyFn)
if err != nil {
return nil, err
}
Expand Down
130 changes: 130 additions & 0 deletions pkg/kvfile/kvfile.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
// Package kvfile provides utilities to parse line-delimited key/value files
// such as used for label-files and env-files.
//
// # File format
//
// key/value files use the following syntax:
//
// - File must be valid UTF-8.
// - BOM headers are removed.
// - Leading whitespace is removed for each line.
// - Lines starting with "#" are ignored.
// - Empty lines are ignored.
// - Key/Value pairs are provided as "KEY[=<VALUE>]".
// - Maximum line-length is limited to [bufio.MaxScanTokenSize].
//
// # Interpolation, substitution, and escaping
//
// Both keys and values are used as-is; no interpolation, substitution or
// escaping is supported, and quotes are considered part of the key or value.
// Whitespace in values (including leading and trailing) is preserved. Given
// that the file format is line-delimited, neither key, nor value, can contain
// newlines.
//
// # Key/Value pairs
//
// Key/Value pairs take the following format:
//
// KEY[=<VALUE>]
//
// KEY is required and may not contain whitespaces or NUL characters. Any
// other character (except for the "=" delimiter) are accepted, but it is
// recommended to use a subset of the POSIX portable character set, as
// outlined in [Environment Variables].
//
// VALUE is optional, but may be empty. If no value is provided (i.e., no
// equal sign ("=") is present), the KEY is omitted in the result, but some
// functions accept a lookup-function to provide a default value for the
// given key.
//
// [Environment Variables]: https://pubs.opengroup.org/onlinepubs/7908799/xbd/envvar.html
package kvfile

import (
"bufio"
"bytes"
"fmt"
"io"
"os"
"strings"
"unicode"
"unicode/utf8"
)

// Parse parses a line-delimited key/value pairs separated by equal sign.
// It accepts a lookupFn to lookup default values for keys that do not define
// a value. An error is produced if parsing failed, the content contains invalid
// UTF-8 characters, or a key contains whitespaces.
func Parse(filename string, lookupFn func(key string) (value string, found bool)) ([]string, error) {
fh, err := os.Open(filename)
if err != nil {
return []string{}, err
}
out, err := parseKeyValueFile(fh, lookupFn)
_ = fh.Close()
if err != nil {
return []string{}, fmt.Errorf("invalid env file (%s): %v", filename, err)
}
return out, nil
}

// ParseFromReader parses a line-delimited key/value pairs separated by equal sign.
// It accepts a lookupFn to lookup default values for keys that do not define
// a value. An error is produced if parsing failed, the content contains invalid
// UTF-8 characters, or a key contains whitespaces.
func ParseFromReader(r io.Reader, lookupFn func(key string) (value string, found bool)) ([]string, error) {
return parseKeyValueFile(r, lookupFn)
}

const whiteSpaces = " \t"

func parseKeyValueFile(r io.Reader, lookupFn func(string) (string, bool)) ([]string, error) {
lines := []string{}
scanner := bufio.NewScanner(r)
utf8bom := []byte{0xEF, 0xBB, 0xBF}
for currentLine := 1; scanner.Scan(); currentLine++ {
scannedBytes := scanner.Bytes()
if !utf8.Valid(scannedBytes) {
return []string{}, fmt.Errorf("invalid utf8 bytes at line %d: %v", currentLine, scannedBytes)
}
// We trim UTF8 BOM
if currentLine == 1 {
scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom)
}
// trim the line from all leading whitespace first. trailing whitespace
// is part of the value, and is kept unmodified.
line := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace)

if len(line) == 0 || line[0] == '#' {
// skip empty lines and comments (lines starting with '#')
continue
}

key, _, hasValue := strings.Cut(line, "=")
if len(key) == 0 {
return []string{}, fmt.Errorf("no variable name on line '%s'", line)
}

// leading whitespace was already removed from the line, but
// variables are not allowed to contain whitespace or have
// trailing whitespace.
if strings.ContainsAny(key, whiteSpaces) {
return []string{}, fmt.Errorf("variable '%s' contains whitespaces", key)
}

if hasValue {
// key/value pair is valid and has a value; add the line as-is.
lines = append(lines, line)
continue
}

if lookupFn != nil {
// No value given; try to look up the value. The value may be
// empty but if no value is found, the key is omitted.
if value, found := lookupFn(line); found {
lines = append(lines, key+"="+value)
}
}
}
return lines, scanner.Err()
}
Loading

0 comments on commit ff3f94a

Please sign in to comment.