tenseleyflow/shithub / e860cb1

Browse files

S12: pkt-line writer + git exec wrapper (CommandContext kill, bounded stderr drain)

Authored by mfwolffe <wolffemf@dukes.jmu.edu>
SHA
e860cb161642bc88b836be7618fe2ba72240eaaa
Parents
271ba33
Tree
917d95b

3 changed files

StatusFile+-
A internal/git/protocol/exec.go 113 0
A internal/git/protocol/pktline.go 61 0
A internal/git/protocol/protocol_test.go 108 0
internal/git/protocol/exec.goadded
@@ -0,0 +1,113 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+package protocol
4
+
5
+import (
6
+	"bytes"
7
+	"context"
8
+	"errors"
9
+	"io"
10
+	"os/exec"
11
+	"sync"
12
+	"time"
13
+)
14
+
15
+// stderrCap bounds the in-memory stderr drainer's buffer so a chatty
16
+// subprocess can't OOM us. Truncated stderr is fine — we only surface
17
+// it on error and a few KB is enough to identify the failure.
18
+const stderrCap = 16 * 1024
19
+
20
+// Service identifies which git plumbing program we're driving.
21
+type Service string
22
+
23
+const (
24
+	UploadPack  Service = "git-upload-pack"
25
+	ReceivePack Service = "git-receive-pack"
26
+)
27
+
28
+// Cmd builds an *exec.Cmd that drives the requested service against the
29
+// bare repo at gitDir. AdvertiseRefs flips on the --advertise-refs flag
30
+// used by the info/refs response. extraEnv is appended to os.Environ()
31
+// — the caller uses it to thread SHITHUB_USER_ID/etc. through git's
32
+// environment so post-receive hooks can read them.
33
+//
34
+// Subprocess lifecycle:
35
+//   - Killed on ctx cancel (Go ≥ 1.20: cmd.Cancel default kills with
36
+//     SIGKILL; we tighten to a 250ms WaitDelay so a stuck subprocess
37
+//     doesn't pin a worker forever).
38
+//   - Stderr is drained by the helper Drain() so the OS pipe buffer
39
+//     doesn't fill and deadlock the caller's stdout copy. Captured
40
+//     stderr is exposed via Stderr() on the returned Process.
41
+func Cmd(ctx context.Context, svc Service, gitDir string, advertiseRefs bool, extraEnv []string) *exec.Cmd {
42
+	args := []string{"--stateless-rpc"}
43
+	if advertiseRefs {
44
+		args = append(args, "--advertise-refs")
45
+	}
46
+	args = append(args, gitDir)
47
+	//nolint:gosec // G204: svc is an enum from a fixed set; gitDir is constructed via storage.RepoFS path validation.
48
+	cmd := exec.CommandContext(ctx, string(svc), args...)
49
+	cmd.Env = extraEnv
50
+	cmd.WaitDelay = 250 * time.Millisecond
51
+	return cmd
52
+}
53
+
54
+// DrainStderr starts a goroutine that copies stderr into a bounded ring
55
+// buffer; returns a function the caller invokes after Wait() to get the
56
+// captured bytes. The cap is `stderrCap`; further writes are silently
57
+// discarded.
58
+//
59
+// Always call this BEFORE Start() and call the returned closure AFTER
60
+// Wait() to avoid the deadlock where a verbose stderr fills the OS
61
+// pipe.
62
+func DrainStderr(cmd *exec.Cmd) func() []byte {
63
+	pipe, err := cmd.StderrPipe()
64
+	if err != nil {
65
+		// Stderr already wired (or another wiring error). Fall back to
66
+		// a no-op drain so callers don't have to special-case.
67
+		return func() []byte { return nil }
68
+	}
69
+	buf := &cappedBuffer{cap: stderrCap}
70
+	done := make(chan struct{})
71
+	go func() {
72
+		defer close(done)
73
+		_, _ = io.Copy(buf, pipe)
74
+	}()
75
+	return func() []byte {
76
+		<-done
77
+		return buf.Bytes()
78
+	}
79
+}
80
+
81
+// cappedBuffer is a bytes.Buffer that drops writes after the cap.
82
+type cappedBuffer struct {
83
+	mu  sync.Mutex
84
+	buf bytes.Buffer
85
+	cap int
86
+}
87
+
88
+func (c *cappedBuffer) Write(p []byte) (int, error) {
89
+	c.mu.Lock()
90
+	defer c.mu.Unlock()
91
+	free := c.cap - c.buf.Len()
92
+	if free <= 0 {
93
+		return len(p), nil // accept-and-drop
94
+	}
95
+	if len(p) > free {
96
+		_, _ = c.buf.Write(p[:free])
97
+		return len(p), nil
98
+	}
99
+	return c.buf.Write(p)
100
+}
101
+
102
+func (c *cappedBuffer) Bytes() []byte {
103
+	c.mu.Lock()
104
+	defer c.mu.Unlock()
105
+	out := make([]byte, c.buf.Len())
106
+	copy(out, c.buf.Bytes())
107
+	return out
108
+}
109
+
110
+// ErrSubprocessFailed is returned by Run when the git subprocess exits
111
+// non-zero. The captured stderr is wrapped via %s so the operator sees
112
+// it in logs without it being escaped through the chain.
113
+var ErrSubprocessFailed = errors.New("git subprocess exited non-zero")
internal/git/protocol/pktline.goadded
@@ -0,0 +1,61 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+// Package protocol carries the smart-HTTP and (later) smart-SSH bits of
4
+// the git wire protocol that we generate ourselves. The bulk of the
5
+// protocol — capability negotiation, want/have, multi-ack, side-band —
6
+// stays inside canonical `git`'s `upload-pack` and `receive-pack`. We
7
+// only emit the framing we need to wrap their advertise-refs output for
8
+// HTTP transport.
9
+package protocol
10
+
11
+import (
12
+	"fmt"
13
+	"io"
14
+)
15
+
16
+// MaxPktLine is git's hard limit for a single packet: 65520 bytes of
17
+// payload + 4 bytes of length prefix. Anything longer is malformed.
18
+const MaxPktLine = 65520
19
+
20
+// FlushPkt is the literal pkt-line sequence that says "no more
21
+// packets." git uses it to terminate sub-streams.
22
+const FlushPkt = "0000"
23
+
24
+// WritePkt writes one pkt-line packet — a 4-hex-digit length prefix
25
+// (covering payload + 4) followed by the payload. Exposed so the smart-
26
+// HTTP info/refs handler can prepend its `# service=...` advertisement.
27
+func WritePkt(w io.Writer, payload string) error {
28
+	if len(payload)+4 > MaxPktLine {
29
+		return fmt.Errorf("pkt-line: payload too long (%d > %d)", len(payload), MaxPktLine-4)
30
+	}
31
+	if _, err := fmt.Fprintf(w, "%04x%s", len(payload)+4, payload); err != nil {
32
+		return err
33
+	}
34
+	return nil
35
+}
36
+
37
+// WriteFlush emits the flush packet (`0000`). Marks the end of a
38
+// sub-stream like the service advertisement preamble.
39
+func WriteFlush(w io.Writer) error {
40
+	_, err := io.WriteString(w, FlushPkt)
41
+	return err
42
+}
43
+
44
+// WriteServiceAdvertisement writes the standard preamble that the smart-
45
+// HTTP info/refs response uses to tell git which service is being
46
+// advertised. The wire format is:
47
+//
48
+//	001e# service=git-upload-pack\n0000
49
+//	└┬┘└──────────┬─────────────┘└─┬─┘
50
+//	 │            │                └ flush
51
+//	 │            └ payload (trailing newline included)
52
+//	 └ length prefix (hex)
53
+//
54
+// Then `git upload-pack --advertise-refs --stateless-rpc <repo>` writes
55
+// its actual ref advertisement after this.
56
+func WriteServiceAdvertisement(w io.Writer, service string) error {
57
+	if err := WritePkt(w, "# service="+service+"\n"); err != nil {
58
+		return err
59
+	}
60
+	return WriteFlush(w)
61
+}
internal/git/protocol/protocol_test.goadded
@@ -0,0 +1,108 @@
1
+// SPDX-License-Identifier: AGPL-3.0-or-later
2
+
3
+package protocol_test
4
+
5
+import (
6
+	"bytes"
7
+	"context"
8
+	"os/exec"
9
+	"path/filepath"
10
+	"strings"
11
+	"testing"
12
+	"time"
13
+
14
+	"github.com/tenseleyFlow/shithub/internal/git/protocol"
15
+)
16
+
17
+func TestWritePkt_Format(t *testing.T) {
18
+	t.Parallel()
19
+	var buf bytes.Buffer
20
+	if err := protocol.WritePkt(&buf, "hi"); err != nil {
21
+		t.Fatalf("WritePkt: %v", err)
22
+	}
23
+	// 4-hex prefix = len(payload)+4 = 6 -> "0006"
24
+	if got := buf.String(); got != "0006hi" {
25
+		t.Fatalf("got %q, want %q", got, "0006hi")
26
+	}
27
+}
28
+
29
+func TestWritePkt_RejectsTooLong(t *testing.T) {
30
+	t.Parallel()
31
+	var buf bytes.Buffer
32
+	huge := strings.Repeat("a", protocol.MaxPktLine)
33
+	if err := protocol.WritePkt(&buf, huge); err == nil {
34
+		t.Fatal("expected error for over-length pkt")
35
+	}
36
+}
37
+
38
+func TestServiceAdvertisement(t *testing.T) {
39
+	t.Parallel()
40
+	var buf bytes.Buffer
41
+	if err := protocol.WriteServiceAdvertisement(&buf, "git-upload-pack"); err != nil {
42
+		t.Fatalf("WriteServiceAdvertisement: %v", err)
43
+	}
44
+	got := buf.String()
45
+	// Expected: "001e# service=git-upload-pack\n0000"
46
+	want := "001e# service=git-upload-pack\n0000"
47
+	if got != want {
48
+		t.Fatalf("got %q, want %q", got, want)
49
+	}
50
+}
51
+
52
+// TestCmd_StreamsAndDrainsStderr exercises the exec wrapper end-to-end:
53
+// init a bare repo, run upload-pack --advertise-refs against it, drain
54
+// stderr, and check stdout looks like a valid (empty) ref advertisement.
55
+func TestCmd_StreamsAndDrainsStderr(t *testing.T) {
56
+	t.Parallel()
57
+	gitDir := initBare(t)
58
+
59
+	cmd := protocol.Cmd(context.Background(), protocol.UploadPack, gitDir, true, nil)
60
+	stderr := protocol.DrainStderr(cmd)
61
+	out, err := cmd.Output()
62
+	if err != nil {
63
+		t.Fatalf("Cmd run: %v\nstderr: %s", err, stderr())
64
+	}
65
+	// First 4 chars are a hex length prefix; the body should mention
66
+	// either a ref or the no-refs sentinel git emits for unborn HEAD.
67
+	if len(out) < 4 {
68
+		t.Fatalf("output too short: %q", out)
69
+	}
70
+}
71
+
72
+// TestCmd_KillsOnContextCancel verifies that cancelling ctx kills the
73
+// subprocess promptly (Go's default cmd.Cancel sends SIGKILL).
74
+func TestCmd_KillsOnContextCancel(t *testing.T) {
75
+	t.Parallel()
76
+	ctx, cancel := context.WithCancel(context.Background())
77
+
78
+	// `git fetch` against a never-resolving URL hangs indefinitely;
79
+	// we'll use that as our long-running subprocess.
80
+	gitDir := initBare(t)
81
+	//nolint:gosec // G204: gitDir is t.TempDir.
82
+	cmd := exec.CommandContext(ctx, "git", "-C", gitDir, "fetch", "https://example.invalid/never-resolves")
83
+	cmd.WaitDelay = 250 * time.Millisecond
84
+	if err := cmd.Start(); err != nil {
85
+		t.Fatalf("Start: %v", err)
86
+	}
87
+
88
+	go func() {
89
+		time.Sleep(100 * time.Millisecond)
90
+		cancel()
91
+	}()
92
+	start := time.Now()
93
+	_ = cmd.Wait() // expect non-nil; we just care it returns
94
+	if elapsed := time.Since(start); elapsed > 5*time.Second {
95
+		t.Fatalf("subprocess took %s to die after cancel; expected <5s", elapsed)
96
+	}
97
+}
98
+
99
+func initBare(t *testing.T) string {
100
+	t.Helper()
101
+	root := t.TempDir()
102
+	gitDir := filepath.Join(root, "x.git")
103
+	//nolint:gosec // G204: t.TempDir.
104
+	if out, err := exec.Command("git", "init", "--bare", "--initial-branch=trunk", gitDir).CombinedOutput(); err != nil {
105
+		t.Fatalf("git init: %v\n%s", err, out)
106
+	}
107
+	return gitDir
108
+}