Merge pull request moby#5052 from tonistiigi/shell-empty-string

shell: handle empty string for var replacements

Author: tonistiigi

frontend/dockerfile/shell/lex.go

@@ -430,7 +430,8 @@ func (sw *shellWord) processDollar() (string, error) {
 		case '%', '#':
 			// %/# matches the shortest pattern expansion, %%/## the longest
 			greedy := false
-			if word[0] == byte(ch) {
+
+			if len(word) > 0 && word[0] == byte(ch) {
 				greedy = true
 				word = word[1:]
 			}

frontend/dockerfile/shell/lex_test.go

@@ -313,6 +313,21 @@ func TestProcessWithMatches(t *testing.T) {
 			matches:   map[string]struct{}{"FOO": {}, "BAR": {}},
 			unmatched: map[string]struct{}{"BAZ": {}},
 		},
+		{
+			input: "${FOO:-}",
+			envs: map[string]string{
+				"FOO": "xxx",
+				"BAR": "",
+			},
+			expected: "xxx",
+			matches:  map[string]struct{}{"FOO": {}},
+		},
+		{
+			input:     "${FOO:-}",
+			envs:      map[string]string{},
+			expected:  "",
+			unmatched: map[string]struct{}{"FOO": {}},
+		},
 
 		{
 			input: "${FOO+aaa} ${BAR+bbb} ${BAZ+ccc}",
@@ -388,6 +403,15 @@ func TestProcessWithMatches(t *testing.T) {
 			expectedErr: true,
 			unmatched:   map[string]struct{}{"BAZ": {}},
 		},
+		{
+			input: "${BAZ:?}",
+			envs: map[string]string{
+				"FOO": "xxx",
+				"BAR": "",
+			},
+			expectedErr: true,
+			unmatched:   map[string]struct{}{"BAZ": {}},
+		},
 
 		{
 			input: "${FOO=aaa}",
@@ -405,6 +429,11 @@ func TestProcessWithMatches(t *testing.T) {
 			},
 			expectedErr: true,
 		},
+		{
+			input:       "${FOO=}",
+			envs:        map[string]string{},
+			expectedErr: true,
+		},
 		{
 			// special characters in regular expressions
 			// } needs to be escaped so it doesn't match the
@@ -426,12 +455,42 @@ func TestProcessWithMatches(t *testing.T) {
 			expected: "y",
 			matches:  map[string]struct{}{"FOO": {}},
 		},
+		{
+			input:    "${FOO#*}",
+			envs:     map[string]string{"FOO": "xxyy"},
+			expected: "xxyy",
+			matches:  map[string]struct{}{"FOO": {}},
+		},
+		{
+			input:    "${FOO#$BAR}",
+			envs:     map[string]string{"FOO": "xxyy", "BAR": "x"},
+			expected: "xyy",
+			matches:  map[string]struct{}{"FOO": {}, "BAR": {}},
+		},
+		{
+			input:    "${FOO#$BAR}",
+			envs:     map[string]string{"FOO": "xxyy", "BAR": ""},
+			expected: "xxyy",
+			matches:  map[string]struct{}{"FOO": {}, "BAR": {}},
+		},
+		{
+			input:    "${FOO#}",
+			envs:     map[string]string{"FOO": "xxyy"},
+			expected: "xxyy",
+			matches:  map[string]struct{}{"FOO": {}},
+		},
 		{
 			input:    "${FOO##*x}",
 			envs:     map[string]string{"FOO": "xxyy"},
 			expected: "yy",
 			matches:  map[string]struct{}{"FOO": {}},
 		},
+		{
+			input:    "${FOO##}",
+			envs:     map[string]string{"FOO": "xxyy"},
+			expected: "xxyy",
+			matches:  map[string]struct{}{"FOO": {}},
+		},
 		{
 			input:    "${FOO#?\\?}",
 			envs:     map[string]string{"FOO": "???y"},
@@ -451,6 +510,18 @@ func TestProcessWithMatches(t *testing.T) {
 			expected: "a",
 			matches:  map[string]struct{}{"FOO": {}},
 		},
+		{
+			input:    "${FOO%}",
+			envs:     map[string]string{"FOO": "xxyy"},
+			expected: "xxyy",
+			matches:  map[string]struct{}{"FOO": {}},
+		},
+		{
+			input:    "${FOO%%$BAR}",
+			envs:     map[string]string{"FOO": "xxyy", "BAR": ""},
+			expected: "xxyy",
+			matches:  map[string]struct{}{"FOO": {}, "BAR": {}},
+		},
 		{
 			// test: wildcards
 			input:    "${FOO/$NEEDLE/.} - ${FOO//$NEEDLE/.}",
@@ -484,6 +555,38 @@ func TestProcessWithMatches(t *testing.T) {
 			expected: "\\/tmp\\/foo.txt",
 			matches:  map[string]struct{}{"FOO": {}},
 		},
+
+		// Following cases with empty/partial values are currently not
+		// guaranteed behavior. Tests are provided to make sure partial
+		// input does not cause runtime error.
+		{
+			input:    "${FOO/$BAR/ww}",
+			envs:     map[string]string{"FOO": "xxyy", "BAR": ""},
+			expected: "wwxxyy",
+			matches:  map[string]struct{}{"FOO": {}, "BAR": {}},
+		},
+		{
+			input:       "${FOO//ww}",
+			envs:        map[string]string{"FOO": "xxyy"},
+			expectedErr: true,
+		},
+		{
+			input:       "${FOO//}",
+			envs:        map[string]string{"FOO": "xxyy"},
+			expectedErr: true,
+		},
+		{
+			input:    "${FOO///}",
+			envs:     map[string]string{"FOO": "xxyy"},
+			expected: "xxyy",
+			matches:  map[string]struct{}{"FOO": {}},
+		},
+		{
+			input:     "${FOO///}",
+			envs:      map[string]string{},
+			expected:  "",
+			unmatched: map[string]struct{}{"FOO": {}},
+		},
 	}
 
 	for _, c := range tc {
@@ -500,12 +603,12 @@ func TestProcessWithMatches(t *testing.T) {
 			require.NoError(t, err)
 			require.Equal(t, c.expected, w)
 
-			require.Equal(t, len(c.matches), len(matches))
+			require.Len(t, matches, len(c.matches), c.matches)
 			for k := range c.matches {
 				require.Contains(t, matches, k)
 			}
 
-			require.Equal(t, len(c.unmatched), len(unmatched))
+			require.Len(t, unmatched, len(c.unmatched), c.unmatched)
 			for k := range c.unmatched {
 				require.Contains(t, unmatched, k)
 			}