65ad319 fix(clipboard): address osc52 review findings · vev

M internal/usecase/copy/mode_test.go +25 -0

@@ -1,6 +1,7 @@
1	1		package copy
2	2
3	3		import (
	4	+	"bytes"
4	5		"encoding/base64"
5	6		"strings"
6	7		"testing"

@@ -366,6 +367,30 @@ func TestOSC52OversizedPayloadIsDeferred(t *testing.T) {
366	367		}
367	368		}
368	369
	370	+	func TestOSC52FromBase64(t *testing.T) {
	371	+	valid := base64.StdEncoding.EncodeToString([]byte("hello"))
	372	+	oversized := base64.StdEncoding.EncodeToString([]byte(strings.Repeat("x", OSC52MaxPayloadBytes+1)))
	373	+
	374	+	tests := []struct {
	375	+	name string
	376	+	in string
	377	+	want []byte
	378	+	}{
	379	+	{name: "valid", in: valid, want: []byte("\x1b]52;c;" + valid + "\x07")},
	380	+	{name: "empty payload", in: "", want: []byte("\x1b]52;c;\x07")},
	381	+	{name: "invalid base64", in: "not-valid-base64!!", want: nil},
	382	+	{name: "oversized decoded payload", in: oversized, want: nil},
	383	+	}
	384	+	for _, tt := range tests {
	385	+	t.Run(tt.name, func(t *testing.T) {
	386	+	got := OSC52FromBase64(tt.in)
	387	+	if !bytes.Equal(got, tt.want) {
	388	+	t.Fatalf("OSC52FromBase64(%q) = %q, want %q", tt.in, got, tt.want)
	389	+	}
	390	+	})
	391	+	}
	392	+	}
	393	+
369	394		func decodeOSC52Chunk(t *testing.T, chunk []byte) []byte {
370	395		t.Helper()
371	396		s := string(chunk)

@@ -1,6 +1,7 @@
1	package copy	1		package copy
2		2
3	import (	3		import (
		4	+	"bytes"
4	"encoding/base64"	5		"encoding/base64"
5	"strings"	6		"strings"
6	"testing"	7		"testing"

@@ -366,6 +367,30 @@ func TestOSC52OversizedPayloadIsDeferred(t *testing.T) {
366	}	367		}
367	}	368		}
368		369
		370	+	func TestOSC52FromBase64(t *testing.T) {
		371	+	valid := base64.StdEncoding.EncodeToString([]byte("hello"))
		372	+	oversized := base64.StdEncoding.EncodeToString([]byte(strings.Repeat("x", OSC52MaxPayloadBytes+1)))
		373	+
		374	+	tests := []struct {
		375	+	name string
		376	+	in string
		377	+	want []byte
		378	+	}{
		379	+	{name: "valid", in: valid, want: []byte("\x1b]52;c;" + valid + "\x07")},
		380	+	{name: "empty payload", in: "", want: []byte("\x1b]52;c;\x07")},
		381	+	{name: "invalid base64", in: "not-valid-base64!!", want: nil},
		382	+	{name: "oversized decoded payload", in: oversized, want: nil},
		383	+	}
		384	+	for _, tt := range tests {
		385	+	t.Run(tt.name, func(t *testing.T) {
		386	+	got := OSC52FromBase64(tt.in)
		387	+	if !bytes.Equal(got, tt.want) {
		388	+	t.Fatalf("OSC52FromBase64(%q) = %q, want %q", tt.in, got, tt.want)
		389	+	}
		390	+	})
		391	+	}
		392	+	}
		393	+
