Minor tweaks to quotes in env vars

Addresses part of #32140, in particular:
- this will make it so that double backslashes in double-quoted
strings will result in a single backslash. While in single quotes it remains
a double backslash.
- missing closing " and ' will now generate an error

Signed-off-by: Doug Davis <dug@us.ibm.com>

builder/dockerfile/envVarTest

@@ -1,18 +1,21 @@
 A|hello                    |     hello
 A|he'll'o                  |     hello
-A|he'llo                   |     hello
+A|he'llo                   |     error
 A|he\'llo                  |     he'llo
-A|he\\'llo                 |     he\llo
+A|he\\'llo                 |     error
 A|abc\tdef                 |     abctdef
 A|"abc\tdef"               |     abc\tdef
+A|"abc\\tdef"              |     abc\tdef
 A|'abc\tdef'               |     abc\tdef
 A|hello\                   |     hello
 A|hello\\                  |     hello\
-A|"hello                   |     hello
+A|"hello                   |     error
-A|"hello\"                 |     hello"
+A|"hello\"                 |     error
 A|"hel'lo"                 |     hel'lo
-A|'hello                   |     hello
+A|'hello                   |     error
 A|'hello\'                 |     hello\
+A|'hello\there'            |     hello\there
+A|'hello\\there'           |     hello\\there
 A|"''"                     |     ''
 A|$.                       |     $.
 A|$1                       |
@@ -24,6 +27,8 @@ W|he$pwd.                  |     he/home.
 A|he$PWD                   |     he/home
 A|he\$PWD                  |     he$PWD
 A|he\\$PWD                 |     he\/home
+A|"he\$PWD"                |     he$PWD
+A|"he\\$PWD"               |     he\/home
 A|he\${}                   |     he${}
 A|he\${}xx                 |     he${}xx
 A|he${}                    |     he
@@ -60,18 +65,18 @@ A|he${XXX:-\$PWD:}xx       |     he$PWD:xx
 A|he${XXX:-\${PWD}z}xx     |     he${PWDz}xx
 A|안녕하세요                 |     안녕하세요
 A|안'녕'하세요               |     안녕하세요
-A|안'녕하세요                |     안녕하세요
+A|안'녕하세요                |     error
 A|안녕\'하세요               |     안녕'하세요
-A|안\\'녕하세요              |     안\녕하세요
+A|안\\'녕하세요              |     error
 A|안녕\t하세요               |     안녕t하세요
 A|"안녕\t하세요"             |     안녕\t하세요
-A|'안녕\t하세요              |     안녕\t하세요
+A|'안녕\t하세요              |     error
 A|안녕하세요\                |     안녕하세요
 A|안녕하세요\\               |     안녕하세요\
-A|"안녕하세요                |     안녕하세요
+A|"안녕하세요                |     error
-A|"안녕하세요\"              |     안녕하세요"
+A|"안녕하세요\"              |     error
 A|"안녕'하세요"              |     안녕'하세요
-A|'안녕하세요                |     안녕하세요
+A|'안녕하세요                |     error
 A|'안녕하세요\'              |     안녕하세요\
 A|안녕$1x                    |     안녕x
 A|안녕$.x                    |     안녕$.x

builder/dockerfile/shell_parser.go

