From 2fb7c3c4f0b2fe15e279729a87c465df4fa70879 Mon Sep 17 00:00:00 2001 From: Doug Davis Date: Mon, 3 Apr 2017 10:45:39 -0700 Subject: [PATCH] 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 --- builder/dockerfile/envVarTest | 27 +++++++++------- builder/dockerfile/shell_parser.go | 40 +++++++++++++++++++++--- builder/dockerfile/shell_parser_test.go | 8 +++-- builder/dockerfile/wordsTest | 7 ++++- integration-cli/docker_cli_build_test.go | 8 +++++ 5 files changed, 71 insertions(+), 19 deletions(-) diff --git a/builder/dockerfile/envVarTest b/builder/dockerfile/envVarTest index 067dca9a54..946b278592 100644 --- a/builder/dockerfile/envVarTest +++ b/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\" | hello" +A|"hello | error +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|"안녕하세요\" | 안녕하세요" +A|"안녕하세요 | error +A|"안녕하세요\" | error A|"안녕'하세요" | 안녕'하세요 -A|'안녕하세요 | 안녕하세요 +A|'안녕하세요 | error A|'안녕하세요\' | 안녕하세요\ A|안녕$1x | 안녕x A|안녕$.x | 안녕$.x diff --git a/builder/dockerfile/shell_parser.go b/builder/dockerfile/shell_parser.go index 189afd1fdb..6537a66e11 100644 --- a/builder/dockerfile/shell_parser.go +++ b/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: + // $ ` " \ . + // 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 == '$' { - // \" and \$ can be escaped, all other \'s are left as-is + // Note: for now don't do anything special with ` chars. + // 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() } } diff --git a/builder/dockerfile/shell_parser_test.go b/builder/dockerfile/shell_parser_test.go index ca4728d798..e384de83f3 100644 --- a/builder/dockerfile/shell_parser_test.go +++ b/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) } } } diff --git a/builder/dockerfile/wordsTest b/builder/dockerfile/wordsTest index fa916c67f9..1fd9f19433 100644 --- a/builder/dockerfile/wordsTest +++ b/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 diff --git a/integration-cli/docker_cli_build_test.go b/integration-cli/docker_cli_build_test.go index bd712615ab..ff15942e35 100644 --- a/integration-cli/docker_cli_build_test.go +++ b/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{}