369	func decodeOSC52Chunk(t *testing.T, chunk []byte) []byte {	394		func decodeOSC52Chunk(t *testing.T, chunk []byte) []byte {
370	t.Helper()	395		t.Helper()
371	s := string(chunk)	396		s := string(chunk)

M internal/usecase/daemon/clipboard.go +16 -11


@@ -4,13 +4,16 @@ import (
4	4		scopy "github.com/bnema/vev/internal/usecase/copy"
5	5		)
6	6
7		-	// forwardClipboard re-emits an app-originated OSC 52 clipboard set request
8		-	// (already captured off the pane's screen while ptyReader held pane.mu, then
9		-	// handed here after that lock is released) to the session's attached client,
10		-	// if any. Invalid base64 or an oversized decoded payload is dropped silently,
11		-	// matching copy mode's own cap (scopy.OSC52MaxPayloadBytes). The caller must
12		-	// not hold pane.mu, tab.mu, or session.mu.
13		-	func (d Daemon) forwardClipboard(sess session, b64 string) {
	7	+	// forwardClipboardAsync re-emits an app-originated OSC 52 clipboard set
	8	+	// request (already captured off the pane's screen while ptyReader held pane.mu,
	9	+	// then handed here after that lock is released) to the session's attached
	10	+	// client, if any. Invalid base64 or an oversized decoded payload is dropped
	11	+	// silently, matching copy mode's own cap (scopy.OSC52MaxPayloadBytes). The
	12	+	// caller must not hold pane.mu, tab.mu, or session.mu.
	13	+	//
	14	+	// The bounded client send runs in its own goroutine so a slow or wedged client
	15	+	// cannot make the PTY reader stop draining child output.
	16	+	func (d Daemon) forwardClipboardAsync(sess session, b64 string) {
14	17		seq := scopy.OSC52FromBase64(b64)
15	18		if seq == nil {
16	19		return

@@ -23,8 +26,10 @@ func (d Daemon) forwardClipboard(sess session, b64 string) {
23	26		return
24	27		}
25	28
26		-	failed, err := d.boundedSendOutputErrTransport(ac, seq)
27		-	if err != nil {
28		-	d.detachOnSendError(sess, ac, failed)
29		-	}
	29	+	go func() {
	30	+	failed, err := d.boundedSendOutputErrTransport(ac, seq)
	31	+	if err != nil {
	32	+	d.detachOnSendError(sess, ac, failed)
	33	+	}
	34	+	}()
30	35		}


@@ -4,13 +4,16 @@ import (
4		scopy "github.com/bnema/vev/internal/usecase/copy"	4		scopy "github.com/bnema/vev/internal/usecase/copy"
5		)	5		)
6			6
7	-	// forwardClipboard re-emits an app-originated OSC 52 clipboard set request	7	+	// forwardClipboardAsync re-emits an app-originated OSC 52 clipboard set
8	-	// (already captured off the pane's screen while ptyReader held pane.mu, then	8	+	// request (already captured off the pane's screen while ptyReader held pane.mu,
9	-	// handed here after that lock is released) to the session's attached client,	9	+	// then handed here after that lock is released) to the session's attached
10	-	// if any. Invalid base64 or an oversized decoded payload is dropped silently,	10	+	// client, if any. Invalid base64 or an oversized decoded payload is dropped
11	-	// matching copy mode's own cap (scopy.OSC52MaxPayloadBytes). The caller must	11	+	// silently, matching copy mode's own cap (scopy.OSC52MaxPayloadBytes). The
12	-	// not hold pane.mu, tab.mu, or session.mu.	12	+	// caller must not hold pane.mu, tab.mu, or session.mu.
13	-	func (d Daemon) forwardClipboard(sess session, b64 string) {	13	+	//
			14	+	// The bounded client send runs in its own goroutine so a slow or wedged client
			15	+	// cannot make the PTY reader stop draining child output.
			16	+	func (d Daemon) forwardClipboardAsync(sess session, b64 string) {
14		seq := scopy.OSC52FromBase64(b64)	17		seq := scopy.OSC52FromBase64(b64)
15		if seq == nil {	18		if seq == nil {
16		return	19		return

@@ -23,8 +26,10 @@ func (d Daemon) forwardClipboard(sess session, b64 string) {
23		return	26		return
24		}	27		}
25			28
26	-	failed, err := d.boundedSendOutputErrTransport(ac, seq)	29	+	go func() {
27	-	if err != nil {	30	+	failed, err := d.boundedSendOutputErrTransport(ac, seq)
28	-	d.detachOnSendError(sess, ac, failed)	31	+	if err != nil {
29	-	}	32	+	d.detachOnSendError(sess, ac, failed)
			33	+	}
			34	+	}()
30		}	35		}

M internal/usecase/daemon/clipboard_test.go +5 -1


@@ -27,7 +27,11 @@ func chunkReadPTY(t testing.T, chunks ...[]byte) portsmocks.MockPTY {
27	27		return 0, io.EOF
28	28		}
29	29		n := copy(buf, chunks[0])
30		-	chunks = chunks[1:]
	30	+	if n == len(chunks[0]) {
	31	+	chunks = chunks[1:]
	32	+	} else {
	33	+	chunks[0] = chunks[0][n:]
	34	+	}
31	35		return n, nil
32	36		})
33	37		p.EXPECT().Write(mock.Anything).Return(0, nil).Maybe()

 			return 0, io.EOF
 		}
 		n := copy(buf, chunks[0])
-		chunks = chunks[1:]
+		if n == len(chunks[0]) {
+			chunks = chunks[1:]
+		} else {
+			chunks[0] = chunks[0][n:]
+		}
 		return n, nil
 	})
 	p.EXPECT().Write(mock.Anything).Return(0, nil).Maybe()

