From 2b500e63cbc8e91235deafa09bb0d8fe34d443e2 Mon Sep 17 00:00:00 2001
From: Yong Tang <yong.tang.github@outlook.com>
Date: Sat, 3 Sep 2016 15:06:42 -0700
Subject: [PATCH] Return error if env file contains non-ascii or utf8 bytes
 (for windows)

This fix tries to address the issue raised in 26179 where an env file
with non-ascii or utf8 bytes will crash on windows platform.

The issue is two-fold:
- Windows will adds a BOM mark at the begining with Notepad as the editor
- Non-utf8 bytes can not be handled by env file parser.

This fix removes utf8 BOM marker if exists so that utf8 encoded env file
could be processed.
This fix also returns an error (instead of a runtime CreateProcess crash)
if env file contains non-utf8 bytes, thus giving users better experiences.

Additional test cases has been added in unit tests.

This fix fixes 26179.

Signed-off-by: Yong Tang <yong.tang.github@outlook.com>
Upstream-commit: 01d3a16f58118a3f6a6207fcb028bb279eb16e73
Component: engine
---
 components/engine/runconfig/opts/envfile.go   |  16 ++++++++++-
 .../engine/runconfig/opts/fixtures/utf16.env  | Bin 0 -> 54 bytes
 .../runconfig/opts/fixtures/utf16be.env       | Bin 0 -> 54 bytes
 .../engine/runconfig/opts/fixtures/utf8.env   |   3 ++
 .../engine/runconfig/opts/parse_test.go       |  27 ++++++++++++++++++
 5 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100755 components/engine/runconfig/opts/fixtures/utf16.env
 create mode 100755 components/engine/runconfig/opts/fixtures/utf16be.env
 create mode 100755 components/engine/runconfig/opts/fixtures/utf8.env

diff --git a/components/engine/runconfig/opts/envfile.go b/components/engine/runconfig/opts/envfile.go
index ba8b4f2016..f723799215 100644
--- a/components/engine/runconfig/opts/envfile.go
+++ b/components/engine/runconfig/opts/envfile.go
@@ -2,9 +2,12 @@ package opts
 
 import (
 	"bufio"
+	"bytes"
 	"fmt"
 	"os"
 	"strings"
+	"unicode"
+	"unicode/utf8"
 )
 
 // ParseEnvFile reads a file with environment variables enumerated by lines
@@ -29,9 +32,20 @@ func ParseEnvFile(filename string) ([]string, error) {
 
 	lines := []string{}
 	scanner := bufio.NewScanner(fh)
+	currentLine := 0
+	utf8bom := []byte{0xEF, 0xBB, 0xBF}
 	for scanner.Scan() {
+		scannedBytes := scanner.Bytes()
+		if !utf8.Valid(scannedBytes) {
+			return []string{}, fmt.Errorf("env file %s contains invalid utf8 bytes at line %d: %v", filename, currentLine+1, scannedBytes)
+		}
+		// We trim UTF8 BOM
+		if currentLine == 0 {
+			scannedBytes = bytes.TrimPrefix(scannedBytes, utf8bom)
+		}
 		// trim the line from all leading whitespace first
-		line := strings.TrimLeft(scanner.Text(), whiteSpaces)
+		line := strings.TrimLeftFunc(string(scannedBytes), unicode.IsSpace)
+		currentLine++
 		// line is not empty, and not starting with '#'
 		if len(line) > 0 && !strings.HasPrefix(line, "#") {
 			data := strings.SplitN(line, "=", 2)
diff --git a/components/engine/runconfig/opts/fixtures/utf16.env b/components/engine/runconfig/opts/fixtures/utf16.env
new file mode 100755
index 0000000000000000000000000000000000000000..3a73358fffbc0d5d3d4df985ccf2f4a1a29cdb2a
GIT binary patch
literal 54
ucmezW&yB$!2yGdh7#tab7<d`D7(5tU8GL{cB)TG@HWDNTk+o%js0IKq+6kus

literal 0
HcmV?d00001

diff --git a/components/engine/runconfig/opts/fixtures/utf16be.env b/components/engine/runconfig/opts/fixtures/utf16be.env
new file mode 100755
index 0000000000000000000000000000000000000000..e523da7af4dab0636ed73fabf730f33a1cb915b4
GIT binary patch
literal 54
ucmezOpTUj69|&z3oERJ#f*5!ixEMSbTp4_T5Gb0kBC-}N29dRes0IKkq6wz}

literal 0
HcmV?d00001

diff --git a/components/engine/runconfig/opts/fixtures/utf8.env b/components/engine/runconfig/opts/fixtures/utf8.env
new file mode 100755
index 0000000000..1ce45055b7
--- /dev/null
+++ b/components/engine/runconfig/opts/fixtures/utf8.env
@@ -0,0 +1,3 @@
+FOO=BAR
+HELLO=您好
+BAR=FOO
\ No newline at end of file
diff --git a/components/engine/runconfig/opts/parse_test.go b/components/engine/runconfig/opts/parse_test.go
index 67f4c40e37..a1be379ae8 100644
--- a/components/engine/runconfig/opts/parse_test.go
+++ b/components/engine/runconfig/opts/parse_test.go
@@ -666,6 +666,33 @@ func TestParseEnvfileVariables(t *testing.T) {
 	}
 }
 
+func TestParseEnvfileVariablesWithBOMUnicode(t *testing.T) {
+	// UTF8 with BOM
+	config, _, _, err := parseRun([]string{"--env-file=fixtures/utf8.env", "img", "cmd"})
+	if err != nil {
+		t.Fatal(err)
+	}
+	env := []string{"FOO=BAR", "HELLO=" + string([]byte{0xe6, 0x82, 0xa8, 0xe5, 0xa5, 0xbd}), "BAR=FOO"}
+	if len(config.Env) != len(env) {
+		t.Fatalf("Expected a config with %d env variables, got %v: %v", len(env), len(config.Env), config.Env)
+	}
+	for i, v := range env {
+		if config.Env[i] != v {
+			t.Fatalf("Expected a config with [%s], got %v", v, []byte(config.Env[i]))
+		}
+	}
+
+	// UTF16 with BOM
+	e := "contains invalid utf8 bytes at line"
+	if _, _, _, err := parseRun([]string{"--env-file=fixtures/utf16.env", "img", "cmd"}); err == nil || !strings.Contains(err.Error(), e) {
+		t.Fatalf("Expected an error with message '%s', got %v", e, err)
+	}
+	// UTF16BE with BOM
+	if _, _, _, err := parseRun([]string{"--env-file=fixtures/utf16be.env", "img", "cmd"}); err == nil || !strings.Contains(err.Error(), e) {
+		t.Fatalf("Expected an error with message '%s', got %v", e, err)
+	}
+}
+
 func TestParseLabelfileVariables(t *testing.T) {
 	e := "open nonexistent: no such file or directory"
 	if runtime.GOOS == "windows" {