diff --git a/cmd/sicd/main.go b/cmd/sicd/main.go index 89b852b..d1816fb 100644 --- a/cmd/sicd/main.go +++ b/cmd/sicd/main.go @@ -91,6 +91,29 @@ func readNetstringStream(r *bufio.Reader) ([]byte, bool) { return content, true } +// parseV2Content splits a v2 netstring's content into argv (a run of netstrings terminated by +// the empty netstring 0:,) and the trailing payload. Length-framed argv preserves argument +// boundaries and lets any byte (space, NUL, 0xFF) appear in an argument; readNetstringStream +// bounds each element and its length token, so a hostile content cannot OOM here. +func parseV2Content(content []byte) (argv [][]byte, payload []byte, ok bool) { + r := bufio.NewReader(bytes.NewReader(content)) + for { + ns, good := readNetstringStream(r) + if !good { + return nil, nil, false + } + if len(ns) == 0 { + break // the empty netstring 0:, terminates argv + } + argv = append(argv, ns) // readNetstringStream returns a fresh slice; no aliasing + } + rest, err := io.ReadAll(r) + if err != nil { + return nil, nil, false + } + return argv, rest, true +} + func waitForChild(cmd *exec.Cmd) int { if err := cmd.Wait(); err != nil { if exitErr, ok := err.(*exec.ExitError); ok { @@ -151,23 +174,23 @@ func runV2() { os.Exit(1) } - nulIdx := bytes.IndexByte(content, 0x00) - if nulIdx < 0 { - fmt.Fprintf(os.Stderr, "sicd: content missing NUL separator\n") + // v2 content = argv netstrings + the empty-netstring terminator 0:, + payload. Length-framed + // argv PRESERVES argument boundaries (sic's founding guarantee: `touch 'a b'` is ONE file) + // and lets ANY byte appear in an argument — unlike the old space-split. Everything after the + // terminator is the payload (a nested frame for the next hop, or empty). + argv, payload, ok := parseV2Content(content) + if !ok { + fmt.Fprintf(os.Stderr, "sicd: invalid v2 content (argv framing)\n") + os.Exit(1) + } + if len(argv) == 0 { + fmt.Fprintf(os.Stderr, "sicd: empty command\n") os.Exit(1) } - command := content[:nulIdx] - payload := content[nulIdx+1:] - - argv := bytes.Split(command, []byte(" ")) argvStr := make([]string, len(argv)) for i, a := range argv { argvStr[i] = string(a) } - if len(argvStr) == 0 { - fmt.Fprintf(os.Stderr, "sicd: empty command\n") - os.Exit(1) - } cmd := exec.Command(argvStr[0], argvStr[1:]...) stdinPipe, err := cmd.StdinPipe() diff --git a/cmd/sicd/sicd_test.go b/cmd/sicd/sicd_test.go index 045500a..5350ea7 100644 --- a/cmd/sicd/sicd_test.go +++ b/cmd/sicd/sicd_test.go @@ -75,7 +75,15 @@ func be32(n uint32) []byte { // frame builds one wire frame: magic, 4-byte BE length, netstring(command NUL payload). func frame(command, payload []byte) []byte { - ns := netstring(concat(command, []byte{0x00}, payload)) + // v2 content = argv netstrings + the empty-netstring terminator 0:, + payload. The command is + // split on spaces HERE (test-side convenience); the wire itself no longer space-splits — argv + // arrives length-framed so `touch 'a b'` keeps its boundary. + var content []byte + for _, a := range bytes.Split(command, []byte(" ")) { + content = concat(content, netstring(a)) + } + content = concat(content, []byte("0:,"), payload) + ns := netstring(content) return concat([]byte{0x00}, be32(uint32(len(ns))), ns) } @@ -590,3 +598,29 @@ func TestPumpStdinCleanEofReturnsChildStatus(t *testing.T) { t.Fatalf("a clean EOF is a legitimate end; must return the child's status 7, got %d", code) } } + +// v2FrameArgv builds a v2 frame from EXPLICIT argv (no space-split) — the whole point of the +// argv-netstring wire: each element is length-framed and rides untouched. +func v2FrameArgv(argv [][]byte, payload []byte) []byte { + var content []byte + for _, a := range argv { + content = concat(content, netstring(a)) + } + content = concat(content, []byte("0:,"), payload) + ns := netstring(content) + return concat([]byte{0x00}, be32(uint32(len(ns))), ns) +} + +func TestV2ArgvBoundaryPreserved(t *testing.T) { + // sic's founding guarantee, now on the v2 wire: an argument containing a space is ONE argv + // element, never re-split. Before the argv-netstring wire this failed — the space-split gave + // the script two args and printed "2|a". This is the regression that pins the fix. + script := shScript(t, `printf '%s|%s\n' "$#" "$1"`) + r := runSicd(t, v2FrameArgv([][]byte{[]byte(script), []byte("a b")}, nil)) + if r.code != 0 { + t.Fatalf("expected exit 0, got %d, stderr=%q", r.code, r.stderr) + } + if got, want := string(r.stdout), "1|a b\n"; got != want { + t.Fatalf("v2 argv boundary LOST: script saw %q, want %q", got, want) + } +}