M internal/usecase/daemon/render.go +1 -1


@@ -78,7 +78,7 @@ func (d Daemon) ptyReader(sess session, tb tab, p pane) {
78	78		}
79	79		if len(clipboards) > 0 {
80	80		for _, b64 := range clipboards {
81		-	d.forwardClipboard(sess, b64)
	81	+	d.forwardClipboardAsync(sess, b64)
82	82		}
83	83		clipboards = clipboards[:0]
84	84		}


@@ -78,7 +78,7 @@ func (d Daemon) ptyReader(sess session, tb tab, p pane) {
78		}	78		}
79		if len(clipboards) > 0 {	79		if len(clipboards) > 0 {
80		for _, b64 := range clipboards {	80		for _, b64 := range clipboards {
81	-	d.forwardClipboard(sess, b64)	81	+	d.forwardClipboardAsync(sess, b64)
82		}	82		}
83		clipboards = clipboards[:0]	83		clipboards = clipboards[:0]
84		}	84		}

M pkg/vt/screen.go +11 -10


@@ -45,12 +45,13 @@ type Screen struct {
45	45		OnBell func()
46	46		// OnNotify is called synchronously from Write for explicit terminal
47	47		// notifications: OSC 9 (body only) and OSC 777 "notify" (title;body).
48		-	// All other OSC payloads remain discarded. Nil disables it.
	48	+	// Other non-clipboard OSC payloads remain discarded. Nil disables it.
49	49		OnNotify func(title, body string)
50	50		// OnClipboard is called synchronously from Write for a complete OSC 52
51		-	// clipboard set request from the child ("52;<selection>;<base64>"),
52		-	// receiving the raw base64 payload. Clipboard queries (data == "?") and
53		-	// malformed payloads are ignored and never invoke it. Nil disables it.
	51	+	// clipboard set request from the child. The OSC 52 selection field is
	52	+	// accepted but ignored; the callback receives only the raw base64 payload.
	53	+	// Clipboard queries (data == "?") and malformed payloads are ignored and
	54	+	// never invoke it. Nil disables it.
54	55		OnClipboard func(b64 string)
55	56
56	57		defaultFG renderer.RGB

@@ -634,14 +635,14 @@ func appendOSCColorComponent(dst []byte, c uint8) []byte {
634	635		// set requests (OSC 52) are acted on; titles and every other OSC are still
635	636		// discarded.
636	637		func (s *Screen) handleOSC(payload []byte) {
637		-	p := string(payload)
638		-	if strings.HasPrefix(p, "52;") {
639		-	s.handleOSC52(p[len("52;"):])
	638	+	if len(payload) >= len("52;") && payload[0] == '5' && payload[1] == '2' && payload[2] == ';' {
	639	+	s.handleOSC52(string(payload[len("52;"):]))
640	640		return
641	641		}
642	642		if s.OnNotify == nil {
643	643		return
644	644		}
	645	+	p := string(payload)
645	646		switch {
646	647		case strings.HasPrefix(p, "9;"):
647	648		if strings.HasPrefix(p, "9;4;") {

@@ -1050,11 +1051,11 @@ func resizeFrame(old renderer.Frame, newW, newH, cursorRow int, evict func([]ren
1050	1051		next := renderer.NewFrame(newW, newH)
1051	1052		shift := clamp(cursorRow-(newH-1), 0, max(old.Height-newH, 0))
1052	1053		if evict != nil {
1053		-	for y := 0; y < shift; y++ {
	1054	+	for y := range shift {
1054	1055		evict(old.Row(y))
1055	1056		}
1056	1057		}
1057		-	for dy := 0; dy < newH; dy++ {
	1058	+	for dy := range newH {
1058	1059		sy := dy + shift
1059	1060		if sy >= old.Height {
1060	1061		break

@@ -1154,7 +1155,7 @@ func (s *Screen) clearScreenMode(mode int) {
1154	1155		s.clampCursor()
1155	1156		switch mode {
1156	1157		case 1:
1157		-	for y := 0; y <= s.Row && y < s.Frame.Height; y++ {
	1158	+	for y := range min(s.Row+1, s.Frame.Height) {
1158	1159		end := s.Frame.Width
1159	1160		if y == s.Row {
1160	1161		end = min(s.Col+1, s.Frame.Width)

 	OnBell func()
 	// OnNotify is called synchronously from Write for explicit terminal
 	// notifications: OSC 9 (body only) and OSC 777 "notify" (title;body).
-	// All other OSC payloads remain discarded. Nil disables it.
+	// Other non-clipboard OSC payloads remain discarded. Nil disables it.
 	OnNotify func(title, body string)
 	// OnClipboard is called synchronously from Write for a complete OSC 52
-	// clipboard set request from the child ("52;<selection>;<base64>"),
+	// clipboard set request from the child. The OSC 52 selection field is
-	// receiving the raw base64 payload. Clipboard queries (data == "?") and
+	// accepted but ignored; the callback receives only the raw base64 payload.
-	// malformed payloads are ignored and never invoke it. Nil disables it.
+	// Clipboard queries (data == "?") and malformed payloads are ignored and
+	// never invoke it. Nil disables it.
 	OnClipboard func(b64 string)
 	defaultFG          renderer.RGB
 // set requests (OSC 52) are acted on; titles and every other OSC are still
 // discarded.
 func (s *Screen) handleOSC(payload []byte) {
-	p := string(payload)
+	if len(payload) >= len("52;") && payload[0] == '5' && payload[1] == '2' && payload[2] == ';' {
-	if strings.HasPrefix(p, "52;") {
+		s.handleOSC52(string(payload[len("52;"):]))
-		s.handleOSC52(p[len("52;"):])
 		return
 	}
 	if s.OnNotify == nil {
 		return
 	}
+	p := string(payload)
 	switch {
 	case strings.HasPrefix(p, "9;"):
 		if strings.HasPrefix(p, "9;4;") {
 	next := renderer.NewFrame(newW, newH)
 	shift := clamp(cursorRow-(newH-1), 0, max(old.Height-newH, 0))
 	if evict != nil {
-		for y := 0; y < shift; y++ {
+		for y := range shift {
 			evict(old.Row(y))
 		}
 	}
-	for dy := 0; dy < newH; dy++ {
+	for dy := range newH {
 		sy := dy + shift
 		if sy >= old.Height {
 			break
 	s.clampCursor()
 	switch mode {
 	case 1:
-		for y := 0; y <= s.Row && y < s.Frame.Height; y++ {
+		for y := range min(s.Row+1, s.Frame.Height) {
 			end := s.Frame.Width
 			if y == s.Row {
 				end = min(s.Col+1, s.Frame.Width)