Skip to content

Commit 0ac3e71

Browse files
committed
MINOR: yml: support inline maps and reject scalar exit-code-map
The parser only knew inline lists, so exit-code-map: {SIGTERM: 0} parsed as a scalar string. StringMap() on a scalar returns an empty map and parseProcess guards on len > 0, so the whole remap table was silently dropped: the config loaded clean and on-failure fired on the supposedly remapped exit code. Teach parseScalar the flow-mapping form, recursively and under the same maxParseDepth cap as block nesting. splitCSV now tracks []/{} depth so commas inside nested structures or quotes do not split entries, and inline maps get the same strictness as block maps: missing colons and duplicate keys fail with line numbers. Sequence items written as inline maps parse as mappings too. Also validate exit-code-map at load: a scalar or list value is now a config error naming the process and the expected form, instead of a silent no-op. An absent or empty key stays legal. environment and signal-rewrite gain the inline form for free via the shared parser.
1 parent 41111ce commit 0ac3e71

5 files changed

Lines changed: 287 additions & 12 deletions

File tree

documentation/exit-code-map/README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ processes:
1919
```
2020
2121
- `exit-code-map: {17: 0}` — an exit code of 17 is remapped to 0. Keys may be
22-
integers or signal names (e.g. `SIGTERM: 0` maps the shell's 143).
22+
integers or signal names (e.g. `SIGTERM: 0` maps the shell's 143). The map
23+
can be written indented (as above) or inline: `exit-code-map: {17: 0}`.
24+
A value that is not a key-value map (e.g. a bare scalar) is a config error.
2325
- Because the remap happens first, the remapped 0 makes the exit a **success**,
2426
so `on-failure: shutdown` never fires.
2527
- `on-success: ignore` — the success exit does not shut gopherd down either;

internal/yml/config.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -437,6 +437,9 @@ func parseProcess(n *Node, env map[string]string) (service.Process, error) {
437437
// "TERM"); signal names map to the shell convention 128+signum, matching
438438
// what waitStatusCode reports for signal-terminated children. Mixed forms
439439
// are fine.
440+
if err := requireMapping(n, "exit-code-map", p.Name, p.Command); err != nil {
441+
return p, err
442+
}
440443
if raw := n.Get("exit-code-map").StringMap(); len(raw) > 0 {
441444
p.ExitCodeMap = make(map[int]int, len(raw))
442445
for k, v := range raw {
@@ -540,6 +543,25 @@ func parseProcess(n *Node, env map[string]string) (service.Process, error) {
540543
// ({{file}}, {{.VAR}}). {{cpu}}/{{mem}} expand to integers, so excluded.
541544
var argSecretTemplateRe = regexp.MustCompile(`\{\{\s*(?:file\b|\.)`)
542545

546+
// requireMapping rejects a value that cannot hold a key-value table. A scalar
547+
// or a list parses to an empty map, so without this the whole setting would be
548+
// silently ignored until a child exits. An absent key, or a bare "key:" with no
549+
// entries, stays legal.
550+
func requireMapping(n *Node, key, procName, command string) error {
551+
v := n.Get(key)
552+
if v == nil || v.kind == kindMapping {
553+
return nil
554+
}
555+
if procName == "" {
556+
procName = command
557+
}
558+
got := fmt.Sprintf("%q", v.String())
559+
if v.kind == kindSequence {
560+
got = "a list"
561+
}
562+
return fmt.Errorf("process %q: %s must be a key-value map, either indented or inline as {SIGTERM: 0}; got %s", procName, key, got)
563+
}
564+
543565
// parseExitCode accepts a decimal exit code ("143") or a signal name
544566
// ("SIGTERM", "TERM"), returning the numeric exit status. Signal names use
545567
// the shell convention 128+signum, matching what waitStatusCode reports for

internal/yml/config_test.go

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,3 +1210,69 @@ func TestArgSecretTemplateRe(t *testing.T) {
12101210
}
12111211
}
12121212
}
1213+
1214+
// The inline flow form must produce the same map as the block form, so a
1215+
// one-line exit-code-map is not silently dropped.
1216+
func TestLoadExitCodeMapInlineForm(t *testing.T) {
1217+
t.Parallel()
1218+
dir := t.TempDir()
1219+
cfgPath := filepath.Join(dir, "em.yml")
1220+
os.WriteFile(cfgPath, []byte(`
1221+
processes:
1222+
- name: app
1223+
command: /bin/app
1224+
exit-code-map: {SIGKILL: 0, SIGTERM: 0, 42: 7}
1225+
`), 0o644)
1226+
cfg, err := Load(cfgPath)
1227+
if err != nil {
1228+
t.Fatalf("Load: %v", err)
1229+
}
1230+
got := cfg.Processes[0].ExitCodeMap
1231+
if got[137] != 0 || got[143] != 0 || got[42] != 7 || len(got) != 3 {
1232+
t.Errorf("ExitCodeMap = %v", got)
1233+
}
1234+
}
1235+
1236+
// A scalar under exit-code-map cannot be a remap table. Rejecting it at load
1237+
// keeps a typo from silently disabling the remap until a child exits.
1238+
func TestLoadExitCodeMapRejectsScalar(t *testing.T) {
1239+
t.Parallel()
1240+
for _, val := range []string{"17", "yes", "[143, 137]"} {
1241+
dir := t.TempDir()
1242+
cfgPath := filepath.Join(dir, "em.yml")
1243+
os.WriteFile(cfgPath, []byte(`
1244+
processes:
1245+
- name: app
1246+
command: /bin/app
1247+
exit-code-map: `+val+`
1248+
`), 0o644)
1249+
_, err := Load(cfgPath)
1250+
if err == nil {
1251+
t.Errorf("exit-code-map: %s: expected error, got nil", val)
1252+
continue
1253+
}
1254+
if !strings.Contains(err.Error(), "exit-code-map") {
1255+
t.Errorf("exit-code-map: %s: error %q does not name the key", val, err.Error())
1256+
}
1257+
}
1258+
}
1259+
1260+
// An absent exit-code-map stays absent; the new validation must not turn a
1261+
// missing key into an error.
1262+
func TestLoadExitCodeMapAbsent(t *testing.T) {
1263+
t.Parallel()
1264+
dir := t.TempDir()
1265+
cfgPath := filepath.Join(dir, "em.yml")
1266+
os.WriteFile(cfgPath, []byte(`
1267+
processes:
1268+
- name: app
1269+
command: /bin/app
1270+
`), 0o644)
1271+
cfg, err := Load(cfgPath)
1272+
if err != nil {
1273+
t.Fatalf("Load: %v", err)
1274+
}
1275+
if cfg.Processes[0].ExitCodeMap != nil {
1276+
t.Errorf("ExitCodeMap = %v, want nil", cfg.Processes[0].ExitCodeMap)
1277+
}
1278+
}

internal/yml/parser.go

Lines changed: 67 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,10 @@ func parseMapping(lines []rawLine, pos, minIndent, depth int) (*Node, int, error
204204
rest := strings.TrimSpace(line.text[colonIdx+1:])
205205

206206
if rest != "" {
207-
val := parseScalar(rest)
207+
val, err := parseScalar(rest, depth)
208+
if err != nil {
209+
return nil, pos, fmt.Errorf("line %d: %w", line.num, err)
210+
}
208211
m.mapping = append(m.mapping, MapEntry{Key: key, Val: val})
209212
pos++
210213
} else {
@@ -248,8 +251,12 @@ func parseSequence(lines []rawLine, pos, seqIndent, depth int) (*Node, int, erro
248251
pos++
249252
}
250253

251-
if len(itemLines) == 1 && isScalarSeqItem(itemText) {
252-
seq.sequence = append(seq.sequence, parseScalar(itemText))
254+
if len(itemLines) == 1 && isInlineSeqItem(itemText) {
255+
item, err := parseScalar(itemText, depth)
256+
if err != nil {
257+
return nil, pos, fmt.Errorf("line %d: %w", line.num, err)
258+
}
259+
seq.sequence = append(seq.sequence, item)
253260
continue
254261
}
255262
item, _, err := parseBlock(itemLines, 0, itemIndent-1, depth+1)
@@ -262,20 +269,30 @@ func parseSequence(lines []rawLine, pos, seqIndent, depth int) (*Node, int, erro
262269
return seq, pos, nil
263270
}
264271

265-
// isScalarSeqItem reports whether a single-line sequence item is a scalar:
266-
// not a nested sequence, and either colon-free or fully quoted (so quoted
267-
// values containing ": ", e.g. JSON blobs, stay one scalar).
268-
func isScalarSeqItem(s string) bool {
272+
// isInlineSeqItem reports whether a single-line sequence item can be handled
273+
// by parseScalar: not a nested sequence, and either an inline map, colon-free,
274+
// or fully quoted (so quoted values containing ": ", e.g. JSON blobs, stay one
275+
// scalar).
276+
func isInlineSeqItem(s string) bool {
269277
if strings.HasPrefix(s, "- ") {
270278
return false
271279
}
280+
if strings.HasPrefix(s, "{") && strings.HasSuffix(s, "}") {
281+
return true
282+
}
272283
if findColon(s) < 0 {
273284
return true
274285
}
275286
return len(s) >= 2 && (s[0] == '\'' || s[0] == '"') && s[len(s)-1] == s[0]
276287
}
277288

278-
func parseScalar(s string) *Node {
289+
// parseScalar parses an inline value: an inline list ("[a, b]"), an inline
290+
// map ("{a: 1, b: 2}"), or a plain scalar. depth bounds brace recursion so a
291+
// malformed config cannot exhaust the PID 1 stack.
292+
func parseScalar(s string, depth int) (*Node, error) {
293+
if depth > maxParseDepth {
294+
return nil, fmt.Errorf("YAML nesting exceeds maximum depth of %d", maxParseDepth)
295+
}
279296
if strings.HasPrefix(s, "[") && strings.HasSuffix(s, "]") {
280297
inner := s[1 : len(s)-1]
281298
items := splitCSV(inner)
@@ -285,9 +302,41 @@ func parseScalar(s string) *Node {
285302
item = unquote(item)
286303
seq.sequence = append(seq.sequence, &Node{kind: kindScalar, scalar: item})
287304
}
288-
return seq
305+
return seq, nil
306+
}
307+
if strings.HasPrefix(s, "{") && strings.HasSuffix(s, "}") {
308+
return parseInlineMap(s[1:len(s)-1], depth)
309+
}
310+
return &Node{kind: kindScalar, scalar: unquote(strings.TrimSpace(s))}, nil
311+
}
312+
313+
// parseInlineMap parses the body of a flow mapping ("a: 1, b: 2"). Entries are
314+
// validated like block keys, so a malformed entry or a duplicate key fails at
315+
// load instead of silently yielding an empty map.
316+
func parseInlineMap(inner string, depth int) (*Node, error) {
317+
m := &Node{kind: kindMapping}
318+
seen := make(map[string]bool)
319+
for _, entry := range splitCSV(inner) {
320+
entry = strings.TrimSpace(entry)
321+
if entry == "" {
322+
continue
323+
}
324+
colonIdx := findColon(entry)
325+
if colonIdx < 0 {
326+
return nil, fmt.Errorf("expected 'key: value' in inline map, got %q", entry)
327+
}
328+
key := unquote(strings.TrimSpace(entry[:colonIdx]))
329+
if seen[key] {
330+
return nil, fmt.Errorf("duplicate key %q in inline map", key)
331+
}
332+
seen[key] = true
333+
val, err := parseScalar(strings.TrimSpace(entry[colonIdx+1:]), depth+1)
334+
if err != nil {
335+
return nil, err
336+
}
337+
m.mapping = append(m.mapping, MapEntry{Key: key, Val: val})
289338
}
290-
return &Node{kind: kindScalar, scalar: unquote(strings.TrimSpace(s))}
339+
return m, nil
291340
}
292341

293342
func splitCSV(s string) []string {
@@ -296,6 +345,7 @@ func splitCSV(s string) []string {
296345
inQuote := false
297346
quoteChar := byte(0)
298347
escaped := false
348+
depth := 0 // nesting of [] and {}; commas only split at depth 0
299349
for i := range len(s) {
300350
c := s[i]
301351
if escaped {
@@ -323,7 +373,13 @@ func splitCSV(s string) []string {
323373
current.WriteByte(c)
324374
continue
325375
}
326-
if c == ',' {
376+
switch c {
377+
case '[', '{':
378+
depth++
379+
case ']', '}':
380+
depth--
381+
}
382+
if c == ',' && depth == 0 {
327383
parts = append(parts, current.String())
328384
current.Reset()
329385
continue

internal/yml/parser_test.go

Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,3 +484,132 @@ func TestParseRejectsDeepNesting(t *testing.T) {
484484
t.Errorf("error %q does not mention nesting depth limit", err.Error())
485485
}
486486
}
487+
488+
func TestParseInlineMap(t *testing.T) {
489+
t.Parallel()
490+
n, err := Parse([]byte(`exit-code-map: {SIGKILL: 0, SIGTERM: 0}`))
491+
if err != nil {
492+
t.Fatal(err)
493+
}
494+
got := n.Get("exit-code-map").StringMap()
495+
if got["SIGKILL"] != "0" || got["SIGTERM"] != "0" || len(got) != 2 {
496+
t.Errorf("StringMap() = %v", got)
497+
}
498+
}
499+
500+
func TestParseInlineMapEmpty(t *testing.T) {
501+
t.Parallel()
502+
n, err := Parse([]byte(`env: {}`))
503+
if err != nil {
504+
t.Fatal(err)
505+
}
506+
got := n.Get("env").StringMap()
507+
if got == nil || len(got) != 0 {
508+
t.Errorf("StringMap() = %v, want empty non-nil map", got)
509+
}
510+
}
511+
512+
// Values may contain commas and colons when quoted; splitting must respect
513+
// quotes so a URL or comma-bearing value stays one entry.
514+
func TestParseInlineMapQuotedValue(t *testing.T) {
515+
t.Parallel()
516+
n, err := Parse([]byte(`env: {LIST: "a, b", URL: "http://h:80"}`))
517+
if err != nil {
518+
t.Fatal(err)
519+
}
520+
got := n.Get("env").StringMap()
521+
if got["LIST"] != "a, b" {
522+
t.Errorf("LIST = %q, want %q", got["LIST"], "a, b")
523+
}
524+
if got["URL"] != "http://h:80" {
525+
t.Errorf("URL = %q", got["URL"])
526+
}
527+
}
528+
529+
// A nested inline map must not be split at the commas inside its braces.
530+
func TestParseInlineMapNested(t *testing.T) {
531+
t.Parallel()
532+
n, err := Parse([]byte(`a: {b: {c: 1, d: 2}, e: 3}`))
533+
if err != nil {
534+
t.Fatal(err)
535+
}
536+
inner := n.Get("a").Get("b").StringMap()
537+
if inner["c"] != "1" || inner["d"] != "2" || len(inner) != 2 {
538+
t.Errorf("inner = %v", inner)
539+
}
540+
if n.Get("a").Get("e").String() != "3" {
541+
t.Errorf("e = %q", n.Get("a").Get("e").String())
542+
}
543+
}
544+
545+
// An inline list nested in an inline map must survive comma splitting too.
546+
func TestParseInlineMapWithInlineList(t *testing.T) {
547+
t.Parallel()
548+
n, err := Parse([]byte(`a: {args: [x, y], n: 1}`))
549+
if err != nil {
550+
t.Fatal(err)
551+
}
552+
if got := n.Get("a").Get("args").Strings(); len(got) != 2 || got[0] != "x" || got[1] != "y" {
553+
t.Errorf("args = %v", got)
554+
}
555+
if n.Get("a").Get("n").String() != "1" {
556+
t.Errorf("n = %q", n.Get("a").Get("n").String())
557+
}
558+
}
559+
560+
func TestParseInlineMapMissingColon(t *testing.T) {
561+
t.Parallel()
562+
_, err := Parse([]byte(`a: {foo, bar}`))
563+
if err == nil {
564+
t.Fatal("expected error for inline map entry without a colon")
565+
}
566+
if !strings.Contains(err.Error(), "key: value") {
567+
t.Errorf("error %q does not explain the expected form", err.Error())
568+
}
569+
}
570+
571+
func TestParseInlineMapDuplicateKey(t *testing.T) {
572+
t.Parallel()
573+
_, err := Parse([]byte(`a: {x: 1, x: 2}`))
574+
if err == nil {
575+
t.Fatal("expected error for duplicate key in inline map")
576+
}
577+
if !strings.Contains(err.Error(), "duplicate key") {
578+
t.Errorf("error %q does not mention a duplicate key", err.Error())
579+
}
580+
}
581+
582+
// A PID 1 process must not blow its stack on an adversarial config, so
583+
// inline-map recursion is capped like block nesting is.
584+
func TestParseInlineMapRejectsDeepNesting(t *testing.T) {
585+
t.Parallel()
586+
const depth = 500 // well beyond maxParseDepth
587+
payload := "a: " + strings.Repeat("{b: ", depth) + "1" + strings.Repeat("}", depth)
588+
_, err := Parse([]byte(payload))
589+
if err == nil {
590+
t.Fatal("expected error for deeply nested inline map, got nil")
591+
}
592+
if !strings.Contains(err.Error(), "nesting exceeds maximum depth") {
593+
t.Errorf("error %q does not mention nesting depth limit", err.Error())
594+
}
595+
}
596+
597+
// A sequence item that is itself an inline map must parse as a mapping, not
598+
// be mangled by the block-mapping path.
599+
func TestParseInlineMapAsSequenceItem(t *testing.T) {
600+
t.Parallel()
601+
n, err := Parse([]byte("list:\n - {name: a, cmd: x}\n - {name: b, cmd: y}\n"))
602+
if err != nil {
603+
t.Fatal(err)
604+
}
605+
items := n.Get("list").Items()
606+
if len(items) != 2 {
607+
t.Fatalf("got %d items, want 2", len(items))
608+
}
609+
if items[0].Get("name").String() != "a" || items[0].Get("cmd").String() != "x" {
610+
t.Errorf("item 0 = %v", items[0].StringMap())
611+
}
612+
if items[1].Get("name").String() != "b" {
613+
t.Errorf("item 1 = %v", items[1].StringMap())
614+
}
615+
}

0 commit comments

Comments
 (0)