fix(graphics): validate placement scrolling
- commit
- a67301cbb68aa950cc595694fd11e246c88eff68
- author
- brice · · 2026-08-26 15:30 UTC
- parent
- 47ca903
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +42 -2
M protocol/kittygraphics/session.go +1 -1
| @@ -407,7 +407,7 @@ func decodeImagePayload(payload []byte, controls Controls, limits Limits) ([]byt | |||
| 407 | 407 | if err != nil { | |
| 408 | 408 | return nil, fmt.Errorf("%w: zlib: %v", ErrInvalidCommand, err) | |
| 409 | 409 | } | |
| 410 | - | defer zr.Close() | |
| 410 | + | defer func() { _ = zr.Close() }() | |
| 411 | 411 | limit := limits.MaxDecodedBytes | |
| 412 | 412 | if limit >= math.MaxInt64 { | |
| 413 | 413 | limit = math.MaxInt64 - 1 | |
| @@ -407,7 +407,7 @@ func decodeImagePayload(payload []byte, controls Controls, limits Limits) ([]byt | |||||
| 407 | if err != nil { | 407 | if err != nil { | ||
| 408 | return nil, fmt.Errorf("%w: zlib: %v", ErrInvalidCommand, err) | 408 | return nil, fmt.Errorf("%w: zlib: %v", ErrInvalidCommand, err) | ||
| 409 | } | 409 | } | ||
| 410 | - | defer zr.Close() | 410 | + | defer func() { _ = zr.Close() }() |
| 411 | limit := limits.MaxDecodedBytes | 411 | limit := limits.MaxDecodedBytes | ||
| 412 | if limit >= math.MaxInt64 { | 412 | if limit >= math.MaxInt64 { | ||
| 413 | limit = math.MaxInt64 - 1 | 413 | limit = math.MaxInt64 - 1 | ||
M screen_graphics_test.go +34 -0
| @@ -2,6 +2,7 @@ package vt | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "encoding/base64" | |
| 5 | + | "math" | |
| 5 | 6 | "strconv" | |
| 6 | 7 | "testing" | |
| 7 | 8 | ||
| @@ -283,3 +284,36 @@ func TestScreenKittyPlacementScrollsWithTallImageCursorAdvance(t *testing.T) { | |||
| 283 | 284 | require.Equal(t, 4, screen.CursorRow()) | |
| 284 | 285 | require.Equal(t, 2, screen.CursorCol()) | |
| 285 | 286 | } | |
| 287 | + | ||
| 288 | + | func TestScreenKittyPlacementScrollsOnlyWhenContainedInRegion(t *testing.T) { | |
| 289 | + | screen := NewScreen(10, 6) | |
| 290 | + | screen.SetGeometry(Geometry{Cols: 10, Rows: 6, PixelWidth: 100, PixelHeight: 60}) | |
| 291 | + | screen.Write([]byte("\x1b[2;5r")) | |
| 292 | + | ||
| 293 | + | insidePayload := base64.RawStdEncoding.EncodeToString(make([]byte, 1*10*4)) | |
| 294 | + | screen.Write([]byte("\x1b[3;1H\x1b_Ga=T,i=1,f=32,s=1,v=10,C=1;" + insidePayload + "\x1b\\")) | |
| 295 | + | straddlingPayload := base64.RawStdEncoding.EncodeToString(make([]byte, 1*20*4)) | |
| 296 | + | screen.Write([]byte("\x1b[1;1H\x1b_Ga=T,i=2,f=32,s=1,v=20,Y=5,C=1;" + straddlingPayload + "\x1b\\")) | |
| 297 | + | ||
| 298 | + | screen.Write([]byte("\x1b[S")) | |
| 299 | + | ||
| 300 | + | placements := screen.GraphicsSnapshot().Placements() | |
| 301 | + | require.Len(t, placements, 2) | |
| 302 | + | require.Equal(t, graphics.PixelRect{X: 0, Y: 10, Width: 1, Height: 10}, placements[0].Destination(), "a placement contained in the margin must scroll") | |
| 303 | + | require.Equal(t, graphics.PixelRect{X: 0, Y: 5, Width: 1, Height: 20}, placements[1].Destination(), "a placement crossing the margin must remain fixed") | |
| 304 | + | } | |
| 305 | + | ||
| 306 | + | func TestScreenKittyPlacementScrollSkipsBottomEdgeOverflow(t *testing.T) { | |
| 307 | + | screen := NewScreen(1, 2) | |
| 308 | + | screen.SetGeometry(Geometry{Cols: 1, Rows: 2, PixelWidth: 10, PixelHeight: 20}) | |
| 309 | + | payload := base64.RawStdEncoding.EncodeToString(make([]byte, 1*10*4)) | |
| 310 | + | screen.Write([]byte("\x1b_Ga=T,i=1,f=32,s=1,v=10,C=1;" + payload + "\x1b\\")) | |
| 311 | + | placement := screen.GraphicsSnapshot().Placements()[0] | |
| 312 | + | nearLimit := graphics.PixelRect{X: 0, Y: math.MaxInt64 - 11, Width: 1, Height: 10} | |
| 313 | + | require.NoError(t, screen.graphics.scene.UpdatePlacement(placement.ID(), graphics.PlacementSpec{Destination: nearLimit})) | |
| 314 | + | ||
| 315 | + | require.NotPanics(t, func() { screen.Write([]byte("\x1b[T")) }) | |
| 316 | + | ||
| 317 | + | placement = screen.GraphicsSnapshot().Placements()[0] | |
| 318 | + | require.Equal(t, nearLimit, placement.Destination()) | |
| 319 | + | } | |
| @@ -2,6 +2,7 @@ package vt | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "encoding/base64" | 4 | "encoding/base64" | ||
| 5 | + | "math" | |||
| 5 | "strconv" | 6 | "strconv" | ||
| 6 | "testing" | 7 | "testing" | ||
| 7 | 8 | ||||
| @@ -283,3 +284,36 @@ func TestScreenKittyPlacementScrollsWithTallImageCursorAdvance(t *testing.T) { | |||||
| 283 | require.Equal(t, 4, screen.CursorRow()) | 284 | require.Equal(t, 4, screen.CursorRow()) | ||
| 284 | require.Equal(t, 2, screen.CursorCol()) | 285 | require.Equal(t, 2, screen.CursorCol()) | ||
| 285 | } | 286 | } | ||
| 287 | + | ||||
| 288 | + | func TestScreenKittyPlacementScrollsOnlyWhenContainedInRegion(t *testing.T) { | |||
| 289 | + | screen := NewScreen(10, 6) | |||
| 290 | + | screen.SetGeometry(Geometry{Cols: 10, Rows: 6, PixelWidth: 100, PixelHeight: 60}) | |||
| 291 | + | screen.Write([]byte("\x1b[2;5r")) | |||
| 292 | + | ||||
| 293 | + | insidePayload := base64.RawStdEncoding.EncodeToString(make([]byte, 1*10*4)) | |||
| 294 | + | screen.Write([]byte("\x1b[3;1H\x1b_Ga=T,i=1,f=32,s=1,v=10,C=1;" + insidePayload + "\x1b\\")) | |||
| 295 | + | straddlingPayload := base64.RawStdEncoding.EncodeToString(make([]byte, 1*20*4)) | |||
| 296 | + | screen.Write([]byte("\x1b[1;1H\x1b_Ga=T,i=2,f=32,s=1,v=20,Y=5,C=1;" + straddlingPayload + "\x1b\\")) | |||
| 297 | + | ||||
| 298 | + | screen.Write([]byte("\x1b[S")) | |||
| 299 | + | ||||
| 300 | + | placements := screen.GraphicsSnapshot().Placements() | |||
| 301 | + | require.Len(t, placements, 2) | |||
| 302 | + | require.Equal(t, graphics.PixelRect{X: 0, Y: 10, Width: 1, Height: 10}, placements[0].Destination(), "a placement contained in the margin must scroll") | |||
| 303 | + | require.Equal(t, graphics.PixelRect{X: 0, Y: 5, Width: 1, Height: 20}, placements[1].Destination(), "a placement crossing the margin must remain fixed") | |||
| 304 | + | } | |||
| 305 | + | ||||
| 306 | + | func TestScreenKittyPlacementScrollSkipsBottomEdgeOverflow(t *testing.T) { | |||
| 307 | + | screen := NewScreen(1, 2) | |||
| 308 | + | screen.SetGeometry(Geometry{Cols: 1, Rows: 2, PixelWidth: 10, PixelHeight: 20}) | |||
| 309 | + | payload := base64.RawStdEncoding.EncodeToString(make([]byte, 1*10*4)) | |||
| 310 | + | screen.Write([]byte("\x1b_Ga=T,i=1,f=32,s=1,v=10,C=1;" + payload + "\x1b\\")) | |||
| 311 | + | placement := screen.GraphicsSnapshot().Placements()[0] | |||
| 312 | + | nearLimit := graphics.PixelRect{X: 0, Y: math.MaxInt64 - 11, Width: 1, Height: 10} | |||
| 313 | + | require.NoError(t, screen.graphics.scene.UpdatePlacement(placement.ID(), graphics.PlacementSpec{Destination: nearLimit})) | |||
| 314 | + | ||||
| 315 | + | require.NotPanics(t, func() { screen.Write([]byte("\x1b[T")) }) | |||
| 316 | + | ||||
| 317 | + | placement = screen.GraphicsSnapshot().Placements()[0] | |||
| 318 | + | require.Equal(t, nearLimit, placement.Destination()) | |||
| 319 | + | } | |||
M screen_scroll.go +7 -1
| @@ -2,6 +2,7 @@ package vt | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "errors" | |
| 5 | + | "fmt" | |
| 5 | 6 | "math" | |
| 6 | 7 | ||
| 7 | 8 | renderer "github.com/bnema/vev-vt/core" | |
| @@ -175,12 +176,17 @@ func (s *Screen) scrollGraphicsRegion(top, bottom, rows int) { | |||
| 175 | 176 | continue | |
| 176 | 177 | } | |
| 177 | 178 | destination.Y += delta | |
| 179 | + | if _, valid := destination.Bottom(); !valid { | |
| 180 | + | continue | |
| 181 | + | } | |
| 178 | 182 | spec := placement.Spec() | |
| 179 | 183 | spec.Destination = destination | |
| 180 | 184 | operations = append(operations, graphics.Operation{Kind: graphics.OperationUpdatePlacement, PlacementID: placement.ID(), Placement: spec}) | |
| 181 | 185 | } | |
| 182 | 186 | if len(operations) != 0 { | |
| 183 | - | _ = s.graphics.scene.Apply(operations...) | |
| 187 | + | if err := s.graphics.scene.Apply(operations...); err != nil { | |
| 188 | + | panic(fmt.Errorf("scroll graphics placements: %w", err)) | |
| 189 | + | } | |
| 184 | 190 | } | |
| 185 | 191 | } | |
| 186 | 192 | ||
| @@ -2,6 +2,7 @@ package vt | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "errors" | 4 | "errors" | ||
| 5 | + | "fmt" | |||
| 5 | "math" | 6 | "math" | ||
| 6 | 7 | ||||
| 7 | renderer "github.com/bnema/vev-vt/core" | 8 | renderer "github.com/bnema/vev-vt/core" | ||
| @@ -175,12 +176,17 @@ func (s *Screen) scrollGraphicsRegion(top, bottom, rows int) { | |||||
| 175 | continue | 176 | continue | ||
| 176 | } | 177 | } | ||
| 177 | destination.Y += delta | 178 | destination.Y += delta | ||
| 179 | + | if _, valid := destination.Bottom(); !valid { | |||
| 180 | + | continue | |||
| 181 | + | } | |||
| 178 | spec := placement.Spec() | 182 | spec := placement.Spec() | ||
| 179 | spec.Destination = destination | 183 | spec.Destination = destination | ||
| 180 | operations = append(operations, graphics.Operation{Kind: graphics.OperationUpdatePlacement, PlacementID: placement.ID(), Placement: spec}) | 184 | operations = append(operations, graphics.Operation{Kind: graphics.OperationUpdatePlacement, PlacementID: placement.ID(), Placement: spec}) | ||
| 181 | } | 185 | } | ||
| 182 | if len(operations) != 0 { | 186 | if len(operations) != 0 { | ||
| 183 | - | _ = s.graphics.scene.Apply(operations...) | 187 | + | if err := s.graphics.scene.Apply(operations...); err != nil { |
| 188 | + | panic(fmt.Errorf("scroll graphics placements: %w", err)) | |||
| 189 | + | } | |||
| 184 | } | 190 | } | ||
| 185 | } | 191 | } | ||
| 186 | 192 | ||||