Skip to content

Commit faaa836

Browse files
tonistiigitonistiigi
authored
Merge pull request moby#3611 from jedevc/posix-variable-substitution
Allow POSIX variable substitution to remove optional colon
2 parents 9ea86dd + 580eb93 commit faaa836

File tree

2 files changed

+177
-69
lines changed

2 files changed

+177
-69
lines changed

‎frontend/dockerfile/shell/lex.go

Lines changed: 28 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -335,39 +335,23 @@ func (sw *shellWord) processDollar() (string, error) {
335335
}
336336
name := sw.processName()
337337
ch := sw.scanner.Next()
338+
chs := string(ch)
339+
nullIsUnset := false
340+
338341
switch ch {
339342
case '}':
340343
// Normal ${xx} case
341-
value, found := sw.getEnv(name)
342-
if !found && sw.skipUnsetEnv {
344+
value, set := sw.getEnv(name)
345+
if !set && sw.skipUnsetEnv {
343346
return fmt.Sprintf("${%s}", name), nil
344347
}
345348
return value, nil
346-
case '?':
347-
word, _, err := sw.processStopOn('}')
348-
if err != nil {
349-
if sw.scanner.Peek() == scanner.EOF {
350-
return "", errors.New("syntax error: missing '}'")
351-
}
352-
return "", err
353-
}
354-
newValue, found := sw.getEnv(name)
355-
if !found {
356-
if sw.skipUnsetEnv {
357-
return fmt.Sprintf("${%s?%s}", name, word), nil
358-
}
359-
message := "is not allowed to be unset"
360-
if word != "" {
361-
message = word
362-
}
363-
return "", errors.Errorf("%s: %s", name, message)
364-
}
365-
return newValue, nil
366349
case ':':
367-
// Special ${xx:...} format processing
368-
// Yes it allows for recursive $'s in the ... spot
369-
modifier := sw.scanner.Next()
370-
350+
nullIsUnset = true
351+
ch = sw.scanner.Next()
352+
chs += string(ch)
353+
fallthrough
354+
case '+', '-', '?':
371355
word, _, err := sw.processStopOn('}')
372356
if err != nil {
373357
if sw.scanner.Peek() == scanner.EOF {
@@ -378,53 +362,44 @@ func (sw *shellWord) processDollar() (string, error) {
378362

379363
// Grab the current value of the variable in question so we
380364
// can use it to determine what to do based on the modifier
381-
newValue, found := sw.getEnv(name)
382-
383-
switch modifier {
384-
case '+':
385-
if newValue != "" {
386-
newValue = word
387-
}
388-
if !found && sw.skipUnsetEnv {
389-
return fmt.Sprintf("${%s:%s%s}", name, string(modifier), word), nil
390-
}
391-
return newValue, nil
365+
value, set := sw.getEnv(name)
366+
if sw.skipUnsetEnv && !set {
367+
return fmt.Sprintf("${%s%s%s}", name, chs, word), nil
368+
}
392369

370+
switch ch {
393371
case '-':
394-
if newValue == "" {
395-
newValue = word
372+
if !set || (nullIsUnset && value == "") {
373+
return word, nil
396374
}
397-
if !found && sw.skipUnsetEnv {
398-
return fmt.Sprintf("${%s:%s%s}", name, string(modifier), word), nil
375+
return value, nil
376+
case '+':
377+
if !set || (nullIsUnset && value == "") {
378+
return "", nil
399379
}
400-
401-
return newValue, nil
402-
380+
return word, nil
403381
case '?':
404-
if !found {
405-
if sw.skipUnsetEnv {
406-
return fmt.Sprintf("${%s:%s%s}", name, string(modifier), word), nil
407-
}
382+
if !set {
408383
message := "is not allowed to be unset"
409384
if word != "" {
410385
message = word
411386
}
412387
return "", errors.Errorf("%s: %s", name, message)
413388
}
414-
if newValue == "" {
389+
if nullIsUnset && value == "" {
415390
message := "is not allowed to be empty"
416391
if word != "" {
417392
message = word
418393
}
419394
return "", errors.Errorf("%s: %s", name, message)
420395
}
421-
return newValue, nil
422-
396+
return value, nil
423397
default:
424-
return "", errors.Errorf("unsupported modifier (%c) in substitution", modifier)
398+
return "", errors.Errorf("unsupported modifier (%s) in substitution", chs)
425399
}
400+
default:
401+
return "", errors.Errorf("unsupported modifier (%s) in substitution", chs)
426402
}
427-
return "", errors.Errorf("missing ':' in substitution")
428403
}
429404

430405
func (sw *shellWord) processName() string {

‎frontend/dockerfile/shell/lex_test.go

Lines changed: 149 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -226,24 +226,157 @@ func TestGetEnv(t *testing.T) {
226226
func TestProcessWithMatches(t *testing.T) {
227227
shlex := NewLex('\\')
228228

229-
w, matches, err := shlex.ProcessWordWithMatches("foo ${BAR} ${UNUSED}", map[string]string{
230-
"ANOTHER": "bar",
231-
"BAR": "baz",
232-
})
233-
require.NoError(t, err)
234-
require.Equal(t, "foo baz ", w)
235-
236-
require.Equal(t, 1, len(matches))
237-
_, ok := matches["BAR"]
238-
require.True(t, ok)
229+
tc := []struct {
230+
input string
231+
envs map[string]string
232+
expected string
233+
expectedErr bool
234+
matches map[string]struct{}
235+
}{
236+
{
237+
input: "x",
238+
envs: map[string]string{"DUMMY": "dummy"},
239+
expected: "x",
240+
matches: nil,
241+
},
242+
{
243+
input: "x ${UNUSED}",
244+
envs: map[string]string{"DUMMY": "dummy"},
245+
expected: "x ",
246+
matches: nil,
247+
},
248+
{
249+
input: "x ${FOO}",
250+
envs: map[string]string{"FOO": "y"},
251+
expected: "x y",
252+
matches: map[string]struct{}{"FOO": {}},
253+
},
254+
255+
{
256+
input: "${FOO-aaa} ${BAR-bbb} ${BAZ-ccc}",
257+
envs: map[string]string{
258+
"FOO": "xxx",
259+
"BAR": "",
260+
},
261+
expected: "xxx ccc",
262+
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
263+
},
264+
{
265+
input: "${FOO:-aaa} ${BAR:-bbb} ${BAZ:-ccc}",
266+
envs: map[string]string{
267+
"FOO": "xxx",
268+
"BAR": "",
269+
},
270+
expected: "xxx bbb ccc",
271+
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
272+
},
273+
274+
{
275+
input: "${FOO+aaa} ${BAR+bbb} ${BAZ+ccc}",
276+
envs: map[string]string{
277+
"FOO": "xxx",
278+
"BAR": "",
279+
},
280+
expected: "aaa bbb ",
281+
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
282+
},
283+
{
284+
input: "${FOO:+aaa} ${BAR:+bbb} ${BAZ:+ccc}",
285+
envs: map[string]string{
286+
"FOO": "xxx",
287+
"BAR": "",
288+
},
289+
expected: "aaa ",
290+
matches: map[string]struct{}{"FOO": {}, "BAR": {}},
291+
},
292+
293+
{
294+
input: "${FOO?aaa}",
295+
envs: map[string]string{
296+
"FOO": "xxx",
297+
"BAR": "",
298+
},
299+
expected: "xxx",
300+
matches: map[string]struct{}{"FOO": {}},
301+
},
302+
{
303+
input: "${BAR?bbb}",
304+
envs: map[string]string{
305+
"FOO": "xxx",
306+
"BAR": "",
307+
},
308+
expected: "",
309+
matches: map[string]struct{}{"BAR": {}},
310+
},
311+
{
312+
input: "${BAZ?ccc}",
313+
envs: map[string]string{
314+
"FOO": "xxx",
315+
"BAR": "",
316+
},
317+
expectedErr: true,
318+
},
319+
{
320+
input: "${FOO:?aaa}",
321+
envs: map[string]string{
322+
"FOO": "xxx",
323+
"BAR": "",
324+
},
325+
expected: "xxx",
326+
matches: map[string]struct{}{"FOO": {}},
327+
},
328+
{
329+
input: "${BAR:?bbb}",
330+
envs: map[string]string{
331+
"FOO": "xxx",
332+
"BAR": "",
333+
},
334+
expectedErr: true,
335+
},
336+
{
337+
input: "${BAZ:?ccc}",
338+
envs: map[string]string{
339+
"FOO": "xxx",
340+
"BAR": "",
341+
},
342+
expectedErr: true,
343+
},
344+
345+
{
346+
input: "${FOO=aaa}",
347+
envs: map[string]string{
348+
"FOO": "xxx",
349+
"BAR": "",
350+
},
351+
expectedErr: true,
352+
},
353+
{
354+
input: "${FOO=:aaa}",
355+
envs: map[string]string{
356+
"FOO": "xxx",
357+
"BAR": "",
358+
},
359+
expectedErr: true,
360+
},
361+
}
239362

240-
w, matches, err = shlex.ProcessWordWithMatches("foo ${BAR:-abc} ${UNUSED}", map[string]string{
241-
"ANOTHER": "bar",
242-
})
243-
require.NoError(t, err)
244-
require.Equal(t, "foo abc ", w)
363+
for _, c := range tc {
364+
c := c
365+
t.Run(c.input, func(t *testing.T) {
366+
w, matches, err := shlex.ProcessWordWithMatches(c.input, c.envs)
367+
if c.expectedErr {
368+
require.Error(t, err)
369+
return
370+
}
371+
require.NoError(t, err)
372+
require.Equal(t, c.expected, w)
245373

246-
require.Equal(t, 0, len(matches))
374+
require.Equal(t, len(c.matches), len(matches))
375+
for k := range c.matches {
376+
require.Contains(t, matches, k)
377+
}
378+
})
379+
}
247380
}
248381

249382
func TestProcessWithMatchesPlatform(t *testing.T) {

0 commit comments

Comments
 (0)