detached b4be9db

fix(graphics): close final review findings

commit
b4be9dbaaeec8b2461db1971e6ae0e06fd8e96ad
author
brice · · 2026-08-20 23:23 UTC
parent
d7da5a2
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
15 files · +327 -137
M graphics/graphics.go +3 -1
@@ -99,7 +99,9 @@ const (
9999 // AssetBlob is the caller-owned description of one encoded asset. AddAsset
100100 // copies Encoded before returning, so later caller mutations cannot alter a
101101 // scene or any snapshot. Width and Height describe the decoded pixel extent
102-// used for clipping and quota accounting.
102+// used for clipping and quota accounting. DecodedPixels is an optional
103+// compatibility declaration; when non-zero it must equal Width*Height, and
104+// accounting always uses the checked product of the dimensions.
103105 type AssetBlob struct {
104106 Encoded []byte
105107 Data []byte
@@ -99,7 +99,9 @@ const (
99 // AssetBlob is the caller-owned description of one encoded asset. AddAsset99 // AssetBlob is the caller-owned description of one encoded asset. AddAsset
100 // copies Encoded before returning, so later caller mutations cannot alter a100 // copies Encoded before returning, so later caller mutations cannot alter a
101 // scene or any snapshot. Width and Height describe the decoded pixel extent101 // scene or any snapshot. Width and Height describe the decoded pixel extent
102-// used for clipping and quota accounting.102+// used for clipping and quota accounting. DecodedPixels is an optional
103+// compatibility declaration; when non-zero it must equal Width*Height, and
104+// accounting always uses the checked product of the dimensions.
103 type AssetBlob struct {105 type AssetBlob struct {
104 Encoded []byte106 Encoded []byte
105 Data []byte107 Data []byte
M graphics/scene.go +6 -2
@@ -646,8 +646,12 @@ func decodedPixels(width, height int64, declared uint64) (uint64, bool) {
646646 return 0, false
647647 }
648648 computed := w * h
649- if declared != 0 {
650- return declared, true
649+ // DecodedPixels is retained as an optional compatibility field, but it is
650+ // only a valid declaration when it agrees with the dimensions. Resource
651+ // accounting must never trust a caller-supplied number that can undercount
652+ // the pixels represented by the asset.
653+ if declared != 0 && declared != computed {
654+ return 0, false
651655 }
652656 return computed, true
653657 }
@@ -646,8 +646,12 @@ func decodedPixels(width, height int64, declared uint64) (uint64, bool) {
646 return 0, false646 return 0, false
647 }647 }
648 computed := w * h648 computed := w * h
649- if declared != 0 {649+ // DecodedPixels is retained as an optional compatibility field, but it is
650- return declared, true650+ // only a valid declaration when it agrees with the dimensions. Resource
651+ // accounting must never trust a caller-supplied number that can undercount
652+ // the pixels represented by the asset.
653+ if declared != 0 && declared != computed {
654+ return 0, false
651 }655 }
652 return computed, true656 return computed, true
653 }657 }
M graphics/scene_test.go +16 -0
@@ -69,6 +69,22 @@ func TestAssetBlobOwnershipAndSnapshotLifecycle(t *testing.T) {
6969 }
7070 }
7171
72+func TestDecodedPixelAccountingRejectsForgedDeclarations(t *testing.T) {
73+ scene := NewScene(Limits{MaxEncodedBytes: 64, MaxDecodedPixels: 4})
74+ _, err := scene.AddAsset(AssetBlob{
75+ Encoded: []byte{1},
76+ Width: 2,
77+ Height: 2,
78+ DecodedPixels: 1,
79+ })
80+ if !errors.Is(err, ErrInvalidAsset) {
81+ t.Fatalf("forged decoded-pixel declaration error = %v", err)
82+ }
83+ if got := scene.Usage(); got != (Usage{}) {
84+ t.Fatalf("rejected asset changed usage: %#v", got)
85+ }
86+}
87+
7288 func TestAssetAndPlacementQuotasAreTransactional(t *testing.T) {
7389 scene := NewScene(Limits{
7490 MaxAssets: 1,
@@ -69,6 +69,22 @@ func TestAssetBlobOwnershipAndSnapshotLifecycle(t *testing.T) {
69 }69 }
70 }70 }
71 71
72+func TestDecodedPixelAccountingRejectsForgedDeclarations(t *testing.T) {
73+ scene := NewScene(Limits{MaxEncodedBytes: 64, MaxDecodedPixels: 4})
74+ _, err := scene.AddAsset(AssetBlob{
75+ Encoded: []byte{1},
76+ Width: 2,
77+ Height: 2,
78+ DecodedPixels: 1,
79+ })
80+ if !errors.Is(err, ErrInvalidAsset) {
81+ t.Fatalf("forged decoded-pixel declaration error = %v", err)
82+ }
83+ if got := scene.Usage(); got != (Usage{}) {
84+ t.Fatalf("rejected asset changed usage: %#v", got)
85+ }
86+}
87+
72 func TestAssetAndPlacementQuotasAreTransactional(t *testing.T) {88 func TestAssetAndPlacementQuotasAreTransactional(t *testing.T) {
73 scene := NewScene(Limits{89 scene := NewScene(Limits{
74 MaxAssets: 1,90 MaxAssets: 1,
M graphics_state.go +22 -19
@@ -48,35 +48,38 @@ func (g *screenGraphicsState) clearPlacements() {
4848 }
4949
5050 func (s *Screen) graphicsClearPlacements() {
51- if s != nil && s.graphics != nil {
51+ if s == nil {
52+ return
53+ }
54+ if s.graphics != nil {
5255 s.graphics.clearPlacements()
5356 }
57+ s.kittyPendingDisplay = nil
5458 }
5559
56-// GraphicsScene returns the active screen buffer's graphics scene, or nil
57-// before graphics has been used. Calling it never allocates graphics state.
58-func (s *Screen) GraphicsScene() *graphics.Scene {
59- if s == nil || s.graphics == nil {
60- return nil
60+func (s *Screen) abortKittyPendingDisplay() {
61+ if s == nil {
62+ return
63+ }
64+ s.kittyPendingDisplay = nil
65+ if s.graphics != nil && s.graphics.kitty != nil {
66+ s.graphics.kitty.AbortPendingUpload()
6167 }
62- return s.graphics.scene
6368 }
6469
65-// Graphics is a concise alias for GraphicsScene.
66-func (s *Screen) Graphics() *graphics.Scene { return s.GraphicsScene() }
67-
68-// GraphicsSession returns the active screen buffer's Kitty graphics adapter,
69-// or nil before graphics has been used.
70-func (s *Screen) GraphicsSession() *kittygraphics.Session {
71- if s == nil || s.graphics == nil {
72- return nil
70+func (s *Screen) abortAllKittyPending() {
71+ if s == nil {
72+ return
73+ }
74+ s.abortKittyPendingDisplay()
75+ if state := s.alternate; state != nil {
76+ state.kittyPendingDisplay = nil
77+ if state.graphics != nil && state.graphics.kitty != nil {
78+ state.graphics.kitty.AbortPendingUpload()
79+ }
7380 }
74- return s.graphics.kitty
7581 }
7682
77-// KittyGraphics is an adapter-named alias for GraphicsSession.
78-func (s *Screen) KittyGraphics() *kittygraphics.Session { return s.GraphicsSession() }
79-
8083 // GraphicsSnapshot returns an immutable snapshot of the active screen
8184 // buffer's graphics scene, or nil before graphics has been used.
8285 func (s *Screen) GraphicsSnapshot() *graphics.Snapshot {
@@ -48,35 +48,38 @@ func (g *screenGraphicsState) clearPlacements() {
48 }48 }
49 49
50 func (s *Screen) graphicsClearPlacements() {50 func (s *Screen) graphicsClearPlacements() {
51- if s != nil && s.graphics != nil {51+ if s == nil {
52+ return
53+ }
54+ if s.graphics != nil {
52 s.graphics.clearPlacements()55 s.graphics.clearPlacements()
53 }56 }
57+ s.kittyPendingDisplay = nil
54 }58 }
55 59
56-// GraphicsScene returns the active screen buffer's graphics scene, or nil60+func (s *Screen) abortKittyPendingDisplay() {
57-// before graphics has been used. Calling it never allocates graphics state.61+ if s == nil {
58-func (s *Screen) GraphicsScene() *graphics.Scene {62+ return
59- if s == nil || s.graphics == nil {63+ }
60- return nil64+ s.kittyPendingDisplay = nil
65+ if s.graphics != nil && s.graphics.kitty != nil {
66+ s.graphics.kitty.AbortPendingUpload()
61 }67 }
62- return s.graphics.scene
63 }68 }
64 69
65-// Graphics is a concise alias for GraphicsScene.70+func (s *Screen) abortAllKittyPending() {
66-func (s *Screen) Graphics() *graphics.Scene { return s.GraphicsScene() }71+ if s == nil {
67-72+ return
68-// GraphicsSession returns the active screen buffer's Kitty graphics adapter,73+ }
69-// or nil before graphics has been used.74+ s.abortKittyPendingDisplay()
70-func (s *Screen) GraphicsSession() *kittygraphics.Session {75+ if state := s.alternate; state != nil {
71- if s == nil || s.graphics == nil {76+ state.kittyPendingDisplay = nil
72- return nil77+ if state.graphics != nil && state.graphics.kitty != nil {
78+ state.graphics.kitty.AbortPendingUpload()
79+ }
73 }80 }
74- return s.graphics.kitty
75 }81 }
76 82
77-// KittyGraphics is an adapter-named alias for GraphicsSession.
78-func (s *Screen) KittyGraphics() *kittygraphics.Session { return s.GraphicsSession() }
79-
80 // GraphicsSnapshot returns an immutable snapshot of the active screen83 // GraphicsSnapshot returns an immutable snapshot of the active screen
81 // buffer's graphics scene, or nil before graphics has been used.84 // buffer's graphics scene, or nil before graphics has been used.
82 func (s *Screen) GraphicsSnapshot() *graphics.Snapshot {85 func (s *Screen) GraphicsSnapshot() *graphics.Snapshot {
M protocol/kittygraphics/controls.go +16 -19
@@ -15,20 +15,26 @@ func parseControls(header []byte) (Controls, error) {
1515 if len(header) == 0 {
1616 return c, nil
1717 }
18- seen := make(map[byte]struct{}, 8)
19- for _, field := range splitComma(header) {
18+ var seen [256]bool
19+ start := 0
20+ for i := 0; i <= len(header); i++ {
21+ if i != len(header) && header[i] != ',' {
22+ continue
23+ }
24+ field := header[start:i]
2025 if len(field) < 3 || field[1] != '=' {
2126 return Controls{}, fmt.Errorf("%w: control %q", ErrInvalidCommand, field)
2227 }
2328 key := field[0]
24- if _, ok := seen[key]; ok {
29+ if seen[key] {
2530 return Controls{}, fmt.Errorf("%w: %c", ErrDuplicateControl, key)
2631 }
27- seen[key] = struct{}{}
32+ seen[key] = true
2833 value := field[2:]
2934 if err := parseControl(&c, key, value); err != nil {
3035 return Controls{}, err
3136 }
37+ start = i + 1
3238 }
3339 if c.HasAction && !c.Action.Valid() {
3440 return Controls{}, fmt.Errorf("%w: %q", ErrUnknownAction, c.Action)
@@ -48,18 +54,6 @@ func parseControls(header []byte) (Controls, error) {
4854 return c, nil
4955 }
5056
51-func splitComma(data []byte) [][]byte {
52- var out [][]byte
53- start := 0
54- for i, b := range data {
55- if b == ',' {
56- out = append(out, data[start:i])
57- start = i + 1
58- }
59- }
60- return append(out, data[start:])
61-}
62-
6357 func parseControl(c *Controls, key byte, value []byte) error {
6458 if len(value) == 0 {
6559 return fmt.Errorf("%w: empty %c value", ErrInvalidInteger, key)
@@ -93,9 +87,12 @@ func parseControl(c *Controls, key byte, value []byte) error {
9387 case ControlMore:
9488 return setUint(&c.More, &c.HasMore, key, value)
9589 case ControlQuiet:
96- var n uint64
97- if err := setUint(&n, nil, key, value); err != nil {
98- return err
90+ n, err := parseUint(value)
91+ if err != nil {
92+ return fieldError(key, err)
93+ }
94+ if n > uint64(QuietAll) {
95+ return fmt.Errorf("%w: quiet=%d", ErrInvalidCommand, n)
9996 }
10097 c.Quiet, c.HasQuiet = Quiet(n), true
10198 case ControlWidth:
@@ -15,20 +15,26 @@ func parseControls(header []byte) (Controls, error) {
15 if len(header) == 0 {15 if len(header) == 0 {
16 return c, nil16 return c, nil
17 }17 }
18- seen := make(map[byte]struct{}, 8)18+ var seen [256]bool
19- for _, field := range splitComma(header) {19+ start := 0
20+ for i := 0; i <= len(header); i++ {
21+ if i != len(header) && header[i] != ',' {
22+ continue
23+ }
24+ field := header[start:i]
20 if len(field) < 3 || field[1] != '=' {25 if len(field) < 3 || field[1] != '=' {
21 return Controls{}, fmt.Errorf("%w: control %q", ErrInvalidCommand, field)26 return Controls{}, fmt.Errorf("%w: control %q", ErrInvalidCommand, field)
22 }27 }
23 key := field[0]28 key := field[0]
24- if _, ok := seen[key]; ok {29+ if seen[key] {
25 return Controls{}, fmt.Errorf("%w: %c", ErrDuplicateControl, key)30 return Controls{}, fmt.Errorf("%w: %c", ErrDuplicateControl, key)
26 }31 }
27- seen[key] = struct{}{}32+ seen[key] = true
28 value := field[2:]33 value := field[2:]
29 if err := parseControl(&c, key, value); err != nil {34 if err := parseControl(&c, key, value); err != nil {
30 return Controls{}, err35 return Controls{}, err
31 }36 }
37+ start = i + 1
32 }38 }
33 if c.HasAction && !c.Action.Valid() {39 if c.HasAction && !c.Action.Valid() {
34 return Controls{}, fmt.Errorf("%w: %q", ErrUnknownAction, c.Action)40 return Controls{}, fmt.Errorf("%w: %q", ErrUnknownAction, c.Action)
@@ -48,18 +54,6 @@ func parseControls(header []byte) (Controls, error) {
48 return c, nil54 return c, nil
49 }55 }
50 56
51-func splitComma(data []byte) [][]byte {
52- var out [][]byte
53- start := 0
54- for i, b := range data {
55- if b == ',' {
56- out = append(out, data[start:i])
57- start = i + 1
58- }
59- }
60- return append(out, data[start:])
61-}
62-
63 func parseControl(c *Controls, key byte, value []byte) error {57 func parseControl(c *Controls, key byte, value []byte) error {
64 if len(value) == 0 {58 if len(value) == 0 {
65 return fmt.Errorf("%w: empty %c value", ErrInvalidInteger, key)59 return fmt.Errorf("%w: empty %c value", ErrInvalidInteger, key)
@@ -93,9 +87,12 @@ func parseControl(c *Controls, key byte, value []byte) error {
93 case ControlMore:87 case ControlMore:
94 return setUint(&c.More, &c.HasMore, key, value)88 return setUint(&c.More, &c.HasMore, key, value)
95 case ControlQuiet:89 case ControlQuiet:
96- var n uint6490+ n, err := parseUint(value)
97- if err := setUint(&n, nil, key, value); err != nil {91+ if err != nil {
98- return err92+ return fieldError(key, err)
93+ }
94+ if n > uint64(QuietAll) {
95+ return fmt.Errorf("%w: quiet=%d", ErrInvalidCommand, n)
99 }96 }
100 c.Quiet, c.HasQuiet = Quiet(n), true97 c.Quiet, c.HasQuiet = Quiet(n), true
101 case ControlWidth:98 case ControlWidth:
M protocol/kittygraphics/kittygraphics.go +20 -26
@@ -325,16 +325,17 @@ const MaxKittyID = uint64(^uint32(0))
325325 // Feed and query methods serially; the mutex also makes read-only mapping
326326 // access safe while a caller publishes a snapshot.
327327 type Session struct {
328- mu sync.Mutex
329- scene *graphics.Scene
330- limits Limits
331- parser *Parser
332- images map[uint64]graphics.AssetID
333- imageNumbers map[uint64]uint64
334- placements map[uint64]graphics.PlacementID
335- children map[uint64]Child
336- nextChild uint64
337- upload *upload
328+ mu sync.Mutex
329+ scene *graphics.Scene
330+ limits Limits
331+ parser *Parser
332+ images map[uint64]graphics.AssetID
333+ imageNumbers map[uint64]uint64
334+ placements map[uint64]graphics.PlacementID
335+ children map[uint64]Child
336+ nextChild uint64
337+ nextPlacement uint64
338+ upload *upload
338339 }
339340
340341 // Adapter is an alias for the stateful Kitty graphics session.
@@ -359,14 +360,15 @@ func NewSession(scene *graphics.Scene, config ...Limits) *Session {
359360 }
360361 l = normalizeLimits(l)
361362 return &Session{
362- scene: scene,
363- limits: l,
364- parser: NewParser(l),
365- images: make(map[uint64]graphics.AssetID),
366- imageNumbers: make(map[uint64]uint64),
367- placements: make(map[uint64]graphics.PlacementID),
368- children: make(map[uint64]Child),
369- nextChild: 1,
363+ scene: scene,
364+ limits: l,
365+ parser: NewParser(l),
366+ images: make(map[uint64]graphics.AssetID),
367+ imageNumbers: make(map[uint64]uint64),
368+ placements: make(map[uint64]graphics.PlacementID),
369+ children: make(map[uint64]Child),
370+ nextChild: 1,
371+ nextPlacement: 1,
370372 }
371373 }
372374
@@ -378,14 +380,6 @@ func NewAdapter(scene *graphics.Scene, config ...Limits) *Session {
378380 // New is a concise constructor alias for NewSession.
379381 func New(scene *graphics.Scene, config ...Limits) *Session { return NewSession(scene, config...) }
380382
381-// Scene returns the scene owned by the session.
382-func (s *Session) Scene() *graphics.Scene {
383- if s == nil {
384- return nil
385- }
386- return s.scene
387-}
388-
389383 // Image returns the graphics asset mapped from a Kitty image ID.
390384 func (s *Session) Image(imageID uint64) (graphics.AssetID, bool) {
391385 if s == nil {
@@ -325,16 +325,17 @@ const MaxKittyID = uint64(^uint32(0))
325 // Feed and query methods serially; the mutex also makes read-only mapping325 // Feed and query methods serially; the mutex also makes read-only mapping
326 // access safe while a caller publishes a snapshot.326 // access safe while a caller publishes a snapshot.
327 type Session struct {327 type Session struct {
328- mu sync.Mutex328+ mu sync.Mutex
329- scene *graphics.Scene329+ scene *graphics.Scene
330- limits Limits330+ limits Limits
331- parser *Parser331+ parser *Parser
332- images map[uint64]graphics.AssetID332+ images map[uint64]graphics.AssetID
333- imageNumbers map[uint64]uint64333+ imageNumbers map[uint64]uint64
334- placements map[uint64]graphics.PlacementID334+ placements map[uint64]graphics.PlacementID
335- children map[uint64]Child335+ children map[uint64]Child
336- nextChild uint64336+ nextChild uint64
337- upload *upload337+ nextPlacement uint64
338+ upload *upload
338 }339 }
339 340
340 // Adapter is an alias for the stateful Kitty graphics session.341 // Adapter is an alias for the stateful Kitty graphics session.
@@ -359,14 +360,15 @@ func NewSession(scene *graphics.Scene, config ...Limits) *Session {
359 }360 }
360 l = normalizeLimits(l)361 l = normalizeLimits(l)
361 return &Session{362 return &Session{
362- scene: scene,363+ scene: scene,
363- limits: l,364+ limits: l,
364- parser: NewParser(l),365+ parser: NewParser(l),
365- images: make(map[uint64]graphics.AssetID),366+ images: make(map[uint64]graphics.AssetID),
366- imageNumbers: make(map[uint64]uint64),367+ imageNumbers: make(map[uint64]uint64),
367- placements: make(map[uint64]graphics.PlacementID),368+ placements: make(map[uint64]graphics.PlacementID),
368- children: make(map[uint64]Child),369+ children: make(map[uint64]Child),
369- nextChild: 1,370+ nextChild: 1,
371+ nextPlacement: 1,
370 }372 }
371 }373 }
372 374
@@ -378,14 +380,6 @@ func NewAdapter(scene *graphics.Scene, config ...Limits) *Session {
378 // New is a concise constructor alias for NewSession.380 // New is a concise constructor alias for NewSession.
379 func New(scene *graphics.Scene, config ...Limits) *Session { return NewSession(scene, config...) }381 func New(scene *graphics.Scene, config ...Limits) *Session { return NewSession(scene, config...) }
380 382
381-// Scene returns the scene owned by the session.
382-func (s *Session) Scene() *graphics.Scene {
383- if s == nil {
384- return nil
385- }
386- return s.scene
387-}
388-
389 // Image returns the graphics asset mapped from a Kitty image ID.383 // Image returns the graphics asset mapped from a Kitty image ID.
390 func (s *Session) Image(imageID uint64) (graphics.AssetID, bool) {384 func (s *Session) Image(imageID uint64) (graphics.AssetID, bool) {
391 if s == nil {385 if s == nil {
M protocol/kittygraphics/kittygraphics_test.go +11 -0
@@ -145,6 +145,9 @@ func TestMalformedTruncatedInterleavedAndOversizeAreBounded(t *testing.T) {
145145 if _, err := ParseCommand([]byte("a=q,i=18446744073709551616;")); !errors.Is(err, ErrIntegerOverflow) {
146146 t.Fatalf("integer overflow error = %v", err)
147147 }
148+ if _, err := ParseCommand([]byte("a=q,q=256;")); !errors.Is(err, ErrInvalidCommand) {
149+ t.Fatalf("out-of-range quiet error = %v", err)
150+ }
148151 if _, err := ParseCommand([]byte("a=q,i=1,I=2;")); !errors.Is(err, ErrInvalidCommand) {
149152 t.Fatalf("mixed image selector error = %v", err)
150153 }
@@ -216,6 +219,14 @@ func TestChunkedTransmitAndDisplayRetainsDisplayAction(t *testing.T) {
216219 }
217220 }
218221
222+func TestImplicitPlacementIDsStopAtKittyLimit(t *testing.T) {
223+ session := NewSession(nil)
224+ session.nextPlacement = MaxKittyID + 1
225+ if _, ok := session.takePlacementID(); ok {
226+ t.Fatal("implicit placement ID exceeded Kitty's uint32 namespace")
227+ }
228+}
229+
219230 func TestPlacementFailureEmitsStableProtocolError(t *testing.T) {
220231 scene := graphics.NewScene(graphics.Limits{})
221232 session := NewSession(scene)
@@ -145,6 +145,9 @@ func TestMalformedTruncatedInterleavedAndOversizeAreBounded(t *testing.T) {
145 if _, err := ParseCommand([]byte("a=q,i=18446744073709551616;")); !errors.Is(err, ErrIntegerOverflow) {145 if _, err := ParseCommand([]byte("a=q,i=18446744073709551616;")); !errors.Is(err, ErrIntegerOverflow) {
146 t.Fatalf("integer overflow error = %v", err)146 t.Fatalf("integer overflow error = %v", err)
147 }147 }
148+ if _, err := ParseCommand([]byte("a=q,q=256;")); !errors.Is(err, ErrInvalidCommand) {
149+ t.Fatalf("out-of-range quiet error = %v", err)
150+ }
148 if _, err := ParseCommand([]byte("a=q,i=1,I=2;")); !errors.Is(err, ErrInvalidCommand) {151 if _, err := ParseCommand([]byte("a=q,i=1,I=2;")); !errors.Is(err, ErrInvalidCommand) {
149 t.Fatalf("mixed image selector error = %v", err)152 t.Fatalf("mixed image selector error = %v", err)
150 }153 }
@@ -216,6 +219,14 @@ func TestChunkedTransmitAndDisplayRetainsDisplayAction(t *testing.T) {
216 }219 }
217 }220 }
218 221
222+func TestImplicitPlacementIDsStopAtKittyLimit(t *testing.T) {
223+ session := NewSession(nil)
224+ session.nextPlacement = MaxKittyID + 1
225+ if _, ok := session.takePlacementID(); ok {
226+ t.Fatal("implicit placement ID exceeded Kitty's uint32 namespace")
227+ }
228+}
229+
219 func TestPlacementFailureEmitsStableProtocolError(t *testing.T) {230 func TestPlacementFailureEmitsStableProtocolError(t *testing.T) {
220 scene := graphics.NewScene(graphics.Limits{})231 scene := graphics.NewScene(graphics.Limits{})
221 session := NewSession(scene)232 session := NewSession(scene)
M protocol/kittygraphics/session.go +55 -18
@@ -546,7 +546,11 @@ func (s *Session) place(c Controls, imageID uint64, assetID graphics.AssetID) ([
546546 }
547547 placementID := c.PlacementID
548548 if placementID == 0 {
549- placementID = s.nextPlacementID()
549+ var ok bool
550+ placementID, ok = s.takePlacementID()
551+ if !ok {
552+ return nil, nil, graphics.ErrIdentifierOverflow
553+ }
550554 }
551555 if existing, ok := s.placements[placementID]; ok {
552556 if err := s.scene.UpdatePlacement(existing, spec); err != nil {
@@ -603,15 +607,15 @@ func (s *Session) delete(c Controls) ([][]byte, *Mutation, error) {
603607 return s.failure(c, id, ErrNoScene)
604608 }
605609 snapshot := s.scene.Snapshot()
610+ placementIDs := make([]uint64, 0)
606611 for placementID, mapped := range s.placements {
607612 placement, ok := snapshot.Placement(mapped)
608- if !ok || placement.AssetID() != asset {
609- continue
610- }
611- if err := s.scene.RemovePlacement(mapped); err != nil {
612- return s.failure(c, id, err)
613+ if ok && placement.AssetID() == asset {
614+ placementIDs = append(placementIDs, placementID)
613615 }
614- delete(s.placements, placementID)
616+ }
617+ if err := s.removePlacements(placementIDs); err != nil {
618+ return s.failure(c, id, err)
615619 }
616620 return s.success(c, id), &Mutation{Kind: MutationDeletePlacements, ImageID: id, AssetID: asset}, nil
617621 case DeletePlacement:
@@ -630,11 +634,12 @@ func (s *Session) delete(c Controls) ([][]byte, *Mutation, error) {
630634 return s.failure(c, responseID, ErrNoScene)
631635 }
632636 if target == DeleteAllPlacements {
633- for placementID, mapped := range s.placements {
634- if err := s.scene.RemovePlacement(mapped); err != nil {
635- return s.failure(c, responseID, err)
636- }
637- delete(s.placements, placementID)
637+ placementIDs := make([]uint64, 0, len(s.placements))
638+ for placementID := range s.placements {
639+ placementIDs = append(placementIDs, placementID)
640+ }
641+ if err := s.removePlacements(placementIDs); err != nil {
642+ return s.failure(c, responseID, err)
638643 }
639644 } else {
640645 if err := s.scene.Clear(); err != nil {
@@ -651,6 +656,33 @@ func (s *Session) delete(c Controls) ([][]byte, *Mutation, error) {
651656 }
652657 }
653658
659+func (s *Session) removePlacements(ids []uint64) error {
660+ if len(ids) == 0 {
661+ return nil
662+ }
663+ operations := make([]graphics.Operation, 0, len(ids))
664+ for _, id := range ids {
665+ mapped, ok := s.placements[id]
666+ if !ok {
667+ continue
668+ }
669+ operations = append(operations, graphics.Operation{
670+ Kind: graphics.OperationRemovePlacement,
671+ PlacementID: mapped,
672+ })
673+ }
674+ if len(operations) == 0 {
675+ return nil
676+ }
677+ if err := s.scene.ApplyOperations(operations); err != nil {
678+ return err
679+ }
680+ for _, id := range ids {
681+ delete(s.placements, id)
682+ }
683+ return nil
684+}
685+
654686 func (s *Session) removeMappingsForAsset(asset graphics.AssetID) {
655687 for imageID, mapped := range s.images {
656688 if mapped == asset {
@@ -730,14 +762,19 @@ func (s *Session) nextImageID() uint64 {
730762 }
731763 }
732764
733-func (s *Session) nextPlacementID() uint64 {
734- max := uint64(0)
735- for id := range s.placements {
736- if id > max {
737- max = id
765+func (s *Session) takePlacementID() (uint64, bool) {
766+ for s.nextPlacement != 0 && s.nextPlacement <= MaxKittyID {
767+ id := s.nextPlacement
768+ if s.nextPlacement == MaxKittyID {
769+ s.nextPlacement = 0
770+ } else {
771+ s.nextPlacement++
772+ }
773+ if _, exists := s.placements[id]; !exists {
774+ return id, true
738775 }
739776 }
740- return max + 1
777+ return 0, false
741778 }
742779
743780 func checkedProduct(left, right uint64) (uint64, bool) {
@@ -546,7 +546,11 @@ func (s *Session) place(c Controls, imageID uint64, assetID graphics.AssetID) ([
546 }546 }
547 placementID := c.PlacementID547 placementID := c.PlacementID
548 if placementID == 0 {548 if placementID == 0 {
549- placementID = s.nextPlacementID()549+ var ok bool
550+ placementID, ok = s.takePlacementID()
551+ if !ok {
552+ return nil, nil, graphics.ErrIdentifierOverflow
553+ }
550 }554 }
551 if existing, ok := s.placements[placementID]; ok {555 if existing, ok := s.placements[placementID]; ok {
552 if err := s.scene.UpdatePlacement(existing, spec); err != nil {556 if err := s.scene.UpdatePlacement(existing, spec); err != nil {
@@ -603,15 +607,15 @@ func (s *Session) delete(c Controls) ([][]byte, *Mutation, error) {
603 return s.failure(c, id, ErrNoScene)607 return s.failure(c, id, ErrNoScene)
604 }608 }
605 snapshot := s.scene.Snapshot()609 snapshot := s.scene.Snapshot()
610+ placementIDs := make([]uint64, 0)
606 for placementID, mapped := range s.placements {611 for placementID, mapped := range s.placements {
607 placement, ok := snapshot.Placement(mapped)612 placement, ok := snapshot.Placement(mapped)
608- if !ok || placement.AssetID() != asset {613+ if ok && placement.AssetID() == asset {
609- continue614+ placementIDs = append(placementIDs, placementID)
610- }
611- if err := s.scene.RemovePlacement(mapped); err != nil {
612- return s.failure(c, id, err)
613 }615 }
614- delete(s.placements, placementID)616+ }
617+ if err := s.removePlacements(placementIDs); err != nil {
618+ return s.failure(c, id, err)
615 }619 }
616 return s.success(c, id), &Mutation{Kind: MutationDeletePlacements, ImageID: id, AssetID: asset}, nil620 return s.success(c, id), &Mutation{Kind: MutationDeletePlacements, ImageID: id, AssetID: asset}, nil
617 case DeletePlacement:621 case DeletePlacement:
@@ -630,11 +634,12 @@ func (s *Session) delete(c Controls) ([][]byte, *Mutation, error) {
630 return s.failure(c, responseID, ErrNoScene)634 return s.failure(c, responseID, ErrNoScene)
631 }635 }
632 if target == DeleteAllPlacements {636 if target == DeleteAllPlacements {
633- for placementID, mapped := range s.placements {637+ placementIDs := make([]uint64, 0, len(s.placements))
634- if err := s.scene.RemovePlacement(mapped); err != nil {638+ for placementID := range s.placements {
635- return s.failure(c, responseID, err)639+ placementIDs = append(placementIDs, placementID)
636- }640+ }
637- delete(s.placements, placementID)641+ if err := s.removePlacements(placementIDs); err != nil {
642+ return s.failure(c, responseID, err)
638 }643 }
639 } else {644 } else {
640 if err := s.scene.Clear(); err != nil {645 if err := s.scene.Clear(); err != nil {
@@ -651,6 +656,33 @@ func (s *Session) delete(c Controls) ([][]byte, *Mutation, error) {
651 }656 }
652 }657 }
653 658
659+func (s *Session) removePlacements(ids []uint64) error {
660+ if len(ids) == 0 {
661+ return nil
662+ }
663+ operations := make([]graphics.Operation, 0, len(ids))
664+ for _, id := range ids {
665+ mapped, ok := s.placements[id]
666+ if !ok {
667+ continue
668+ }
669+ operations = append(operations, graphics.Operation{
670+ Kind: graphics.OperationRemovePlacement,
671+ PlacementID: mapped,
672+ })
673+ }
674+ if len(operations) == 0 {
675+ return nil
676+ }
677+ if err := s.scene.ApplyOperations(operations); err != nil {
678+ return err
679+ }
680+ for _, id := range ids {
681+ delete(s.placements, id)
682+ }
683+ return nil
684+}
685+
654 func (s *Session) removeMappingsForAsset(asset graphics.AssetID) {686 func (s *Session) removeMappingsForAsset(asset graphics.AssetID) {
655 for imageID, mapped := range s.images {687 for imageID, mapped := range s.images {
656 if mapped == asset {688 if mapped == asset {
@@ -730,14 +762,19 @@ func (s *Session) nextImageID() uint64 {
730 }762 }
731 }763 }
732 764
733-func (s *Session) nextPlacementID() uint64 {765+func (s *Session) takePlacementID() (uint64, bool) {
734- max := uint64(0)766+ for s.nextPlacement != 0 && s.nextPlacement <= MaxKittyID {
735- for id := range s.placements {767+ id := s.nextPlacement
736- if id > max {768+ if s.nextPlacement == MaxKittyID {
737- max = id769+ s.nextPlacement = 0
770+ } else {
771+ s.nextPlacement++
772+ }
773+ if _, exists := s.placements[id]; !exists {
774+ return id, true
738 }775 }
739 }776 }
740- return max + 1777+ return 0, false
741 }778 }
742 779
743 func checkedProduct(left, right uint64) (uint64, bool) {780 func checkedProduct(left, right uint64) (uint64, bool) {
M protocol/terminalquery/query.go +23 -8
@@ -145,20 +145,35 @@ func da1Terminator(data []byte) (int, bool) {
145145 }
146146
147147 func validKittyResponse(data []byte) bool {
148- if len(data) < len("\x1b_Gi=31;OK\x1b\\") || !bytes.HasPrefix(data, []byte("\x1b_G")) || !bytes.HasSuffix(data, []byte("\x1b\\")) {
149- return false
150- }
151- head := data[3 : len(data)-2]
152- return bytes.Contains(head, []byte("i=31")) && bytes.Contains(head, []byte("OK"))
148+ return bytes.Equal(data, []byte("\x1b_Gi=31;OK\x1b\\"))
153149 }
154150
155151 func validDA1Response(data []byte) bool {
156- if len(data) < 4 || !bytes.HasPrefix(data, []byte("\x1b[")) || data[len(data)-1] != 'c' {
152+ if len(data) < 5 || !bytes.HasPrefix(data, []byte("\x1b[")) || data[len(data)-1] != 'c' || data[2] != '?' || len(data) > maxProbeResponseBytes {
157153 return false
158154 }
159155 // DA1 is a primary response. Secondary DA (ESC[>...c) is deliberately
160- // not sufficient for a direct graphics declaration.
161- return data[2] == '?' && len(data) <= maxProbeResponseBytes
156+ // not sufficient for a direct graphics declaration. The parameter body is
157+ // a non-empty semicolon-separated list of decimal numbers.
158+ body := data[3 : len(data)-1]
159+ if len(body) == 0 || body[0] == ';' || body[len(body)-1] == ';' {
160+ return false
161+ }
162+ previousSeparator := false
163+ for _, b := range body {
164+ switch {
165+ case b >= '0' && b <= '9':
166+ previousSeparator = false
167+ case b == ';':
168+ if previousSeparator {
169+ return false
170+ }
171+ previousSeparator = true
172+ default:
173+ return false
174+ }
175+ }
176+ return !previousSeparator
162177 }
163178
164179 func suffixPrefixLen(data []byte) int {
@@ -145,20 +145,35 @@ func da1Terminator(data []byte) (int, bool) {
145 }145 }
146 146
147 func validKittyResponse(data []byte) bool {147 func validKittyResponse(data []byte) bool {
148- if len(data) < len("\x1b_Gi=31;OK\x1b\\") || !bytes.HasPrefix(data, []byte("\x1b_G")) || !bytes.HasSuffix(data, []byte("\x1b\\")) {148+ return bytes.Equal(data, []byte("\x1b_Gi=31;OK\x1b\\"))
149- return false
150- }
151- head := data[3 : len(data)-2]
152- return bytes.Contains(head, []byte("i=31")) && bytes.Contains(head, []byte("OK"))
153 }149 }
154 150
155 func validDA1Response(data []byte) bool {151 func validDA1Response(data []byte) bool {
156- if len(data) < 4 || !bytes.HasPrefix(data, []byte("\x1b[")) || data[len(data)-1] != 'c' {152+ if len(data) < 5 || !bytes.HasPrefix(data, []byte("\x1b[")) || data[len(data)-1] != 'c' || data[2] != '?' || len(data) > maxProbeResponseBytes {
157 return false153 return false
158 }154 }
159 // DA1 is a primary response. Secondary DA (ESC[>...c) is deliberately155 // DA1 is a primary response. Secondary DA (ESC[>...c) is deliberately
160- // not sufficient for a direct graphics declaration.156+ // not sufficient for a direct graphics declaration. The parameter body is
161- return data[2] == '?' && len(data) <= maxProbeResponseBytes157+ // a non-empty semicolon-separated list of decimal numbers.
158+ body := data[3 : len(data)-1]
159+ if len(body) == 0 || body[0] == ';' || body[len(body)-1] == ';' {
160+ return false
161+ }
162+ previousSeparator := false
163+ for _, b := range body {
164+ switch {
165+ case b >= '0' && b <= '9':
166+ previousSeparator = false
167+ case b == ';':
168+ if previousSeparator {
169+ return false
170+ }
171+ previousSeparator = true
172+ default:
173+ return false
174+ }
175+ }
176+ return !previousSeparator
162 }177 }
163 178
164 func suffixPrefixLen(data []byte) int {179 func suffixPrefixLen(data []byte) int {
M protocol/terminalquery/query_test.go +25 -0
@@ -39,6 +39,31 @@ func TestProbeRecognizesFragmentedResponsesAndReplaysInput(t *testing.T) {
3939 require.Equal(t, "abz", string(unrelated))
4040 }
4141
42+func TestProbeRequiresExactCapabilityResponses(t *testing.T) {
43+ for _, data := range []string{
44+ "\x1b_Gi=31;OK,extra\x1b\\",
45+ "\x1b_Gi=310;OK\x1b\\",
46+ "\x1b_Gi=31;OKAY\x1b\\",
47+ "\x1b[?1;;2c",
48+ "\x1b[?x;2c",
49+ } {
50+ var p Probe
51+ got := append(p.Feed([]byte(data)), p.Finish()...)
52+ require.False(t, p.Ready(), "response %q was accepted", data)
53+ require.Equal(t, data, string(got))
54+ }
55+}
56+
57+func FuzzProbeNeverPanics(f *testing.F) {
58+ f.Add([]byte("text\x1b[?1;2c\x1b_Gi=31;OK\x1b\\"))
59+ f.Add([]byte("\x1b_G"))
60+ f.Fuzz(func(t *testing.T, data []byte) {
61+ var p Probe
62+ _ = p.Feed(data)
63+ _ = p.Finish()
64+ })
65+}
66+
4267 func TestProbeBoundsUnterminatedResponse(t *testing.T) {
4368 var p Probe
4469 input := append([]byte("prefix"), []byte("\x1b_G")...)
@@ -39,6 +39,31 @@ func TestProbeRecognizesFragmentedResponsesAndReplaysInput(t *testing.T) {
39 require.Equal(t, "abz", string(unrelated))39 require.Equal(t, "abz", string(unrelated))
40 }40 }
41 41
42+func TestProbeRequiresExactCapabilityResponses(t *testing.T) {
43+ for _, data := range []string{
44+ "\x1b_Gi=31;OK,extra\x1b\\",
45+ "\x1b_Gi=310;OK\x1b\\",
46+ "\x1b_Gi=31;OKAY\x1b\\",
47+ "\x1b[?1;;2c",
48+ "\x1b[?x;2c",
49+ } {
50+ var p Probe
51+ got := append(p.Feed([]byte(data)), p.Finish()...)
52+ require.False(t, p.Ready(), "response %q was accepted", data)
53+ require.Equal(t, data, string(got))
54+ }
55+}
56+
57+func FuzzProbeNeverPanics(f *testing.F) {
58+ f.Add([]byte("text\x1b[?1;2c\x1b_Gi=31;OK\x1b\\"))
59+ f.Add([]byte("\x1b_G"))
60+ f.Fuzz(func(t *testing.T, data []byte) {
61+ var p Probe
62+ _ = p.Feed(data)
63+ _ = p.Finish()
64+ })
65+}
66+
42 func TestProbeBoundsUnterminatedResponse(t *testing.T) {67 func TestProbeBoundsUnterminatedResponse(t *testing.T) {
43 var p Probe68 var p Probe
44 input := append([]byte("prefix"), []byte("\x1b_G")...)69 input := append([]byte("prefix"), []byte("\x1b_G")...)
M screen.go +49 -19
@@ -172,6 +172,7 @@ func (s *Screen) Write(data []byte) {
172172 if len(data) <= limit {
173173 s.escapeBuf = append(s.escapeBuf[:0], data...)
174174 } else if isKittyEscapePrefix(data) {
175+ s.abortKittyPendingDisplay()
175176 s.kittyDiscard = true
176177 s.kittyDiscardEscaped = false
177178 }
@@ -219,6 +220,8 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte {
219220 if len(prefix)+1 <= maxKittyEscapeBufferLen {
220221 apc := append(prefix, data[:1]...)
221222 s.dispatchKittyGraphics(apc)
223+ } else {
224+ s.abortKittyPendingDisplay()
222225 }
223226 return data[1:]
224227 }
@@ -228,6 +231,8 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte {
228231 if len(prefix)+end <= maxKittyEscapeBufferLen {
229232 apc := append(prefix, data[:end]...)
230233 s.dispatchKittyGraphics(apc)
234+ } else {
235+ s.abortKittyPendingDisplay()
231236 }
232237 return data[end:]
233238 }
@@ -236,6 +241,8 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte {
236241 if len(prefix)+end <= maxKittyEscapeBufferLen {
237242 apc := append(prefix, data[:end]...)
238243 s.dispatchKittyGraphics(apc)
244+ } else {
245+ s.abortKittyPendingDisplay()
239246 }
240247 return data[end:]
241248 }
@@ -246,8 +253,13 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte {
246253 s.escapeBuf = returnData
247254 return nil
248255 }
256+ s.abortKittyPendingDisplay()
249257 s.kittyDiscard = true
250- s.kittyDiscardEscaped = false
258+ escaped := len(prefix) != 0 && prefix[len(prefix)-1] == 0x1b
259+ if remaining > 0 {
260+ escaped = data[remaining-1] == 0x1b
261+ }
262+ s.kittyDiscardEscaped = escaped
251263 tail := data[remaining:]
252264 return tail[s.consumeKittyDiscard(tail):]
253265 }
@@ -349,12 +361,22 @@ func (s *Screen) consumeKittyGraphics(data []byte) (consumed int, partial bool)
349361 for i := 3; i < len(data); i++ {
350362 switch data[i] {
351363 case 0x9c: // C1 string terminator.
352- s.dispatchKittyGraphics(data[:i+1])
353- return i + 1, false
364+ end := i + 1
365+ if end > maxKittyEscapeBufferLen {
366+ s.abortKittyPendingDisplay()
367+ return end, false
368+ }
369+ s.dispatchKittyGraphics(data[:end])
370+ return end, false
354371 case 0x1b:
355372 if i+1 < len(data) && data[i+1] == '\\' {
356- s.dispatchKittyGraphics(data[:i+2])
357- return i + 2, false
373+ end := i + 2
374+ if end > maxKittyEscapeBufferLen {
375+ s.abortKittyPendingDisplay()
376+ return end, false
377+ }
378+ s.dispatchKittyGraphics(data[:end])
379+ return end, false
358380 }
359381 }
360382 }
@@ -362,13 +384,14 @@ func (s *Screen) consumeKittyGraphics(data []byte) (consumed int, partial bool)
362384 }
363385
364386 func (s *Screen) dispatchKittyGraphics(apc []byte) {
365- if s.graphics == nil {
366- s.graphics = newScreenGraphicsState()
367- }
368387 command, err := kittygraphics.ParseAPC(apc)
369388 if err != nil {
389+ s.abortKittyPendingDisplay()
370390 return
371391 }
392+ if s.graphics == nil {
393+ s.graphics = newScreenGraphicsState()
394+ }
372395 controls := command.Controls
373396 pending := s.kittyPendingDisplay
374397 continuation := pending != nil
@@ -405,7 +428,7 @@ func (s *Screen) dispatchKittyGraphics(apc []byte) {
405428 }
406429 result, processErr := s.graphics.kitty.Process(command)
407430 if processErr != nil {
408- s.kittyPendingDisplay = nil
431+ s.abortKittyPendingDisplay()
409432 }
410433 for _, response := range result.Responses {
411434 s.respond(response)
@@ -422,25 +445,32 @@ func (s *Screen) dispatchKittyGraphics(apc []byte) {
422445 }
423446
424447 func (s *Screen) kittyCursorX() uint64 {
425- if s.geometry.PixelWidth > 0 && s.geometry.Cols > 0 {
426- cellWidth := s.geometry.PixelWidth / s.geometry.Cols
427- if cellWidth > 0 {
428- return uint64(s.Col * cellWidth)
429- }
448+ if cellWidth, _, ok := s.kittyCellPixels(); ok {
449+ return uint64(s.Col * cellWidth)
430450 }
431451 return uint64(s.Col)
432452 }
433453
434454 func (s *Screen) kittyCursorY() uint64 {
435- if s.geometry.PixelHeight > 0 && s.geometry.Rows > 0 {
436- cellHeight := s.geometry.PixelHeight / s.geometry.Rows
437- if cellHeight > 0 {
438- return uint64(s.Row * cellHeight)
439- }
455+ _, cellHeight, ok := s.kittyCellPixels()
456+ if ok {
457+ return uint64(s.Row * cellHeight)
440458 }
441459 return uint64(s.Row)
442460 }
443461
462+func (s *Screen) kittyCellPixels() (width, height int, ok bool) {
463+ if !s.geometry.PixelsKnown() || s.geometry.Cols <= 0 || s.geometry.Rows <= 0 {
464+ return 0, 0, false
465+ }
466+ width = s.geometry.PixelWidth / s.geometry.Cols
467+ height = s.geometry.PixelHeight / s.geometry.Rows
468+ if width <= 0 || height <= 0 {
469+ return 0, 0, false
470+ }
471+ return width, height, true
472+}
473+
444474 func (s *Screen) applyKittyCursorMovement(c kittygraphics.Controls) {
445475 if c.HasCursor && c.Cursor == 1 {
446476 return
@@ -172,6 +172,7 @@ func (s *Screen) Write(data []byte) {
172 if len(data) <= limit {172 if len(data) <= limit {
173 s.escapeBuf = append(s.escapeBuf[:0], data...)173 s.escapeBuf = append(s.escapeBuf[:0], data...)
174 } else if isKittyEscapePrefix(data) {174 } else if isKittyEscapePrefix(data) {
175+ s.abortKittyPendingDisplay()
175 s.kittyDiscard = true176 s.kittyDiscard = true
176 s.kittyDiscardEscaped = false177 s.kittyDiscardEscaped = false
177 }178 }
@@ -219,6 +220,8 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte {
219 if len(prefix)+1 <= maxKittyEscapeBufferLen {220 if len(prefix)+1 <= maxKittyEscapeBufferLen {
220 apc := append(prefix, data[:1]...)221 apc := append(prefix, data[:1]...)
221 s.dispatchKittyGraphics(apc)222 s.dispatchKittyGraphics(apc)
223+ } else {
224+ s.abortKittyPendingDisplay()
222 }225 }
223 return data[1:]226 return data[1:]
224 }227 }
@@ -228,6 +231,8 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte {
228 if len(prefix)+end <= maxKittyEscapeBufferLen {231 if len(prefix)+end <= maxKittyEscapeBufferLen {
229 apc := append(prefix, data[:end]...)232 apc := append(prefix, data[:end]...)
230 s.dispatchKittyGraphics(apc)233 s.dispatchKittyGraphics(apc)
234+ } else {
235+ s.abortKittyPendingDisplay()
231 }236 }
232 return data[end:]237 return data[end:]
233 }238 }
@@ -236,6 +241,8 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte {
236 if len(prefix)+end <= maxKittyEscapeBufferLen {241 if len(prefix)+end <= maxKittyEscapeBufferLen {
237 apc := append(prefix, data[:end]...)242 apc := append(prefix, data[:end]...)
238 s.dispatchKittyGraphics(apc)243 s.dispatchKittyGraphics(apc)
244+ } else {
245+ s.abortKittyPendingDisplay()
239 }246 }
240 return data[end:]247 return data[end:]
241 }248 }
@@ -246,8 +253,13 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte {
246 s.escapeBuf = returnData253 s.escapeBuf = returnData
247 return nil254 return nil
248 }255 }
256+ s.abortKittyPendingDisplay()
249 s.kittyDiscard = true257 s.kittyDiscard = true
250- s.kittyDiscardEscaped = false258+ escaped := len(prefix) != 0 && prefix[len(prefix)-1] == 0x1b
259+ if remaining > 0 {
260+ escaped = data[remaining-1] == 0x1b
261+ }
262+ s.kittyDiscardEscaped = escaped
251 tail := data[remaining:]263 tail := data[remaining:]
252 return tail[s.consumeKittyDiscard(tail):]264 return tail[s.consumeKittyDiscard(tail):]
253 }265 }
@@ -349,12 +361,22 @@ func (s *Screen) consumeKittyGraphics(data []byte) (consumed int, partial bool)
349 for i := 3; i < len(data); i++ {361 for i := 3; i < len(data); i++ {
350 switch data[i] {362 switch data[i] {
351 case 0x9c: // C1 string terminator.363 case 0x9c: // C1 string terminator.
352- s.dispatchKittyGraphics(data[:i+1])364+ end := i + 1
353- return i + 1, false365+ if end > maxKittyEscapeBufferLen {
366+ s.abortKittyPendingDisplay()
367+ return end, false
368+ }
369+ s.dispatchKittyGraphics(data[:end])
370+ return end, false
354 case 0x1b:371 case 0x1b:
355 if i+1 < len(data) && data[i+1] == '\\' {372 if i+1 < len(data) && data[i+1] == '\\' {
356- s.dispatchKittyGraphics(data[:i+2])373+ end := i + 2
357- return i + 2, false374+ if end > maxKittyEscapeBufferLen {
375+ s.abortKittyPendingDisplay()
376+ return end, false
377+ }
378+ s.dispatchKittyGraphics(data[:end])
379+ return end, false
358 }380 }
359 }381 }
360 }382 }
@@ -362,13 +384,14 @@ func (s *Screen) consumeKittyGraphics(data []byte) (consumed int, partial bool)
362 }384 }
363 385
364 func (s *Screen) dispatchKittyGraphics(apc []byte) {386 func (s *Screen) dispatchKittyGraphics(apc []byte) {
365- if s.graphics == nil {
366- s.graphics = newScreenGraphicsState()
367- }
368 command, err := kittygraphics.ParseAPC(apc)387 command, err := kittygraphics.ParseAPC(apc)
369 if err != nil {388 if err != nil {
389+ s.abortKittyPendingDisplay()
370 return390 return
371 }391 }
392+ if s.graphics == nil {
393+ s.graphics = newScreenGraphicsState()
394+ }
372 controls := command.Controls395 controls := command.Controls
373 pending := s.kittyPendingDisplay396 pending := s.kittyPendingDisplay
374 continuation := pending != nil397 continuation := pending != nil
@@ -405,7 +428,7 @@ func (s *Screen) dispatchKittyGraphics(apc []byte) {
405 }428 }
406 result, processErr := s.graphics.kitty.Process(command)429 result, processErr := s.graphics.kitty.Process(command)
407 if processErr != nil {430 if processErr != nil {
408- s.kittyPendingDisplay = nil431+ s.abortKittyPendingDisplay()
409 }432 }
410 for _, response := range result.Responses {433 for _, response := range result.Responses {
411 s.respond(response)434 s.respond(response)
@@ -422,25 +445,32 @@ func (s *Screen) dispatchKittyGraphics(apc []byte) {
422 }445 }
423 446
424 func (s *Screen) kittyCursorX() uint64 {447 func (s *Screen) kittyCursorX() uint64 {
425- if s.geometry.PixelWidth > 0 && s.geometry.Cols > 0 {448+ if cellWidth, _, ok := s.kittyCellPixels(); ok {
426- cellWidth := s.geometry.PixelWidth / s.geometry.Cols449+ return uint64(s.Col * cellWidth)
427- if cellWidth > 0 {
428- return uint64(s.Col * cellWidth)
429- }
430 }450 }
431 return uint64(s.Col)451 return uint64(s.Col)
432 }452 }
433 453
434 func (s *Screen) kittyCursorY() uint64 {454 func (s *Screen) kittyCursorY() uint64 {
435- if s.geometry.PixelHeight > 0 && s.geometry.Rows > 0 {455+ _, cellHeight, ok := s.kittyCellPixels()
436- cellHeight := s.geometry.PixelHeight / s.geometry.Rows456+ if ok {
437- if cellHeight > 0 {457+ return uint64(s.Row * cellHeight)
438- return uint64(s.Row * cellHeight)
439- }
440 }458 }
441 return uint64(s.Row)459 return uint64(s.Row)
442 }460 }
443 461
462+func (s *Screen) kittyCellPixels() (width, height int, ok bool) {
463+ if !s.geometry.PixelsKnown() || s.geometry.Cols <= 0 || s.geometry.Rows <= 0 {
464+ return 0, 0, false
465+ }
466+ width = s.geometry.PixelWidth / s.geometry.Cols
467+ height = s.geometry.PixelHeight / s.geometry.Rows
468+ if width <= 0 || height <= 0 {
469+ return 0, 0, false
470+ }
471+ return width, height, true
472+}
473+
444 func (s *Screen) applyKittyCursorMovement(c kittygraphics.Controls) {474 func (s *Screen) applyKittyCursorMovement(c kittygraphics.Controls) {
445 if c.HasCursor && c.Cursor == 1 {475 if c.HasCursor && c.Cursor == 1 {
446 return476 return
M screen_geometry.go +6 -0
@@ -16,9 +16,15 @@ func (s *Screen) SetGeometry(geometry Geometry) {
1616 return
1717 }
1818 cellsChanged := s.geometry.Cols != geometry.Cols || s.geometry.Rows != geometry.Rows
19+ geometryChanged := s.geometry != geometry
1920 s.geometry = geometry
2021 if cellsChanged {
2122 s.Resize(geometry.Cols, geometry.Rows)
23+ } else if geometryChanged {
24+ s.escapeBuf = s.escapeBuf[:0]
25+ s.kittyDiscard = false
26+ s.kittyDiscardEscaped = false
27+ s.abortAllKittyPending()
2228 }
2329 }
2430
@@ -16,9 +16,15 @@ func (s *Screen) SetGeometry(geometry Geometry) {
16 return16 return
17 }17 }
18 cellsChanged := s.geometry.Cols != geometry.Cols || s.geometry.Rows != geometry.Rows18 cellsChanged := s.geometry.Cols != geometry.Cols || s.geometry.Rows != geometry.Rows
19+ geometryChanged := s.geometry != geometry
19 s.geometry = geometry20 s.geometry = geometry
20 if cellsChanged {21 if cellsChanged {
21 s.Resize(geometry.Cols, geometry.Rows)22 s.Resize(geometry.Cols, geometry.Rows)
23+ } else if geometryChanged {
24+ s.escapeBuf = s.escapeBuf[:0]
25+ s.kittyDiscard = false
26+ s.kittyDiscardEscaped = false
27+ s.abortAllKittyPending()
22 }28 }
23 }29 }
24 30
M screen_graphics_test.go +40 -1
@@ -88,6 +88,45 @@ func TestScreenFullClearAbortsPendingUploadAndClearsPlacements(t *testing.T) {
8888 require.Equal(t, uint64(0), graphicsSnapshot.Usage().Placements)
8989 }
9090
91+func TestScreenPendingTransmitDisplayFailsClosedAcrossErrorAndResize(t *testing.T) {
92+ pending := []byte("\x1b_Ga=T,i=1,f=32,s=1,v=1,m=1,C=1;AQ\x1b\\")
93+ continuation := []byte("\x1b_Gm=0;IDBA\x1b\\")
94+
95+ screen := NewScreen(4, 2)
96+ screen.Write(pending)
97+ screen.Write([]byte("\x1b_Ga=p,i=1;\x1b\\"))
98+ screen.Write(continuation)
99+ require.NotNil(t, screen.GraphicsSnapshot())
100+ require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements)
101+
102+ screen.Write(pending)
103+ screen.Resize(4, 2)
104+ screen.Write(continuation)
105+ require.NotNil(t, screen.GraphicsSnapshot())
106+ require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements)
107+}
108+
109+func TestScreenPendingTransmitDisplayIsScopedToScreenBuffer(t *testing.T) {
110+ screen := NewScreen(4, 2)
111+ screen.Write([]byte("\x1b_Ga=T,i=1,f=32,s=1,v=1,m=1,C=1;AQ\x1b\\"))
112+ screen.Write([]byte("\x1b[?1049h"))
113+ screen.Write([]byte("\x1b_Gm=0;IDBA\x1b\\"))
114+ require.NotNil(t, screen.GraphicsSnapshot())
115+ require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements)
116+ screen.Write([]byte("\x1b[?1049l"))
117+ screen.Write([]byte("\x1b_Gm=0;IDBA\x1b\\"))
118+ require.Equal(t, uint64(1), screen.GraphicsSnapshot().Usage().Placements)
119+}
120+
121+func TestScreenPartialPixelGeometryUsesCellCoordinatesForBothAxes(t *testing.T) {
122+ screen := NewScreen(10, 4)
123+ screen.SetGeometry(Geometry{Cols: 10, Rows: 4, PixelWidth: 100})
124+ screen.Write([]byte("\x1b[2;3H"))
125+ screen.Write(screenKittyImageAPC("T"))
126+ placement := screen.GraphicsSnapshot().Placements()[0]
127+ require.Equal(t, graphics.PixelRect{X: 2, Y: 1, Width: 1, Height: 1}, placement.Destination())
128+}
129+
91130 func TestScreenKittyGraphicsOrdinaryResetClearScrollAndResize(t *testing.T) {
92131 screen := NewScreen(3, 2)
93132 screen.Write(screenKittyImageAPC("T"))
@@ -133,7 +172,7 @@ func TestScreenSnapshotCapturesOwnedActiveGraphicsSnapshot(t *testing.T) {
133172 func TestScreenOrdinaryTextPathDoesNotAllocateGraphicsState(t *testing.T) {
134173 screen := NewScreen(4, 2)
135174 screen.Write([]byte("text"))
136- require.Nil(t, screen.GraphicsScene())
175+ require.Nil(t, screen.GraphicsSnapshot())
137176 require.Nil(t, screen.Snapshot().GraphicsSnapshot())
138177 require.Equal(t, "text", rowText(screen.Snapshot().Row(0)))
139178 }
@@ -88,6 +88,45 @@ func TestScreenFullClearAbortsPendingUploadAndClearsPlacements(t *testing.T) {
88 require.Equal(t, uint64(0), graphicsSnapshot.Usage().Placements)88 require.Equal(t, uint64(0), graphicsSnapshot.Usage().Placements)
89 }89 }
90 90
91+func TestScreenPendingTransmitDisplayFailsClosedAcrossErrorAndResize(t *testing.T) {
92+ pending := []byte("\x1b_Ga=T,i=1,f=32,s=1,v=1,m=1,C=1;AQ\x1b\\")
93+ continuation := []byte("\x1b_Gm=0;IDBA\x1b\\")
94+
95+ screen := NewScreen(4, 2)
96+ screen.Write(pending)
97+ screen.Write([]byte("\x1b_Ga=p,i=1;\x1b\\"))
98+ screen.Write(continuation)
99+ require.NotNil(t, screen.GraphicsSnapshot())
100+ require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements)
101+
102+ screen.Write(pending)
103+ screen.Resize(4, 2)
104+ screen.Write(continuation)
105+ require.NotNil(t, screen.GraphicsSnapshot())
106+ require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements)
107+}
108+
109+func TestScreenPendingTransmitDisplayIsScopedToScreenBuffer(t *testing.T) {
110+ screen := NewScreen(4, 2)
111+ screen.Write([]byte("\x1b_Ga=T,i=1,f=32,s=1,v=1,m=1,C=1;AQ\x1b\\"))
112+ screen.Write([]byte("\x1b[?1049h"))
113+ screen.Write([]byte("\x1b_Gm=0;IDBA\x1b\\"))
114+ require.NotNil(t, screen.GraphicsSnapshot())
115+ require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements)
116+ screen.Write([]byte("\x1b[?1049l"))
117+ screen.Write([]byte("\x1b_Gm=0;IDBA\x1b\\"))
118+ require.Equal(t, uint64(1), screen.GraphicsSnapshot().Usage().Placements)
119+}
120+
121+func TestScreenPartialPixelGeometryUsesCellCoordinatesForBothAxes(t *testing.T) {
122+ screen := NewScreen(10, 4)
123+ screen.SetGeometry(Geometry{Cols: 10, Rows: 4, PixelWidth: 100})
124+ screen.Write([]byte("\x1b[2;3H"))
125+ screen.Write(screenKittyImageAPC("T"))
126+ placement := screen.GraphicsSnapshot().Placements()[0]
127+ require.Equal(t, graphics.PixelRect{X: 2, Y: 1, Width: 1, Height: 1}, placement.Destination())
128+}
129+
91 func TestScreenKittyGraphicsOrdinaryResetClearScrollAndResize(t *testing.T) {130 func TestScreenKittyGraphicsOrdinaryResetClearScrollAndResize(t *testing.T) {
92 screen := NewScreen(3, 2)131 screen := NewScreen(3, 2)
93 screen.Write(screenKittyImageAPC("T"))132 screen.Write(screenKittyImageAPC("T"))
@@ -133,7 +172,7 @@ func TestScreenSnapshotCapturesOwnedActiveGraphicsSnapshot(t *testing.T) {
133 func TestScreenOrdinaryTextPathDoesNotAllocateGraphicsState(t *testing.T) {172 func TestScreenOrdinaryTextPathDoesNotAllocateGraphicsState(t *testing.T) {
134 screen := NewScreen(4, 2)173 screen := NewScreen(4, 2)
135 screen.Write([]byte("text"))174 screen.Write([]byte("text"))
136- require.Nil(t, screen.GraphicsScene())175+ require.Nil(t, screen.GraphicsSnapshot())
137 require.Nil(t, screen.Snapshot().GraphicsSnapshot())176 require.Nil(t, screen.Snapshot().GraphicsSnapshot())
138 require.Equal(t, "text", rowText(screen.Snapshot().Row(0)))177 require.Equal(t, "text", rowText(screen.Snapshot().Row(0)))
139 }178 }
M screen_modes.go +30 -23
@@ -1,19 +1,23 @@
11 package vt
22
3-import renderer "github.com/bnema/vev-vt/core"
3+import (
4+ renderer "github.com/bnema/vev-vt/core"
5+ "github.com/bnema/vev-vt/protocol/kittygraphics"
6+)
47
58 type screenState struct {
6- frame renderer.Frame
7- buffer *buffer
8- row int
9- col int
10- style renderer.Style
11- scrollTop int
12- scrollBottom int
13- savedCursor cursorState
14- graphics *screenGraphicsState
15- originMode bool
16- insertMode bool
9+ frame renderer.Frame
10+ buffer *buffer
11+ row int
12+ col int
13+ style renderer.Style
14+ scrollTop int
15+ scrollBottom int
16+ savedCursor cursorState
17+ graphics *screenGraphicsState
18+ kittyPendingDisplay *kittygraphics.Controls
19+ originMode bool
20+ insertMode bool
1721 }
1822
1923 // SyncUpdateActive reports whether DEC private mode 2026 (synchronized update)
@@ -198,19 +202,21 @@ func (s *Screen) enterAlternateScreen() {
198202 return
199203 }
200204 s.alternate = &screenState{
201- frame: cloneFrame(s.Frame),
202- buffer: s.buffer.clone(),
203- row: s.Row,
204- col: s.Col,
205- style: s.Style,
206- scrollTop: s.scrollTop,
207- scrollBottom: s.scrollBottom,
208- savedCursor: s.savedCursor,
209- graphics: s.graphics,
210- originMode: s.originMode,
211- insertMode: s.insertMode,
205+ frame: cloneFrame(s.Frame),
206+ buffer: s.buffer.clone(),
207+ row: s.Row,
208+ col: s.Col,
209+ style: s.Style,
210+ scrollTop: s.scrollTop,
211+ scrollBottom: s.scrollBottom,
212+ savedCursor: s.savedCursor,
213+ graphics: s.graphics,
214+ kittyPendingDisplay: s.kittyPendingDisplay,
215+ originMode: s.originMode,
216+ insertMode: s.insertMode,
212217 }
213218 s.graphics = nil
219+ s.kittyPendingDisplay = nil
214220 s.buffer = s.newBuffer(s.Frame.Width, s.Frame.Height)
215221 s.Frame = s.buffer.frame
216222 s.Row, s.Col = 0, 0
@@ -238,6 +244,7 @@ func (s *Screen) exitAlternateScreen() {
238244 s.scrollBottom = state.scrollBottom
239245 s.savedCursor = state.savedCursor
240246 s.graphics = state.graphics
247+ s.kittyPendingDisplay = state.kittyPendingDisplay
241248 s.originMode = state.originMode
242249 s.insertMode = state.insertMode
243250 s.alternate = nil
@@ -1,19 +1,23 @@
1 package vt1 package vt
2 2
3-import renderer "github.com/bnema/vev-vt/core"3+import (
4+ renderer "github.com/bnema/vev-vt/core"
5+ "github.com/bnema/vev-vt/protocol/kittygraphics"
6+)
4 7
5 type screenState struct {8 type screenState struct {
6- frame renderer.Frame9+ frame renderer.Frame
7- buffer *buffer10+ buffer *buffer
8- row int11+ row int
9- col int12+ col int
10- style renderer.Style13+ style renderer.Style
11- scrollTop int14+ scrollTop int
12- scrollBottom int15+ scrollBottom int
13- savedCursor cursorState16+ savedCursor cursorState
14- graphics *screenGraphicsState17+ graphics *screenGraphicsState
15- originMode bool18+ kittyPendingDisplay *kittygraphics.Controls
16- insertMode bool19+ originMode bool
20+ insertMode bool
17 }21 }
18 22
19 // SyncUpdateActive reports whether DEC private mode 2026 (synchronized update)23 // SyncUpdateActive reports whether DEC private mode 2026 (synchronized update)
@@ -198,19 +202,21 @@ func (s *Screen) enterAlternateScreen() {
198 return202 return
199 }203 }
200 s.alternate = &screenState{204 s.alternate = &screenState{
201- frame: cloneFrame(s.Frame),205+ frame: cloneFrame(s.Frame),
202- buffer: s.buffer.clone(),206+ buffer: s.buffer.clone(),
203- row: s.Row,207+ row: s.Row,
204- col: s.Col,208+ col: s.Col,
205- style: s.Style,209+ style: s.Style,
206- scrollTop: s.scrollTop,210+ scrollTop: s.scrollTop,
207- scrollBottom: s.scrollBottom,211+ scrollBottom: s.scrollBottom,
208- savedCursor: s.savedCursor,212+ savedCursor: s.savedCursor,
209- graphics: s.graphics,213+ graphics: s.graphics,
210- originMode: s.originMode,214+ kittyPendingDisplay: s.kittyPendingDisplay,
211- insertMode: s.insertMode,215+ originMode: s.originMode,
216+ insertMode: s.insertMode,
212 }217 }
213 s.graphics = nil218 s.graphics = nil
219+ s.kittyPendingDisplay = nil
214 s.buffer = s.newBuffer(s.Frame.Width, s.Frame.Height)220 s.buffer = s.newBuffer(s.Frame.Width, s.Frame.Height)
215 s.Frame = s.buffer.frame221 s.Frame = s.buffer.frame
216 s.Row, s.Col = 0, 0222 s.Row, s.Col = 0, 0
@@ -238,6 +244,7 @@ func (s *Screen) exitAlternateScreen() {
238 s.scrollBottom = state.scrollBottom244 s.scrollBottom = state.scrollBottom
239 s.savedCursor = state.savedCursor245 s.savedCursor = state.savedCursor
240 s.graphics = state.graphics246 s.graphics = state.graphics
247+ s.kittyPendingDisplay = state.kittyPendingDisplay
241 s.originMode = state.originMode248 s.originMode = state.originMode
242 s.insertMode = state.insertMode249 s.insertMode = state.insertMode
243 s.alternate = nil250 s.alternate = nil
M screen_resize.go +5 -1
@@ -5,6 +5,10 @@ import renderer "github.com/bnema/vev-vt/core"
55 func (s *Screen) Resize(width, height int) {
66 if width == s.Frame.Width && height == s.Frame.Height {
77 s.geometry.Cols, s.geometry.Rows = width, height
8+ s.escapeBuf = s.escapeBuf[:0]
9+ s.kittyDiscard = false
10+ s.kittyDiscardEscaped = false
11+ s.abortAllKittyPending()
812 return
913 }
1014 if width <= 0 || height <= 0 {
@@ -67,7 +71,7 @@ func (s *Screen) Resize(width, height int) {
6771 s.escapeBuf = s.escapeBuf[:0]
6872 s.kittyDiscard = false
6973 s.kittyDiscardEscaped = false
70- s.kittyPendingDisplay = nil
74+ s.abortAllKittyPending()
7175 s.resetScrollRegion()
7276 s.fullRedraw()
7377 }
@@ -5,6 +5,10 @@ import renderer "github.com/bnema/vev-vt/core"
5 func (s *Screen) Resize(width, height int) {5 func (s *Screen) Resize(width, height int) {
6 if width == s.Frame.Width && height == s.Frame.Height {6 if width == s.Frame.Width && height == s.Frame.Height {
7 s.geometry.Cols, s.geometry.Rows = width, height7 s.geometry.Cols, s.geometry.Rows = width, height
8+ s.escapeBuf = s.escapeBuf[:0]
9+ s.kittyDiscard = false
10+ s.kittyDiscardEscaped = false
11+ s.abortAllKittyPending()
8 return12 return
9 }13 }
10 if width <= 0 || height <= 0 {14 if width <= 0 || height <= 0 {
@@ -67,7 +71,7 @@ func (s *Screen) Resize(width, height int) {
67 s.escapeBuf = s.escapeBuf[:0]71 s.escapeBuf = s.escapeBuf[:0]
68 s.kittyDiscard = false72 s.kittyDiscard = false
69 s.kittyDiscardEscaped = false73 s.kittyDiscardEscaped = false
70- s.kittyPendingDisplay = nil74+ s.abortAllKittyPending()
71 s.resetScrollRegion()75 s.resetScrollRegion()
72 s.fullRedraw()76 s.fullRedraw()
73 }77 }

Keyboard shortcuts

Single-key shortcuts are on. ctrl k and esc work whatever this says.

ctrl k :
go to a section, branch, tag, file or repository
?
this list
j k
move the row cursor down / up
enter
open the row under the cursor; on Files, a directory opens or closes in place
↑ ↓ ← → home end
in the file tree (focus on a row): previous / next row, close / open a directory or go to its parent / first entry, first / last row. They never open a file
l h
on Files: open a directory or a file / close a directory or go up to its parent, then to the parent page. Elsewhere: into the side panel (on a merge request, its files) / back to the content; there j k move between its controls
g g G
first / last row
/
filter the list; on Files, find a file anywhere in the repository (esc clears, enter opens the best match)
b
switch branch or tag
g s g f g c g b g m g i g r g a
tabs: summary, files, commits, branches & tags, merge requests, issues, releases, actions (when available)
g h
home
w
wide mode: use the whole width of the screen, or not (remembered)
y
copy a permalink pinned to the commit
d u x
diff: split / unified / collapse or expand all
e E
diff: the whole file in view / every file, with the lines the hunks hide
n
merge request: next note in the diff
esc
close a dialog, leave a field

Branches 2

Tags 13

↑↓ moveenter openesc close