@@ -166,15 +166,28 @@ func (sw *shellWord) processStopOn(stopChar rune) (string, []string, error) {
 func (sw *shellWord) processSingleQuote() (string, error) {
 	// All chars between single quotes are taken as-is
 	// Note, you can't escape '
+	//
+	// From the "sh" man page:
+	// Single Quotes
+	//   Enclosing characters in single quotes preserves the literal meaning of
+	//   all the characters (except single quotes, making it impossible to put
+	//   single-quotes in a single-quoted string).
+
 	var result string
 
 	sw.scanner.Next()
 
 	for {
 		ch := sw.scanner.Next()
-		if ch == '\'' || ch == scanner.EOF {
+
+		if ch == scanner.EOF {
+			return "", fmt.Errorf("unexpected end of statement while looking for matching single-quote")
+		}
+
+		if ch == '\'' {
 			break
 		}
+
 		result += string(ch)
 	}
 
@@ -184,16 +197,32 @@ func (sw *shellWord) processSingleQuote() (string, error) {
 func (sw *shellWord) processDoubleQuote() (string, error) {
 	// All chars up to the next " are taken as-is, even ', except any $ chars
 	// But you can escape " with a \ (or ` if escape token set accordingly)
+	//
+	// From the "sh" man page:
+	// Double Quotes
+	//  Enclosing characters within double quotes preserves the literal meaning
+	//  of all characters except dollarsign ($), backquote (`), and backslash
+	//  (\).  The backslash inside double quotes is historically weird, and
+	//  serves to quote only the following characters:
+	//    $ ` " \ <newline>.
+	//  Otherwise it remains literal.
+
 	var result string
 
 	sw.scanner.Next()
 
-	for sw.scanner.Peek() != scanner.EOF {
+	for {
 		ch := sw.scanner.Peek()
+
+		if ch == scanner.EOF {
+			return "", fmt.Errorf("unexpected end of statement while looking for matching double-quote")
+		}
+
 		if ch == '"' {
 			sw.scanner.Next()
 			break
 		}
+
 		if ch == '$' {
 			tmp, err := sw.processDollar()
 			if err != nil {
@@ -210,8 +239,11 @@ func (sw *shellWord) processDoubleQuote() (string, error) {
 					continue
 				}
 
-				if chNext == '"' || chNext == '$' {
+				// Note: for now don't do anything special with ` chars.
-					// \" and \$ can be escaped, all other \'s are left as-is
+				// Not sure what to do with them anyway since we're not going
+				// to execute the text in there (not now anyway).
+				if chNext == '"' || chNext == '$' || chNext == sw.escapeToken {
+					// These chars can be escaped, all other \'s are left as-is
 					ch = sw.scanner.Next()
 				}
 			}

builder/dockerfile/shell_parser_test.go

@@ -75,8 +75,10 @@ func TestShellParser4Words(t *testing.T) {
 
 	envs := []string{}
 	scanner := bufio.NewScanner(file)
+	lineNum := 0
 	for scanner.Scan() {
 		line := scanner.Text()
+		lineNum = lineNum + 1
 
 		if strings.HasPrefix(line, "#") {
 			continue
@@ -90,7 +92,7 @@ func TestShellParser4Words(t *testing.T) {
 
 		words := strings.Split(line, "|")
 		if len(words) != 2 {
-			t.Fatalf("Error in '%s' - should be exactly one | in: %q", fn, line)
+			t.Fatalf("Error in '%s'(line %d) - should be exactly one | in: %q", fn, lineNum, line)
 		}
 		test := strings.TrimSpace(words[0])
 		expected := strings.Split(strings.TrimLeft(words[1], " "), ",")
@@ -102,11 +104,11 @@ func TestShellParser4Words(t *testing.T) {
 		}
 
 		if len(result) != len(expected) {
-			t.Fatalf("Error. %q was suppose to result in %q, but got %q instead", test, expected, result)
+			t.Fatalf("Error on line %d. %q was suppose to result in %q, but got %q instead", lineNum, test, expected, result)
 		}
 		for i, w := range expected {
 			if w != result[i] {
-				t.Fatalf("Error. %q was suppose to result in %q, but got %q instead", test, expected, result)
+				t.Fatalf("Error on line %d. %q was suppose to result in %q, but got %q instead", lineNum, test, expected, result)
 			}
 		}
 	}

builder/dockerfile/wordsTest

@@ -21,5 +21,10 @@ hel"lo${trailing}" | helloab c
 hello" there  " | hello there  
 hello there     | hello,there
 hello\ there | hello there
-hello" there | hello there
+hello" there | error
 hello\" there | hello",there
+hello"\\there" | hello\there
+hello"\there" | hello\there
+hello'\\there' | hello\\there
+hello'\there' | hello\there
+hello'$there' | hello$there

integration-cli/docker_cli_build_test.go

@@ -182,6 +182,14 @@ func (s *DockerSuite) TestBuildEnvironmentReplacementEnv(c *check.C) {
   RUN [ "$abc3" = '$foo' ] && (echo "$abc3" | grep -q foo)
   ENV abc4 "\$foo"
   RUN [ "$abc4" = '$foo' ] && (echo "$abc4" | grep -q foo)
+  ENV foo2="abc\def"
+  RUN [ "$foo2" = 'abc\def' ]
+  ENV foo3="abc\\def"
+  RUN [ "$foo3" = 'abc\def' ]
+  ENV foo4='abc\\def'
+  RUN [ "$foo4" = 'abc\\def' ]
+  ENV foo5='abc\def'
+  RUN [ "$foo5" = 'abc\def' ]
   `))
 
 	envResult := []string{}