fix(graphics): address protocol review findings
- commit
- 78f404d584c1aaff37f42b16506c2d3502ad8510
- author
- brice · · 2026-08-23 16:15 UTC
- parent
- 52577cd
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
21 files · +280 -177
M README.md +3 -1
| @@ -56,7 +56,9 @@ and parser limits. File, temporary-file, shared-memory, animation, composition, | |||
| 56 | 56 | relative placement, and Unicode-placeholder commands remain unsupported. | |
| 57 | 57 | ||
| 58 | 58 | `Screen` allocates graphics state only after a Kitty APC. Graphics snapshots are | |
| 59 | - | separate from cells and history bytes; VTH3 remains text/history-only. | |
| 59 | + | separate from cells and history bytes; VTH3 remains text/history-only. Static | |
| 60 | + | placements retain their configured pixel rectangles across text scrolling and | |
| 61 | + | reflow; row-anchored and relative-placement movement is unsupported. | |
| 60 | 62 | ||
| 61 | 63 | ## Checks | |
| 62 | 64 | ||
| @@ -56,7 +56,9 @@ and parser limits. File, temporary-file, shared-memory, animation, composition, | |||||
| 56 | relative placement, and Unicode-placeholder commands remain unsupported. | 56 | relative placement, and Unicode-placeholder commands remain unsupported. | ||
| 57 | 57 | ||||
| 58 | `Screen` allocates graphics state only after a Kitty APC. Graphics snapshots are | 58 | `Screen` allocates graphics state only after a Kitty APC. Graphics snapshots are | ||
| 59 | - | separate from cells and history bytes; VTH3 remains text/history-only. | 59 | + | separate from cells and history bytes; VTH3 remains text/history-only. Static |
| 60 | + | placements retain their configured pixel rectangles across text scrolling and | |||
| 61 | + | reflow; row-anchored and relative-placement movement is unsupported. | |||
| 60 | 62 | ||||
| 61 | ## Checks | 63 | ## Checks | ||
| 62 | 64 | ||||
M benchmark_test.go +23 -0
| @@ -2,6 +2,7 @@ package vt | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "bytes" | |
| 5 | + | "encoding/base64" | |
| 5 | 6 | "testing" | |
| 6 | 7 | ||
| 7 | 8 | renderer "github.com/bnema/vev-vt/core" | |
| @@ -237,6 +238,28 @@ func BenchmarkScreenPrintableASCII(b *testing.B) { | |||
| 237 | 238 | } | |
| 238 | 239 | } | |
| 239 | 240 | ||
| 241 | + | func BenchmarkScreenKittyAPC(b *testing.B) { | |
| 242 | + | payload := base64.RawStdEncoding.EncodeToString(make([]byte, 256*256*4)) | |
| 243 | + | apc := []byte("\x1b_Ga=t,i=1,f=32,s=256,v=256,q=2;" + payload + "\x1b\\") | |
| 244 | + | b.SetBytes(int64(len(apc))) | |
| 245 | + | for _, fragmented := range []bool{false, true} { | |
| 246 | + | b.Run(map[bool]string{false: "single-write", true: "byte-by-byte"}[fragmented], func(b *testing.B) { | |
| 247 | + | s := NewScreen(120, 40) | |
| 248 | + | b.ReportAllocs() | |
| 249 | + | for b.Loop() { | |
| 250 | + | if fragmented { | |
| 251 | + | for _, part := range apc { | |
| 252 | + | s.Write([]byte{part}) | |
| 253 | + | } | |
| 254 | + | } else { | |
| 255 | + | s.Write(apc) | |
| 256 | + | } | |
| 257 | + | s.ClearDamage() | |
| 258 | + | } | |
| 259 | + | }) | |
| 260 | + | } | |
| 261 | + | } | |
| 262 | + | ||
| 240 | 263 | func BenchmarkScreenMixedUTF8(b *testing.B) { | |
| 241 | 264 | s := NewScreen(120, 40) | |
| 242 | 265 | chunk := append(bytes.Repeat([]byte("Aé界"), 20), '\r') | |
| @@ -2,6 +2,7 @@ package vt | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "bytes" | 4 | "bytes" | ||
| 5 | + | "encoding/base64" | |||
| 5 | "testing" | 6 | "testing" | ||
| 6 | 7 | ||||
| 7 | renderer "github.com/bnema/vev-vt/core" | 8 | renderer "github.com/bnema/vev-vt/core" | ||
| @@ -237,6 +238,28 @@ func BenchmarkScreenPrintableASCII(b *testing.B) { | |||||
| 237 | } | 238 | } | ||
| 238 | } | 239 | } | ||
| 239 | 240 | ||||
| 241 | + | func BenchmarkScreenKittyAPC(b *testing.B) { | |||
| 242 | + | payload := base64.RawStdEncoding.EncodeToString(make([]byte, 256*256*4)) | |||
| 243 | + | apc := []byte("\x1b_Ga=t,i=1,f=32,s=256,v=256,q=2;" + payload + "\x1b\\") | |||
| 244 | + | b.SetBytes(int64(len(apc))) | |||
| 245 | + | for _, fragmented := range []bool{false, true} { | |||
| 246 | + | b.Run(map[bool]string{false: "single-write", true: "byte-by-byte"}[fragmented], func(b *testing.B) { | |||
| 247 | + | s := NewScreen(120, 40) | |||
| 248 | + | b.ReportAllocs() | |||
| 249 | + | for b.Loop() { | |||
| 250 | + | if fragmented { | |||
| 251 | + | for _, part := range apc { | |||
| 252 | + | s.Write([]byte{part}) | |||
| 253 | + | } | |||
| 254 | + | } else { | |||
| 255 | + | s.Write(apc) | |||
| 256 | + | } | |||
| 257 | + | s.ClearDamage() | |||
| 258 | + | } | |||
| 259 | + | }) | |||
| 260 | + | } | |||
| 261 | + | } | |||
| 262 | + | ||||
| 240 | func BenchmarkScreenMixedUTF8(b *testing.B) { | 263 | func BenchmarkScreenMixedUTF8(b *testing.B) { | ||
| 241 | s := NewScreen(120, 40) | 264 | s := NewScreen(120, 40) | ||
| 242 | chunk := append(bytes.Repeat([]byte("Aé界"), 20), '\r') | 265 | chunk := append(bytes.Repeat([]byte("Aé界"), 20), '\r') | ||
M graphics/consumer_external_test.go +1 -1
| @@ -22,7 +22,7 @@ func TestExternalConsumerCanBuildAndReadImmutableScene(t *testing.T) { | |||
| 22 | 22 | data[0] = 'X' | |
| 23 | 23 | ||
| 24 | 24 | placementID, err := scene.AddPlacement(graphics.PlacementSpec{ | |
| 25 | - | AssetID: assetID, | |
| 25 | + | Asset: assetID, | |
| 26 | 26 | Destination: graphics.PixelRect{X: -2, Y: 3, Width: 16, Height: 8}, | |
| 27 | 27 | }) | |
| 28 | 28 | if err != nil { | |
| @@ -22,7 +22,7 @@ func TestExternalConsumerCanBuildAndReadImmutableScene(t *testing.T) { | |||||
| 22 | data[0] = 'X' | 22 | data[0] = 'X' | ||
| 23 | 23 | ||||
| 24 | placementID, err := scene.AddPlacement(graphics.PlacementSpec{ | 24 | placementID, err := scene.AddPlacement(graphics.PlacementSpec{ | ||
| 25 | - | AssetID: assetID, | 25 | + | Asset: assetID, |
| 26 | Destination: graphics.PixelRect{X: -2, Y: 3, Width: 16, Height: 8}, | 26 | Destination: graphics.PixelRect{X: -2, Y: 3, Width: 16, Height: 8}, | ||
| 27 | }) | 27 | }) | ||
| 28 | if err != nil { | 28 | if err != nil { | ||
M graphics/graphics.go +18 -13
| @@ -96,15 +96,25 @@ const ( | |||
| 96 | 96 | DefaultMaxDecodedPixelsPerAsset = uint64(128 << 20) | |
| 97 | 97 | ) | |
| 98 | 98 | ||
| 99 | + | // AssetFormat identifies the encoded pixel layout supplied by an asset. | |
| 100 | + | type AssetFormat uint8 | |
| 101 | + | ||
| 102 | + | const ( | |
| 103 | + | AssetFormatUnknown AssetFormat = iota | |
| 104 | + | AssetFormatRGB | |
| 105 | + | AssetFormatRGBA | |
| 106 | + | AssetFormatPNG | |
| 107 | + | ) | |
| 108 | + | ||
| 99 | 109 | // AssetBlob is the caller-owned description of one encoded asset. AddAsset | |
| 100 | 110 | // copies Encoded before returning, so later caller mutations cannot alter a | |
| 101 | - | // scene or any snapshot. Width and Height describe the decoded pixel extent | |
| 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. | |
| 111 | + | // scene or any snapshot. Format preserves the encoded pixel layout for the | |
| 112 | + | // renderer. Width and Height describe the decoded pixel extent used for | |
| 113 | + | // clipping and quota accounting. DecodedPixels, when non-zero, must equal | |
| 114 | + | // Width*Height; accounting always uses the checked product. | |
| 105 | 115 | type AssetBlob struct { | |
| 106 | 116 | Encoded []byte | |
| 107 | - | Data []byte | |
| 117 | + | Format AssetFormat | |
| 108 | 118 | Width, Height int64 | |
| 109 | 119 | DecodedPixels uint64 | |
| 110 | 120 | } | |
| @@ -113,18 +123,14 @@ type AssetBlob struct { | |||
| 113 | 123 | type AssetSpec = AssetBlob | |
| 114 | 124 | ||
| 115 | 125 | // PlacementSpec describes one sparse placement. Source defaults to the full | |
| 116 | - | // asset. Destination is a pixel rectangle; Dest is accepted as a concise alias | |
| 117 | - | // for callers that use that spelling. Cells optionally records the cell-space | |
| 118 | - | // extent associated with the same placement. HasCells and HasLayer preserve | |
| 119 | - | // explicit zero values when updating an existing placement. | |
| 126 | + | // asset. Destination is a pixel rectangle. Cells optionally records the | |
| 127 | + | // cell-space extent associated with the same placement. HasCells and HasLayer | |
| 128 | + | // preserve explicit zero values when updating an existing placement. | |
| 120 | 129 | type PlacementSpec struct { | |
| 121 | 130 | Asset AssetID | |
| 122 | - | AssetID AssetID | |
| 123 | 131 | Source PixelRect | |
| 124 | 132 | Destination PixelRect | |
| 125 | - | Dest PixelRect | |
| 126 | 133 | Cells CellRect | |
| 127 | - | CellBounds CellRect | |
| 128 | 134 | HasCells bool | |
| 129 | 135 | Layer int64 | |
| 130 | 136 | HasLayer bool | |
| @@ -178,7 +184,6 @@ type Operation struct { | |||
| 178 | 184 | Kind OperationKind | |
| 179 | 185 | Blob AssetBlob | |
| 180 | 186 | Asset AssetID | |
| 181 | - | AssetID AssetID | |
| 182 | 187 | Placement PlacementSpec | |
| 183 | 188 | PlacementID PlacementID | |
| 184 | 189 | } | |
| @@ -96,15 +96,25 @@ const ( | |||||
| 96 | DefaultMaxDecodedPixelsPerAsset = uint64(128 << 20) | 96 | DefaultMaxDecodedPixelsPerAsset = uint64(128 << 20) | ||
| 97 | ) | 97 | ) | ||
| 98 | 98 | ||||
| 99 | + | // AssetFormat identifies the encoded pixel layout supplied by an asset. | |||
| 100 | + | type AssetFormat uint8 | |||
| 101 | + | ||||
| 102 | + | const ( | |||
| 103 | + | AssetFormatUnknown AssetFormat = iota | |||
| 104 | + | AssetFormatRGB | |||
| 105 | + | AssetFormatRGBA | |||
| 106 | + | AssetFormatPNG | |||
| 107 | + | ) | |||
| 108 | + | ||||
| 99 | // AssetBlob is the caller-owned description of one encoded asset. AddAsset | 109 | // AssetBlob is the caller-owned description of one encoded asset. AddAsset | ||
| 100 | // copies Encoded before returning, so later caller mutations cannot alter a | 110 | // copies Encoded before returning, so later caller mutations cannot alter a | ||
| 101 | - | // scene or any snapshot. Width and Height describe the decoded pixel extent | 111 | + | // scene or any snapshot. Format preserves the encoded pixel layout for the |
| 102 | - | // used for clipping and quota accounting. DecodedPixels is an optional | 112 | + | // renderer. Width and Height describe the decoded pixel extent used for |
| 103 | - | // compatibility declaration; when non-zero it must equal Width*Height, and | 113 | + | // clipping and quota accounting. DecodedPixels, when non-zero, must equal |
| 104 | - | // accounting always uses the checked product of the dimensions. | 114 | + | // Width*Height; accounting always uses the checked product. |
| 105 | type AssetBlob struct { | 115 | type AssetBlob struct { | ||
| 106 | Encoded []byte | 116 | Encoded []byte | ||
| 107 | - | Data []byte | 117 | + | Format AssetFormat |
| 108 | Width, Height int64 | 118 | Width, Height int64 | ||
| 109 | DecodedPixels uint64 | 119 | DecodedPixels uint64 | ||
| 110 | } | 120 | } | ||
| @@ -113,18 +123,14 @@ type AssetBlob struct { | |||||
| 113 | type AssetSpec = AssetBlob | 123 | type AssetSpec = AssetBlob | ||
| 114 | 124 | ||||
| 115 | // PlacementSpec describes one sparse placement. Source defaults to the full | 125 | // PlacementSpec describes one sparse placement. Source defaults to the full | ||
| 116 | - | // asset. Destination is a pixel rectangle; Dest is accepted as a concise alias | 126 | + | // asset. Destination is a pixel rectangle. Cells optionally records the |
| 117 | - | // for callers that use that spelling. Cells optionally records the cell-space | 127 | + | // cell-space extent associated with the same placement. HasCells and HasLayer |
| 118 | - | // extent associated with the same placement. HasCells and HasLayer preserve | 128 | + | // preserve explicit zero values when updating an existing placement. |
| 119 | - | // explicit zero values when updating an existing placement. | |||
| 120 | type PlacementSpec struct { | 129 | type PlacementSpec struct { | ||
| 121 | Asset AssetID | 130 | Asset AssetID | ||
| 122 | - | AssetID AssetID | |||
| 123 | Source PixelRect | 131 | Source PixelRect | ||
| 124 | Destination PixelRect | 132 | Destination PixelRect | ||
| 125 | - | Dest PixelRect | |||
| 126 | Cells CellRect | 133 | Cells CellRect | ||
| 127 | - | CellBounds CellRect | |||
| 128 | HasCells bool | 134 | HasCells bool | ||
| 129 | Layer int64 | 135 | Layer int64 | ||
| 130 | HasLayer bool | 136 | HasLayer bool | ||
| @@ -178,7 +184,6 @@ type Operation struct { | |||||
| 178 | Kind OperationKind | 184 | Kind OperationKind | ||
| 179 | Blob AssetBlob | 185 | Blob AssetBlob | ||
| 180 | Asset AssetID | 186 | Asset AssetID | ||
| 181 | - | AssetID AssetID | |||
| 182 | Placement PlacementSpec | 187 | Placement PlacementSpec | ||
| 183 | PlacementID PlacementID | 188 | PlacementID PlacementID | ||
| 184 | } | 189 | } | ||
M graphics/rect.go +0 -14
| @@ -147,17 +147,3 @@ func checkedEdge(start, length int64) (int64, bool) { | |||
| 147 | 147 | } | |
| 148 | 148 | return start + length, true | |
| 149 | 149 | } | |
| 150 | - | ||
| 151 | - | func max(a, b int64) int64 { | |
| 152 | - | if a > b { | |
| 153 | - | return a | |
| 154 | - | } | |
| 155 | - | return b | |
| 156 | - | } | |
| 157 | - | ||
| 158 | - | func min(a, b int64) int64 { | |
| 159 | - | if a < b { | |
| 160 | - | return a | |
| 161 | - | } | |
| 162 | - | return b | |
| 163 | - | } | |
| @@ -147,17 +147,3 @@ func checkedEdge(start, length int64) (int64, bool) { | |||||
| 147 | } | 147 | } | ||
| 148 | return start + length, true | 148 | return start + length, true | ||
| 149 | } | 149 | } | ||
| 150 | - | ||||
| 151 | - | func max(a, b int64) int64 { | |||
| 152 | - | if a > b { | |||
| 153 | - | return a | |||
| 154 | - | } | |||
| 155 | - | return b | |||
| 156 | - | } | |||
| 157 | - | ||||
| 158 | - | func min(a, b int64) int64 { | |||
| 159 | - | if a < b { | |||
| 160 | - | return a | |||
| 161 | - | } | |||
| 162 | - | return b | |||
| 163 | - | } | |||
M graphics/scene.go +22 -30
| @@ -2,7 +2,7 @@ package graphics | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "fmt" | |
| 5 | - | "math/big" | |
| 5 | + | "math/bits" | |
| 6 | 6 | "sort" | |
| 7 | 7 | "sync" | |
| 8 | 8 | ) | |
| @@ -10,6 +10,7 @@ import ( | |||
| 10 | 10 | type assetRecord struct { | |
| 11 | 11 | id AssetID | |
| 12 | 12 | encoded []byte | |
| 13 | + | format AssetFormat | |
| 13 | 14 | width int64 | |
| 14 | 15 | height int64 | |
| 15 | 16 | pixels uint64 | |
| @@ -390,12 +391,6 @@ func (s *Scene) Usage() Usage { | |||
| 390 | 391 | ||
| 391 | 392 | func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState) (assetRecord, error) { | |
| 392 | 393 | data := blob.Encoded | |
| 393 | - | if data == nil { | |
| 394 | - | data = blob.Data | |
| 395 | - | } | |
| 396 | - | if len(data) == 0 && blob.Encoded != nil && blob.Data != nil && len(blob.Data) != 0 { | |
| 397 | - | data = blob.Data | |
| 398 | - | } | |
| 399 | 394 | if blob.Width <= 0 || blob.Height <= 0 { | |
| 400 | 395 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrInvalidAsset) | |
| 401 | 396 | } | |
| @@ -419,14 +414,11 @@ func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState) (assetRecord, er | |||
| 419 | 414 | if !within(state.usage.DecodedPixels, pixels, s.limits.MaxDecodedPixels) { | |
| 420 | 415 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrDecodedPixelBudget) | |
| 421 | 416 | } | |
| 422 | - | return assetRecord{encoded: append([]byte(nil), data...), width: blob.Width, height: blob.Height, pixels: pixels}, nil | |
| 417 | + | return assetRecord{encoded: append([]byte(nil), data...), format: blob.Format, width: blob.Width, height: blob.Height, pixels: pixels}, nil | |
| 423 | 418 | } | |
| 424 | 419 | ||
| 425 | 420 | func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing *placementRecord) (placementRecord, error) { | |
| 426 | 421 | asset := spec.Asset | |
| 427 | - | if !asset.Valid() { | |
| 428 | - | asset = spec.AssetID | |
| 429 | - | } | |
| 430 | 422 | if !asset.Valid() && existing != nil { | |
| 431 | 423 | asset = existing.asset | |
| 432 | 424 | } | |
| @@ -435,9 +427,6 @@ func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing | |||
| 435 | 427 | return placementRecord{}, fmt.Errorf("place: %w", ErrAssetNotFound) | |
| 436 | 428 | } | |
| 437 | 429 | destination := spec.Destination | |
| 438 | - | if destination == (PixelRect{}) { | |
| 439 | - | destination = spec.Dest | |
| 440 | - | } | |
| 441 | 430 | if existing != nil && destination == (PixelRect{}) { | |
| 442 | 431 | destination = existing.destination | |
| 443 | 432 | } | |
| @@ -460,9 +449,6 @@ func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing | |||
| 460 | 449 | return placementRecord{}, fmt.Errorf("place: %w", ErrInvalidPlacement) | |
| 461 | 450 | } | |
| 462 | 451 | cells := spec.Cells | |
| 463 | - | if cells == (CellRect{}) { | |
| 464 | - | cells = spec.CellBounds | |
| 465 | - | } | |
| 466 | 452 | if existing != nil && !spec.HasCells && cells == (CellRect{}) { | |
| 467 | 453 | cells = existing.cells | |
| 468 | 454 | } | |
| @@ -523,11 +509,7 @@ func (s *Scene) applyOperation(state *sceneState, operation Operation) error { | |||
| 523 | 509 | state.usage.EncodedBytes += uint64(len(record.encoded)) | |
| 524 | 510 | state.usage.DecodedPixels += record.pixels | |
| 525 | 511 | case OperationRemoveAsset: | |
| 526 | - | id := operation.Asset | |
| 527 | - | if !id.Valid() { | |
| 528 | - | id = operation.AssetID | |
| 529 | - | } | |
| 530 | - | if err := removeAsset(state, id, false); err != nil { | |
| 512 | + | if err := removeAsset(state, operation.Asset, false); err != nil { | |
| 531 | 513 | return err | |
| 532 | 514 | } | |
| 533 | 515 | case OperationPlace: | |
| @@ -661,6 +643,7 @@ func decodedPixels(width, height int64, declared uint64) (uint64, bool) { | |||
| 661 | 643 | type AssetView struct { | |
| 662 | 644 | id AssetID | |
| 663 | 645 | encoded []byte | |
| 646 | + | format AssetFormat | |
| 664 | 647 | width int64 | |
| 665 | 648 | height int64 | |
| 666 | 649 | pixels uint64 | |
| @@ -670,6 +653,7 @@ type AssetView struct { | |||
| 670 | 653 | type Asset = AssetView | |
| 671 | 654 | ||
| 672 | 655 | func (a AssetView) ID() AssetID { return a.id } | |
| 656 | + | func (a AssetView) Format() AssetFormat { return a.format } | |
| 673 | 657 | func (a AssetView) Width() int64 { return a.width } | |
| 674 | 658 | func (a AssetView) Height() int64 { return a.height } | |
| 675 | 659 | func (a AssetView) DecodedPixels() uint64 { return a.pixels } | |
| @@ -677,7 +661,7 @@ func (a AssetView) EncodedSize() uint64 { return uint64(len(a.encoded)) } | |||
| 677 | 661 | func (a AssetView) Encoded() []byte { return append([]byte(nil), a.encoded...) } | |
| 678 | 662 | func (a AssetView) Bytes() []byte { return a.Encoded() } | |
| 679 | 663 | func (a AssetView) Blob() AssetBlob { | |
| 680 | - | return AssetBlob{Encoded: a.Encoded(), Width: a.width, Height: a.height, DecodedPixels: a.pixels} | |
| 664 | + | return AssetBlob{Encoded: a.Encoded(), Format: a.format, Width: a.width, Height: a.height, DecodedPixels: a.pixels} | |
| 681 | 665 | } | |
| 682 | 666 | ||
| 683 | 667 | // PlacementView is an immutable view of a sparse placement. | |
| @@ -994,16 +978,24 @@ func ratio(offset, numerator, denominator int64, ceil bool) (int64, bool) { | |||
| 994 | 978 | if offset < 0 || numerator <= 0 || denominator <= 0 { | |
| 995 | 979 | return 0, false | |
| 996 | 980 | } | |
| 997 | - | product := new(big.Int).Mul(big.NewInt(offset), big.NewInt(numerator)) | |
| 998 | - | quotient, remainder := new(big.Int), new(big.Int) | |
| 999 | - | quotient.QuoRem(product, big.NewInt(denominator), remainder) | |
| 1000 | - | if ceil && remainder.Sign() != 0 { | |
| 1001 | - | quotient.Add(quotient, big.NewInt(1)) | |
| 981 | + | hi, lo := bits.Mul64(uint64(offset), uint64(numerator)) | |
| 982 | + | divisor := uint64(denominator) | |
| 983 | + | // bits.Div64 requires hi < divisor. Otherwise the quotient cannot fit in | |
| 984 | + | // uint64 and consequently cannot fit in the int64 result type. | |
| 985 | + | if hi >= divisor { | |
| 986 | + | return 0, false | |
| 987 | + | } | |
| 988 | + | quotient, remainder := bits.Div64(hi, lo, divisor) | |
| 989 | + | if ceil && remainder != 0 { | |
| 990 | + | if quotient == ^uint64(0) { | |
| 991 | + | return 0, false | |
| 992 | + | } | |
| 993 | + | quotient++ | |
| 1002 | 994 | } | |
| 1003 | - | if !quotient.IsInt64() || quotient.Sign() < 0 { | |
| 995 | + | if quotient > uint64(maxInt64) { | |
| 1004 | 996 | return 0, false | |
| 1005 | 997 | } | |
| 1006 | - | return quotient.Int64(), true | |
| 998 | + | return int64(quotient), true | |
| 1007 | 999 | } | |
| 1008 | 1000 | ||
| 1009 | 1001 | func checkedAdd(a, b int64) (int64, bool) { | |
| @@ -2,7 +2,7 @@ package graphics | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "fmt" | 4 | "fmt" | ||
| 5 | - | "math/big" | 5 | + | "math/bits" |
| 6 | "sort" | 6 | "sort" | ||
| 7 | "sync" | 7 | "sync" | ||
| 8 | ) | 8 | ) | ||
| @@ -10,6 +10,7 @@ import ( | |||||
| 10 | type assetRecord struct { | 10 | type assetRecord struct { | ||
| 11 | id AssetID | 11 | id AssetID | ||
| 12 | encoded []byte | 12 | encoded []byte | ||
| 13 | + | format AssetFormat | |||
| 13 | width int64 | 14 | width int64 | ||
| 14 | height int64 | 15 | height int64 | ||
| 15 | pixels uint64 | 16 | pixels uint64 | ||
| @@ -390,12 +391,6 @@ func (s *Scene) Usage() Usage { | |||||
| 390 | 391 | ||||
| 391 | func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState) (assetRecord, error) { | 392 | func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState) (assetRecord, error) { | ||
| 392 | data := blob.Encoded | 393 | data := blob.Encoded | ||
| 393 | - | if data == nil { | |||
| 394 | - | data = blob.Data | |||
| 395 | - | } | |||
| 396 | - | if len(data) == 0 && blob.Encoded != nil && blob.Data != nil && len(blob.Data) != 0 { | |||
| 397 | - | data = blob.Data | |||
| 398 | - | } | |||
| 399 | if blob.Width <= 0 || blob.Height <= 0 { | 394 | if blob.Width <= 0 || blob.Height <= 0 { | ||
| 400 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrInvalidAsset) | 395 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrInvalidAsset) | ||
| 401 | } | 396 | } | ||
| @@ -419,14 +414,11 @@ func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState) (assetRecord, er | |||||
| 419 | if !within(state.usage.DecodedPixels, pixels, s.limits.MaxDecodedPixels) { | 414 | if !within(state.usage.DecodedPixels, pixels, s.limits.MaxDecodedPixels) { | ||
| 420 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrDecodedPixelBudget) | 415 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrDecodedPixelBudget) | ||
| 421 | } | 416 | } | ||
| 422 | - | return assetRecord{encoded: append([]byte(nil), data...), width: blob.Width, height: blob.Height, pixels: pixels}, nil | 417 | + | return assetRecord{encoded: append([]byte(nil), data...), format: blob.Format, width: blob.Width, height: blob.Height, pixels: pixels}, nil |
| 423 | } | 418 | } | ||
| 424 | 419 | ||||
| 425 | func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing *placementRecord) (placementRecord, error) { | 420 | func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing *placementRecord) (placementRecord, error) { | ||
| 426 | asset := spec.Asset | 421 | asset := spec.Asset | ||
| 427 | - | if !asset.Valid() { | |||
| 428 | - | asset = spec.AssetID | |||
| 429 | - | } | |||
| 430 | if !asset.Valid() && existing != nil { | 422 | if !asset.Valid() && existing != nil { | ||
| 431 | asset = existing.asset | 423 | asset = existing.asset | ||
| 432 | } | 424 | } | ||
| @@ -435,9 +427,6 @@ func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing | |||||
| 435 | return placementRecord{}, fmt.Errorf("place: %w", ErrAssetNotFound) | 427 | return placementRecord{}, fmt.Errorf("place: %w", ErrAssetNotFound) | ||
| 436 | } | 428 | } | ||
| 437 | destination := spec.Destination | 429 | destination := spec.Destination | ||
| 438 | - | if destination == (PixelRect{}) { | |||
| 439 | - | destination = spec.Dest | |||
| 440 | - | } | |||
| 441 | if existing != nil && destination == (PixelRect{}) { | 430 | if existing != nil && destination == (PixelRect{}) { | ||
| 442 | destination = existing.destination | 431 | destination = existing.destination | ||
| 443 | } | 432 | } | ||
| @@ -460,9 +449,6 @@ func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing | |||||
| 460 | return placementRecord{}, fmt.Errorf("place: %w", ErrInvalidPlacement) | 449 | return placementRecord{}, fmt.Errorf("place: %w", ErrInvalidPlacement) | ||
| 461 | } | 450 | } | ||
| 462 | cells := spec.Cells | 451 | cells := spec.Cells | ||
| 463 | - | if cells == (CellRect{}) { | |||
| 464 | - | cells = spec.CellBounds | |||
| 465 | - | } | |||
| 466 | if existing != nil && !spec.HasCells && cells == (CellRect{}) { | 452 | if existing != nil && !spec.HasCells && cells == (CellRect{}) { | ||
| 467 | cells = existing.cells | 453 | cells = existing.cells | ||
| 468 | } | 454 | } | ||
| @@ -523,11 +509,7 @@ func (s *Scene) applyOperation(state *sceneState, operation Operation) error { | |||||
| 523 | state.usage.EncodedBytes += uint64(len(record.encoded)) | 509 | state.usage.EncodedBytes += uint64(len(record.encoded)) | ||
| 524 | state.usage.DecodedPixels += record.pixels | 510 | state.usage.DecodedPixels += record.pixels | ||
| 525 | case OperationRemoveAsset: | 511 | case OperationRemoveAsset: | ||
| 526 | - | id := operation.Asset | 512 | + | if err := removeAsset(state, operation.Asset, false); err != nil { |
| 527 | - | if !id.Valid() { | |||
| 528 | - | id = operation.AssetID | |||
| 529 | - | } | |||
| 530 | - | if err := removeAsset(state, id, false); err != nil { | |||
| 531 | return err | 513 | return err | ||
| 532 | } | 514 | } | ||
| 533 | case OperationPlace: | 515 | case OperationPlace: | ||
| @@ -661,6 +643,7 @@ func decodedPixels(width, height int64, declared uint64) (uint64, bool) { | |||||
| 661 | type AssetView struct { | 643 | type AssetView struct { | ||
| 662 | id AssetID | 644 | id AssetID | ||
| 663 | encoded []byte | 645 | encoded []byte | ||
| 646 | + | format AssetFormat | |||
| 664 | width int64 | 647 | width int64 | ||
| 665 | height int64 | 648 | height int64 | ||
| 666 | pixels uint64 | 649 | pixels uint64 | ||
| @@ -670,6 +653,7 @@ type AssetView struct { | |||||
| 670 | type Asset = AssetView | 653 | type Asset = AssetView | ||
| 671 | 654 | ||||
| 672 | func (a AssetView) ID() AssetID { return a.id } | 655 | func (a AssetView) ID() AssetID { return a.id } | ||
| 656 | + | func (a AssetView) Format() AssetFormat { return a.format } | |||
| 673 | func (a AssetView) Width() int64 { return a.width } | 657 | func (a AssetView) Width() int64 { return a.width } | ||
| 674 | func (a AssetView) Height() int64 { return a.height } | 658 | func (a AssetView) Height() int64 { return a.height } | ||
| 675 | func (a AssetView) DecodedPixels() uint64 { return a.pixels } | 659 | func (a AssetView) DecodedPixels() uint64 { return a.pixels } | ||
| @@ -677,7 +661,7 @@ func (a AssetView) EncodedSize() uint64 { return uint64(len(a.encoded)) } | |||||
| 677 | func (a AssetView) Encoded() []byte { return append([]byte(nil), a.encoded...) } | 661 | func (a AssetView) Encoded() []byte { return append([]byte(nil), a.encoded...) } | ||
| 678 | func (a AssetView) Bytes() []byte { return a.Encoded() } | 662 | func (a AssetView) Bytes() []byte { return a.Encoded() } | ||
| 679 | func (a AssetView) Blob() AssetBlob { | 663 | func (a AssetView) Blob() AssetBlob { | ||
| 680 | - | return AssetBlob{Encoded: a.Encoded(), Width: a.width, Height: a.height, DecodedPixels: a.pixels} | 664 | + | return AssetBlob{Encoded: a.Encoded(), Format: a.format, Width: a.width, Height: a.height, DecodedPixels: a.pixels} |
| 681 | } | 665 | } | ||
| 682 | 666 | ||||
| 683 | // PlacementView is an immutable view of a sparse placement. | 667 | // PlacementView is an immutable view of a sparse placement. | ||
| @@ -994,16 +978,24 @@ func ratio(offset, numerator, denominator int64, ceil bool) (int64, bool) { | |||||
| 994 | if offset < 0 || numerator <= 0 || denominator <= 0 { | 978 | if offset < 0 || numerator <= 0 || denominator <= 0 { | ||
| 995 | return 0, false | 979 | return 0, false | ||
| 996 | } | 980 | } | ||
| 997 | - | product := new(big.Int).Mul(big.NewInt(offset), big.NewInt(numerator)) | 981 | + | hi, lo := bits.Mul64(uint64(offset), uint64(numerator)) |
| 998 | - | quotient, remainder := new(big.Int), new(big.Int) | 982 | + | divisor := uint64(denominator) |
| 999 | - | quotient.QuoRem(product, big.NewInt(denominator), remainder) | 983 | + | // bits.Div64 requires hi < divisor. Otherwise the quotient cannot fit in |
| 1000 | - | if ceil && remainder.Sign() != 0 { | 984 | + | // uint64 and consequently cannot fit in the int64 result type. |
| 1001 | - | quotient.Add(quotient, big.NewInt(1)) | 985 | + | if hi >= divisor { |
| 986 | + | return 0, false | |||
| 987 | + | } | |||
| 988 | + | quotient, remainder := bits.Div64(hi, lo, divisor) | |||
| 989 | + | if ceil && remainder != 0 { | |||
| 990 | + | if quotient == ^uint64(0) { | |||
| 991 | + | return 0, false | |||
| 992 | + | } | |||
| 993 | + | quotient++ | |||
| 1002 | } | 994 | } | ||
| 1003 | - | if !quotient.IsInt64() || quotient.Sign() < 0 { | 995 | + | if quotient > uint64(maxInt64) { |
| 1004 | return 0, false | 996 | return 0, false | ||
| 1005 | } | 997 | } | ||
| 1006 | - | return quotient.Int64(), true | 998 | + | return int64(quotient), true |
| 1007 | } | 999 | } | ||
| 1008 | 1000 | ||||
| 1009 | func checkedAdd(a, b int64) (int64, bool) { | 1001 | func checkedAdd(a, b int64) (int64, bool) { | ||
M graphics/scene_test.go +3 -2
| @@ -103,14 +103,15 @@ func TestAssetAndPlacementQuotasAreTransactional(t *testing.T) { | |||
| 103 | 103 | if scene.Snapshot().Generation() != generation { | |
| 104 | 104 | t.Fatal("failed asset addition changed generation") | |
| 105 | 105 | } | |
| 106 | - | if _, err := scene.Place(PlacementSpec{Asset: id, Destination: PixelRect{Width: 2, Height: 2}}); err != nil { | |
| 106 | + | firstPlacement, err := scene.Place(PlacementSpec{Asset: id, Destination: PixelRect{Width: 2, Height: 2}}) | |
| 107 | + | if err != nil { | |
| 107 | 108 | t.Fatal(err) | |
| 108 | 109 | } | |
| 109 | 110 | if _, err := scene.Place(PlacementSpec{Asset: id, Destination: PixelRect{X: 2, Width: 2, Height: 2}}); !errors.Is(err, ErrTooManyPlacements) { | |
| 110 | 111 | t.Fatalf("placement quota error = %v", err) | |
| 111 | 112 | } | |
| 112 | 113 | if err := scene.Apply( | |
| 113 | - | Operation{Kind: OperationRemovePlacement, PlacementID: PlacementID{value: 1}}, | |
| 114 | + | Operation{Kind: OperationRemovePlacement, PlacementID: firstPlacement}, | |
| 114 | 115 | Operation{Kind: OperationPlace, Placement: PlacementSpec{Asset: id, Destination: PixelRect{Width: 2, Height: 2}}}, | |
| 115 | 116 | ); err != nil { | |
| 116 | 117 | t.Fatal(err) | |
| @@ -103,14 +103,15 @@ func TestAssetAndPlacementQuotasAreTransactional(t *testing.T) { | |||||
| 103 | if scene.Snapshot().Generation() != generation { | 103 | if scene.Snapshot().Generation() != generation { | ||
| 104 | t.Fatal("failed asset addition changed generation") | 104 | t.Fatal("failed asset addition changed generation") | ||
| 105 | } | 105 | } | ||
| 106 | - | if _, err := scene.Place(PlacementSpec{Asset: id, Destination: PixelRect{Width: 2, Height: 2}}); err != nil { | 106 | + | firstPlacement, err := scene.Place(PlacementSpec{Asset: id, Destination: PixelRect{Width: 2, Height: 2}}) |
| 107 | + | if err != nil { | |||
| 107 | t.Fatal(err) | 108 | t.Fatal(err) | ||
| 108 | } | 109 | } | ||
| 109 | if _, err := scene.Place(PlacementSpec{Asset: id, Destination: PixelRect{X: 2, Width: 2, Height: 2}}); !errors.Is(err, ErrTooManyPlacements) { | 110 | if _, err := scene.Place(PlacementSpec{Asset: id, Destination: PixelRect{X: 2, Width: 2, Height: 2}}); !errors.Is(err, ErrTooManyPlacements) { | ||
| 110 | t.Fatalf("placement quota error = %v", err) | 111 | t.Fatalf("placement quota error = %v", err) | ||
| 111 | } | 112 | } | ||
| 112 | if err := scene.Apply( | 113 | if err := scene.Apply( | ||
| 113 | - | Operation{Kind: OperationRemovePlacement, PlacementID: PlacementID{value: 1}}, | 114 | + | Operation{Kind: OperationRemovePlacement, PlacementID: firstPlacement}, |
| 114 | Operation{Kind: OperationPlace, Placement: PlacementSpec{Asset: id, Destination: PixelRect{Width: 2, Height: 2}}}, | 115 | Operation{Kind: OperationPlace, Placement: PlacementSpec{Asset: id, Destination: PixelRect{Width: 2, Height: 2}}}, | ||
| 115 | ); err != nil { | 116 | ); err != nil { | ||
| 116 | t.Fatal(err) | 117 | t.Fatal(err) | ||
M graphics_state.go +4 -10
| @@ -30,7 +30,7 @@ func (g *screenGraphicsState) snapshot() *graphics.Snapshot { | |||
| 30 | 30 | ||
| 31 | 31 | // clearPlacements applies the ordinary full-screen erase policy without | |
| 32 | 32 | // discarding uploaded image mappings. The protocol session owns those | |
| 33 | - | // mappings, so the operation goes through its delete-all-placements command | |
| 33 | + | // mappings, so the operation goes through Kitty's delete-all-visible command | |
| 34 | 34 | // rather than mutating the scene directly. | |
| 35 | 35 | func (g *screenGraphicsState) clearPlacements() { | |
| 36 | 36 | if g == nil || g.kitty == nil { | |
| @@ -40,7 +40,7 @@ func (g *screenGraphicsState) clearPlacements() { | |||
| 40 | 40 | _, _ = g.kitty.Process(kittygraphics.Command{Controls: kittygraphics.Controls{ | |
| 41 | 41 | Action: kittygraphics.ActionDelete, | |
| 42 | 42 | HasAction: true, | |
| 43 | - | Delete: kittygraphics.DeleteAllPlacements, | |
| 43 | + | Delete: kittygraphics.DeleteAll, | |
| 44 | 44 | HasDelete: true, | |
| 45 | 45 | Quiet: kittygraphics.QuietAll, | |
| 46 | 46 | HasQuiet: true, | |
| @@ -81,17 +81,11 @@ func (s *Screen) abortAllKittyPending() { | |||
| 81 | 81 | } | |
| 82 | 82 | ||
| 83 | 83 | // GraphicsSnapshot returns an immutable snapshot of the active screen | |
| 84 | - | // buffer's graphics scene, or nil before graphics has been used. | |
| 84 | + | // buffer's graphics scene, or nil before graphics has been used. The returned | |
| 85 | + | // reference remains valid after subsequent Screen mutations. | |
| 85 | 86 | func (s *Screen) GraphicsSnapshot() *graphics.Snapshot { | |
| 86 | 87 | if s == nil || s.graphics == nil { | |
| 87 | 88 | return nil | |
| 88 | 89 | } | |
| 89 | 90 | return s.graphics.snapshot() | |
| 90 | 91 | } | |
| 91 | - | ||
| 92 | - | // CaptureGraphicsSnapshot names the ownership boundary used by renderers that | |
| 93 | - | // retain graphics independently of the text frame. The returned reference is | |
| 94 | - | // immutable and remains valid after subsequent Screen mutations. | |
| 95 | - | func (s *Screen) CaptureGraphicsSnapshot() *graphics.Snapshot { | |
| 96 | - | return s.GraphicsSnapshot() | |
| 97 | - | } | |
| @@ -30,7 +30,7 @@ func (g *screenGraphicsState) snapshot() *graphics.Snapshot { | |||||
| 30 | 30 | ||||
| 31 | // clearPlacements applies the ordinary full-screen erase policy without | 31 | // clearPlacements applies the ordinary full-screen erase policy without | ||
| 32 | // discarding uploaded image mappings. The protocol session owns those | 32 | // discarding uploaded image mappings. The protocol session owns those | ||
| 33 | - | // mappings, so the operation goes through its delete-all-placements command | 33 | + | // mappings, so the operation goes through Kitty's delete-all-visible command |
| 34 | // rather than mutating the scene directly. | 34 | // rather than mutating the scene directly. | ||
| 35 | func (g *screenGraphicsState) clearPlacements() { | 35 | func (g *screenGraphicsState) clearPlacements() { | ||
| 36 | if g == nil || g.kitty == nil { | 36 | if g == nil || g.kitty == nil { | ||
| @@ -40,7 +40,7 @@ func (g *screenGraphicsState) clearPlacements() { | |||||
| 40 | _, _ = g.kitty.Process(kittygraphics.Command{Controls: kittygraphics.Controls{ | 40 | _, _ = g.kitty.Process(kittygraphics.Command{Controls: kittygraphics.Controls{ | ||
| 41 | Action: kittygraphics.ActionDelete, | 41 | Action: kittygraphics.ActionDelete, | ||
| 42 | HasAction: true, | 42 | HasAction: true, | ||
| 43 | - | Delete: kittygraphics.DeleteAllPlacements, | 43 | + | Delete: kittygraphics.DeleteAll, |
| 44 | HasDelete: true, | 44 | HasDelete: true, | ||
| 45 | Quiet: kittygraphics.QuietAll, | 45 | Quiet: kittygraphics.QuietAll, | ||
| 46 | HasQuiet: true, | 46 | HasQuiet: true, | ||
| @@ -81,17 +81,11 @@ func (s *Screen) abortAllKittyPending() { | |||||
| 81 | } | 81 | } | ||
| 82 | 82 | ||||
| 83 | // GraphicsSnapshot returns an immutable snapshot of the active screen | 83 | // GraphicsSnapshot returns an immutable snapshot of the active screen | ||
| 84 | - | // buffer's graphics scene, or nil before graphics has been used. | 84 | + | // buffer's graphics scene, or nil before graphics has been used. The returned |
| 85 | + | // reference remains valid after subsequent Screen mutations. | |||
| 85 | func (s *Screen) GraphicsSnapshot() *graphics.Snapshot { | 86 | func (s *Screen) GraphicsSnapshot() *graphics.Snapshot { | ||
| 86 | if s == nil || s.graphics == nil { | 87 | if s == nil || s.graphics == nil { | ||
| 87 | return nil | 88 | return nil | ||
| 88 | } | 89 | } | ||
| 89 | return s.graphics.snapshot() | 90 | return s.graphics.snapshot() | ||
| 90 | } | 91 | } | ||
| 91 | - | ||||
| 92 | - | // CaptureGraphicsSnapshot names the ownership boundary used by renderers that | |||
| 93 | - | // retain graphics independently of the text frame. The returned reference is | |||
| 94 | - | // immutable and remains valid after subsequent Screen mutations. | |||
| 95 | - | func (s *Screen) CaptureGraphicsSnapshot() *graphics.Snapshot { | |||
| 96 | - | return s.GraphicsSnapshot() | |||
| 97 | - | } | |||
M protocol/kittygraphics/controls.go +11 -2
| @@ -121,7 +121,7 @@ func parseControl(c *Controls, key byte, value []byte) error { | |||
| 121 | 121 | } | |
| 122 | 122 | d := DeleteTarget(value[0]) | |
| 123 | 123 | switch d { | |
| 124 | - | case DeleteImage, DeleteImageNumber, DeletePlacement, DeleteAll, DeleteAllImages, DeleteAllPlacements: | |
| 124 | + | case DeleteImage, DeleteImageNumber, DeleteCell, DeleteAll, DeleteAllImages, DeleteCellAll: | |
| 125 | 125 | c.Delete, c.HasDelete = d, true | |
| 126 | 126 | default: | |
| 127 | 127 | return fmt.Errorf("%w: d=%q", ErrInvalidCommand, value) | |
| @@ -144,7 +144,10 @@ func parseControl(c *Controls, key byte, value []byte) error { | |||
| 144 | 144 | case ControlParent: | |
| 145 | 145 | return setUint(&c.Parent, &c.HasParent, key, value) | |
| 146 | 146 | default: | |
| 147 | - | return fmt.Errorf("%w: %c", ErrUnknownControl, key) | |
| 147 | + | // Kitty reserves unknown controls for forward-compatible extensions. | |
| 148 | + | // Commands that depend on an unsupported control are rejected later by | |
| 149 | + | // their action-specific validation. | |
| 150 | + | return nil | |
| 148 | 151 | } | |
| 149 | 152 | return nil | |
| 150 | 153 | } | |
| @@ -168,6 +171,9 @@ func setUint(dst *uint64, present *bool, key byte, value []byte) error { | |||
| 168 | 171 | if err != nil { | |
| 169 | 172 | return fieldError(key, err) | |
| 170 | 173 | } | |
| 174 | + | if n > MaxKittyID { | |
| 175 | + | return fieldError(key, ErrIntegerOverflow) | |
| 176 | + | } | |
| 171 | 177 | *dst = n | |
| 172 | 178 | if present != nil { | |
| 173 | 179 | *present = true | |
| @@ -180,6 +186,9 @@ func setInt(dst *int64, present *bool, key byte, value []byte) error { | |||
| 180 | 186 | if err != nil { | |
| 181 | 187 | return fieldError(key, err) | |
| 182 | 188 | } | |
| 189 | + | if n < -1<<31 || n > 1<<31-1 { | |
| 190 | + | return fieldError(key, ErrIntegerOverflow) | |
| 191 | + | } | |
| 183 | 192 | *dst = n | |
| 184 | 193 | if present != nil { | |
| 185 | 194 | *present = true | |
| @@ -121,7 +121,7 @@ func parseControl(c *Controls, key byte, value []byte) error { | |||||
| 121 | } | 121 | } | ||
| 122 | d := DeleteTarget(value[0]) | 122 | d := DeleteTarget(value[0]) | ||
| 123 | switch d { | 123 | switch d { | ||
| 124 | - | case DeleteImage, DeleteImageNumber, DeletePlacement, DeleteAll, DeleteAllImages, DeleteAllPlacements: | 124 | + | case DeleteImage, DeleteImageNumber, DeleteCell, DeleteAll, DeleteAllImages, DeleteCellAll: |
| 125 | c.Delete, c.HasDelete = d, true | 125 | c.Delete, c.HasDelete = d, true | ||
| 126 | default: | 126 | default: | ||
| 127 | return fmt.Errorf("%w: d=%q", ErrInvalidCommand, value) | 127 | return fmt.Errorf("%w: d=%q", ErrInvalidCommand, value) | ||
| @@ -144,7 +144,10 @@ func parseControl(c *Controls, key byte, value []byte) error { | |||||
| 144 | case ControlParent: | 144 | case ControlParent: | ||
| 145 | return setUint(&c.Parent, &c.HasParent, key, value) | 145 | return setUint(&c.Parent, &c.HasParent, key, value) | ||
| 146 | default: | 146 | default: | ||
| 147 | - | return fmt.Errorf("%w: %c", ErrUnknownControl, key) | 147 | + | // Kitty reserves unknown controls for forward-compatible extensions. |
| 148 | + | // Commands that depend on an unsupported control are rejected later by | |||
| 149 | + | // their action-specific validation. | |||
| 150 | + | return nil | |||
| 148 | } | 151 | } | ||
| 149 | return nil | 152 | return nil | ||
| 150 | } | 153 | } | ||
| @@ -168,6 +171,9 @@ func setUint(dst *uint64, present *bool, key byte, value []byte) error { | |||||
| 168 | if err != nil { | 171 | if err != nil { | ||
| 169 | return fieldError(key, err) | 172 | return fieldError(key, err) | ||
| 170 | } | 173 | } | ||
| 174 | + | if n > MaxKittyID { | |||
| 175 | + | return fieldError(key, ErrIntegerOverflow) | |||
| 176 | + | } | |||
| 171 | *dst = n | 177 | *dst = n | ||
| 172 | if present != nil { | 178 | if present != nil { | ||
| 173 | *present = true | 179 | *present = true | ||
| @@ -180,6 +186,9 @@ func setInt(dst *int64, present *bool, key byte, value []byte) error { | |||||
| 180 | if err != nil { | 186 | if err != nil { | ||
| 181 | return fieldError(key, err) | 187 | return fieldError(key, err) | ||
| 182 | } | 188 | } | ||
| 189 | + | if n < -1<<31 || n > 1<<31-1 { | |||
| 190 | + | return fieldError(key, ErrIntegerOverflow) | |||
| 191 | + | } | |||
| 183 | *dst = n | 192 | *dst = n | ||
| 184 | if present != nil { | 193 | if present != nil { | ||
| 185 | *present = true | 194 | *present = true | ||
M protocol/kittygraphics/kittygraphics.go +21 -15
| @@ -3,6 +3,7 @@ package kittygraphics | |||
| 3 | 3 | import ( | |
| 4 | 4 | "errors" | |
| 5 | 5 | "math" | |
| 6 | + | "sort" | |
| 6 | 7 | "sync" | |
| 7 | 8 | ||
| 8 | 9 | "github.com/bnema/vev-vt/graphics" | |
| @@ -39,6 +40,7 @@ const ( | |||
| 39 | 40 | DefaultMaxUploadBytes = uint64(16 << 20) | |
| 40 | 41 | DefaultMaxChunks = uint64(1024) | |
| 41 | 42 | DefaultMaxPixels = uint64(128 << 20) | |
| 43 | + | DefaultMaxDecodedBytes = uint64(256 << 20) | |
| 42 | 44 | DefaultMaxDimension = uint64(1 << 20) | |
| 43 | 45 | ) | |
| 44 | 46 | ||
| @@ -51,6 +53,7 @@ type Limits struct { | |||
| 51 | 53 | MaxUploadBytes uint64 | |
| 52 | 54 | MaxChunks uint64 | |
| 53 | 55 | MaxDecodedPixels uint64 | |
| 56 | + | MaxDecodedBytes uint64 | |
| 54 | 57 | MaxDimension uint64 | |
| 55 | 58 | MaxResponseBytes uint64 | |
| 56 | 59 | MaxImages uint64 | |
| @@ -76,6 +79,9 @@ func normalizeLimits(l Limits) Limits { | |||
| 76 | 79 | if l.MaxDecodedPixels == 0 { | |
| 77 | 80 | l.MaxDecodedPixels = DefaultMaxPixels | |
| 78 | 81 | } | |
| 82 | + | if l.MaxDecodedBytes == 0 { | |
| 83 | + | l.MaxDecodedBytes = DefaultMaxDecodedBytes | |
| 84 | + | } | |
| 79 | 85 | if l.MaxDimension == 0 { | |
| 80 | 86 | l.MaxDimension = DefaultMaxDimension | |
| 81 | 87 | } | |
| @@ -157,12 +163,12 @@ const ( | |||
| 157 | 163 | type DeleteTarget byte | |
| 158 | 164 | ||
| 159 | 165 | const ( | |
| 160 | - | DeleteImage DeleteTarget = 'i' | |
| 161 | - | DeleteImageNumber DeleteTarget = 'I' | |
| 162 | - | DeletePlacement DeleteTarget = 'p' | |
| 163 | - | DeleteAll DeleteTarget = 'a' | |
| 164 | - | DeleteAllImages DeleteTarget = 'A' | |
| 165 | - | DeleteAllPlacements DeleteTarget = 'P' | |
| 166 | + | DeleteImage DeleteTarget = 'i' | |
| 167 | + | DeleteImageNumber DeleteTarget = 'I' | |
| 168 | + | DeleteCell DeleteTarget = 'p' | |
| 169 | + | DeleteAll DeleteTarget = 'a' | |
| 170 | + | DeleteAllImages DeleteTarget = 'A' | |
| 171 | + | DeleteCellAll DeleteTarget = 'P' | |
| 166 | 172 | ) | |
| 167 | 173 | ||
| 168 | 174 | // Transmission is the direct transmission mode. File, temporary-file and | |
| @@ -293,7 +299,6 @@ type Parser struct { | |||
| 293 | 299 | candidate []byte | |
| 294 | 300 | apc []byte | |
| 295 | 301 | inAPC bool | |
| 296 | - | discard bool | |
| 297 | 302 | escaped bool | |
| 298 | 303 | oversize bool | |
| 299 | 304 | } | |
| @@ -412,19 +417,21 @@ func (s *Session) PlacementID(imageID, placementID uint64) (graphics.PlacementID | |||
| 412 | 417 | return s.Placement(imageID, placementID) | |
| 413 | 418 | } | |
| 414 | 419 | ||
| 415 | - | // Child returns the session child associated with an image ID. | |
| 420 | + | // Child returns the oldest session child associated with an image ID. | |
| 416 | 421 | func (s *Session) Child(imageID uint64) (Child, bool) { | |
| 417 | 422 | if s == nil { | |
| 418 | 423 | return Child{}, false | |
| 419 | 424 | } | |
| 420 | 425 | s.mu.Lock() | |
| 421 | 426 | defer s.mu.Unlock() | |
| 427 | + | var selected Child | |
| 428 | + | found := false | |
| 422 | 429 | for _, child := range s.children { | |
| 423 | - | if child.ImageID == imageID { | |
| 424 | - | return child, true | |
| 430 | + | if child.ImageID == imageID && (!found || child.ID < selected.ID) { | |
| 431 | + | selected, found = child, true | |
| 425 | 432 | } | |
| 426 | 433 | } | |
| 427 | - | return Child{}, false | |
| 434 | + | return selected, found | |
| 428 | 435 | } | |
| 429 | 436 | ||
| 430 | 437 | // ChildByID returns a session child by its adapter-local ID. | |
| @@ -456,10 +463,9 @@ func (s *Session) Children() []Child { | |||
| 456 | 463 | s.mu.Lock() | |
| 457 | 464 | defer s.mu.Unlock() | |
| 458 | 465 | children := make([]Child, 0, len(s.children)) | |
| 459 | - | for id := uint64(1); id < s.nextChild; id++ { | |
| 460 | - | if child, ok := s.children[id]; ok { | |
| 461 | - | children = append(children, child) | |
| 462 | - | } | |
| 466 | + | for _, child := range s.children { | |
| 467 | + | children = append(children, child) | |
| 463 | 468 | } | |
| 469 | + | sort.Slice(children, func(i, j int) bool { return children[i].ID < children[j].ID }) | |
| 464 | 470 | return children | |
| 465 | 471 | } | |
| @@ -3,6 +3,7 @@ package kittygraphics | |||||
| 3 | import ( | 3 | import ( | ||
| 4 | "errors" | 4 | "errors" | ||
| 5 | "math" | 5 | "math" | ||
| 6 | + | "sort" | |||
| 6 | "sync" | 7 | "sync" | ||
| 7 | 8 | ||||
| 8 | "github.com/bnema/vev-vt/graphics" | 9 | "github.com/bnema/vev-vt/graphics" | ||
| @@ -39,6 +40,7 @@ const ( | |||||
| 39 | DefaultMaxUploadBytes = uint64(16 << 20) | 40 | DefaultMaxUploadBytes = uint64(16 << 20) | ||
| 40 | DefaultMaxChunks = uint64(1024) | 41 | DefaultMaxChunks = uint64(1024) | ||
| 41 | DefaultMaxPixels = uint64(128 << 20) | 42 | DefaultMaxPixels = uint64(128 << 20) | ||
| 43 | + | DefaultMaxDecodedBytes = uint64(256 << 20) | |||
| 42 | DefaultMaxDimension = uint64(1 << 20) | 44 | DefaultMaxDimension = uint64(1 << 20) | ||
| 43 | ) | 45 | ) | ||
| 44 | 46 | ||||
| @@ -51,6 +53,7 @@ type Limits struct { | |||||
| 51 | MaxUploadBytes uint64 | 53 | MaxUploadBytes uint64 | ||
| 52 | MaxChunks uint64 | 54 | MaxChunks uint64 | ||
| 53 | MaxDecodedPixels uint64 | 55 | MaxDecodedPixels uint64 | ||
| 56 | + | MaxDecodedBytes uint64 | |||
| 54 | MaxDimension uint64 | 57 | MaxDimension uint64 | ||
| 55 | MaxResponseBytes uint64 | 58 | MaxResponseBytes uint64 | ||
| 56 | MaxImages uint64 | 59 | MaxImages uint64 | ||
| @@ -76,6 +79,9 @@ func normalizeLimits(l Limits) Limits { | |||||
| 76 | if l.MaxDecodedPixels == 0 { | 79 | if l.MaxDecodedPixels == 0 { | ||
| 77 | l.MaxDecodedPixels = DefaultMaxPixels | 80 | l.MaxDecodedPixels = DefaultMaxPixels | ||
| 78 | } | 81 | } | ||
| 82 | + | if l.MaxDecodedBytes == 0 { | |||
| 83 | + | l.MaxDecodedBytes = DefaultMaxDecodedBytes | |||
| 84 | + | } | |||
| 79 | if l.MaxDimension == 0 { | 85 | if l.MaxDimension == 0 { | ||
| 80 | l.MaxDimension = DefaultMaxDimension | 86 | l.MaxDimension = DefaultMaxDimension | ||
| 81 | } | 87 | } | ||
| @@ -157,12 +163,12 @@ const ( | |||||
| 157 | type DeleteTarget byte | 163 | type DeleteTarget byte | ||
| 158 | 164 | ||||
| 159 | const ( | 165 | const ( | ||
| 160 | - | DeleteImage DeleteTarget = 'i' | 166 | + | DeleteImage DeleteTarget = 'i' |
| 161 | - | DeleteImageNumber DeleteTarget = 'I' | 167 | + | DeleteImageNumber DeleteTarget = 'I' |
| 162 | - | DeletePlacement DeleteTarget = 'p' | 168 | + | DeleteCell DeleteTarget = 'p' |
| 163 | - | DeleteAll DeleteTarget = 'a' | 169 | + | DeleteAll DeleteTarget = 'a' |
| 164 | - | DeleteAllImages DeleteTarget = 'A' | 170 | + | DeleteAllImages DeleteTarget = 'A' |
| 165 | - | DeleteAllPlacements DeleteTarget = 'P' | 171 | + | DeleteCellAll DeleteTarget = 'P' |
| 166 | ) | 172 | ) | ||
| 167 | 173 | ||||
| 168 | // Transmission is the direct transmission mode. File, temporary-file and | 174 | // Transmission is the direct transmission mode. File, temporary-file and | ||
| @@ -293,7 +299,6 @@ type Parser struct { | |||||
| 293 | candidate []byte | 299 | candidate []byte | ||
| 294 | apc []byte | 300 | apc []byte | ||
| 295 | inAPC bool | 301 | inAPC bool | ||
| 296 | - | discard bool | |||
| 297 | escaped bool | 302 | escaped bool | ||
| 298 | oversize bool | 303 | oversize bool | ||
| 299 | } | 304 | } | ||
| @@ -412,19 +417,21 @@ func (s *Session) PlacementID(imageID, placementID uint64) (graphics.PlacementID | |||||
| 412 | return s.Placement(imageID, placementID) | 417 | return s.Placement(imageID, placementID) | ||
| 413 | } | 418 | } | ||
| 414 | 419 | ||||
| 415 | - | // Child returns the session child associated with an image ID. | 420 | + | // Child returns the oldest session child associated with an image ID. |
| 416 | func (s *Session) Child(imageID uint64) (Child, bool) { | 421 | func (s *Session) Child(imageID uint64) (Child, bool) { | ||
| 417 | if s == nil { | 422 | if s == nil { | ||
| 418 | return Child{}, false | 423 | return Child{}, false | ||
| 419 | } | 424 | } | ||
| 420 | s.mu.Lock() | 425 | s.mu.Lock() | ||
| 421 | defer s.mu.Unlock() | 426 | defer s.mu.Unlock() | ||
| 427 | + | var selected Child | |||
| 428 | + | found := false | |||
| 422 | for _, child := range s.children { | 429 | for _, child := range s.children { | ||
| 423 | - | if child.ImageID == imageID { | 430 | + | if child.ImageID == imageID && (!found || child.ID < selected.ID) { |
| 424 | - | return child, true | 431 | + | selected, found = child, true |
| 425 | } | 432 | } | ||
| 426 | } | 433 | } | ||
| 427 | - | return Child{}, false | 434 | + | return selected, found |
| 428 | } | 435 | } | ||
| 429 | 436 | ||||
| 430 | // ChildByID returns a session child by its adapter-local ID. | 437 | // ChildByID returns a session child by its adapter-local ID. | ||
| @@ -456,10 +463,9 @@ func (s *Session) Children() []Child { | |||||
| 456 | s.mu.Lock() | 463 | s.mu.Lock() | ||
| 457 | defer s.mu.Unlock() | 464 | defer s.mu.Unlock() | ||
| 458 | children := make([]Child, 0, len(s.children)) | 465 | children := make([]Child, 0, len(s.children)) | ||
| 459 | - | for id := uint64(1); id < s.nextChild; id++ { | 466 | + | for _, child := range s.children { |
| 460 | - | if child, ok := s.children[id]; ok { | 467 | + | children = append(children, child) |
| 461 | - | children = append(children, child) | |||
| 462 | - | } | |||
| 463 | } | 468 | } | ||
| 469 | + | sort.Slice(children, func(i, j int) bool { return children[i].ID < children[j].ID }) | |||
| 464 | return children | 470 | return children | ||
| 465 | } | 471 | } | ||
M protocol/kittygraphics/kittygraphics_test.go +59 -0
| @@ -180,6 +180,18 @@ func TestPlacementIDsAreScopedToTheirImage(t *testing.T) { | |||
| 180 | 180 | } | |
| 181 | 181 | } | |
| 182 | 182 | ||
| 183 | + | func TestParseAPCAcceptsPayloadFreeCommands(t *testing.T) { | |
| 184 | + | for _, raw := range []string{"\x1b_Ga=d,d=a\x1b\\", "\x1b_Ga=p,i=1\x1b\\"} { | |
| 185 | + | command, err := ParseAPC([]byte(raw)) | |
| 186 | + | if err != nil { | |
| 187 | + | t.Fatalf("ParseAPC(%q) error = %v", raw, err) | |
| 188 | + | } | |
| 189 | + | if len(command.Payload) != 0 { | |
| 190 | + | t.Fatalf("ParseAPC(%q) payload = %q", raw, command.Payload) | |
| 191 | + | } | |
| 192 | + | } | |
| 193 | + | } | |
| 194 | + | ||
| 183 | 195 | func TestMalformedTruncatedInterleavedAndOversizeAreBounded(t *testing.T) { | |
| 184 | 196 | if _, err := ParseCommand([]byte("a=q,a=t;")); !errors.Is(err, ErrDuplicateControl) { | |
| 185 | 197 | t.Fatalf("duplicate control error = %v", err) | |
| @@ -286,6 +298,19 @@ func TestChunkedTransmitAndDisplayRetainsDisplayAction(t *testing.T) { | |||
| 286 | 298 | } | |
| 287 | 299 | } | |
| 288 | 300 | ||
| 301 | + | func TestControlsIgnoreExtensionsAndBoundIntegerFields(t *testing.T) { | |
| 302 | + | controls, err := ParseControls([]byte("a=p,i=1,N=1,S=2,O=3")) | |
| 303 | + | if err != nil || controls.ImageID != 1 { | |
| 304 | + | t.Fatalf("extension controls = %#v, %v", controls, err) | |
| 305 | + | } | |
| 306 | + | if _, err := ParseControls([]byte("a=p,s=4294967296")); !errors.Is(err, ErrIntegerOverflow) { | |
| 307 | + | t.Fatalf("out-of-range uint32 control error = %v", err) | |
| 308 | + | } | |
| 309 | + | if _, err := ParseControls([]byte("a=p,z=2147483648")); !errors.Is(err, ErrIntegerOverflow) { | |
| 310 | + | t.Fatalf("out-of-range int32 control error = %v", err) | |
| 311 | + | } | |
| 312 | + | } | |
| 313 | + | ||
| 289 | 314 | func TestImplicitPlacementIDsStopAtKittyLimit(t *testing.T) { | |
| 290 | 315 | session := NewSession(nil) | |
| 291 | 316 | session.nextPlacement = MaxKittyID + 1 | |
| @@ -294,6 +319,40 @@ func TestImplicitPlacementIDsStopAtKittyLimit(t *testing.T) { | |||
| 294 | 319 | } | |
| 295 | 320 | } | |
| 296 | 321 | ||
| 322 | + | func TestUnsupportedRelativeAndCellDeleteControlsFailClosed(t *testing.T) { | |
| 323 | + | session := NewSession(graphics.NewScene(graphics.Limits{})) | |
| 324 | + | payload := base64.RawStdEncoding.EncodeToString([]byte{1, 2, 3, 4}) | |
| 325 | + | if _, err := session.Feed(apc("a=t,i=1,f=32,s=1,v=1,q=2", payload)); err != nil { | |
| 326 | + | t.Fatal(err) | |
| 327 | + | } | |
| 328 | + | for _, header := range []string{"a=p,i=1,P=2", "a=p,i=1,H=1", "a=d,d=p,x=1,y=1", "a=d,d=P,x=1,y=1"} { | |
| 329 | + | if _, err := session.Feed(apc(header, "")); !errors.Is(err, ErrUnsupported) { | |
| 330 | + | t.Fatalf("%q error = %v, want ErrUnsupported", header, err) | |
| 331 | + | } | |
| 332 | + | } | |
| 333 | + | } | |
| 334 | + | ||
| 335 | + | func TestPNGDecodedByteBudgetAndAssetFormat(t *testing.T) { | |
| 336 | + | data := pngBytes(t, 2, 2) | |
| 337 | + | if _, _, err := imageGeometry(data, FormatPNG, Controls{}, normalizeLimits(Limits{MaxDecodedBytes: 15})); !errors.Is(err, ErrPayloadTooLarge) { | |
| 338 | + | t.Fatalf("PNG byte-budget error = %v", err) | |
| 339 | + | } | |
| 340 | + | scene := graphics.NewScene(graphics.Limits{}) | |
| 341 | + | session := NewSession(scene) | |
| 342 | + | payload := base64.RawStdEncoding.EncodeToString(data) | |
| 343 | + | if _, err := session.Feed(apc("a=t,i=1,f=100,q=2", payload)); err != nil { | |
| 344 | + | t.Fatal(err) | |
| 345 | + | } | |
| 346 | + | assetID, ok := session.Image(1) | |
| 347 | + | if !ok { | |
| 348 | + | t.Fatal("image mapping missing") | |
| 349 | + | } | |
| 350 | + | asset, ok := scene.Snapshot().Asset(assetID) | |
| 351 | + | if !ok || asset.Format() != graphics.AssetFormatPNG || asset.Blob().Format != graphics.AssetFormatPNG { | |
| 352 | + | t.Fatalf("asset format = %#v, ok=%v", asset, ok) | |
| 353 | + | } | |
| 354 | + | } | |
| 355 | + | ||
| 297 | 356 | func TestPlacementFailureEmitsStableProtocolError(t *testing.T) { | |
| 298 | 357 | scene := graphics.NewScene(graphics.Limits{}) | |
| 299 | 358 | session := NewSession(scene) | |
| @@ -180,6 +180,18 @@ func TestPlacementIDsAreScopedToTheirImage(t *testing.T) { | |||||
| 180 | } | 180 | } | ||
| 181 | } | 181 | } | ||
| 182 | 182 | ||||
| 183 | + | func TestParseAPCAcceptsPayloadFreeCommands(t *testing.T) { | |||
| 184 | + | for _, raw := range []string{"\x1b_Ga=d,d=a\x1b\\", "\x1b_Ga=p,i=1\x1b\\"} { | |||
| 185 | + | command, err := ParseAPC([]byte(raw)) | |||
| 186 | + | if err != nil { | |||
| 187 | + | t.Fatalf("ParseAPC(%q) error = %v", raw, err) | |||
| 188 | + | } | |||
| 189 | + | if len(command.Payload) != 0 { | |||
| 190 | + | t.Fatalf("ParseAPC(%q) payload = %q", raw, command.Payload) | |||
| 191 | + | } | |||
| 192 | + | } | |||
| 193 | + | } | |||
| 194 | + | ||||
| 183 | func TestMalformedTruncatedInterleavedAndOversizeAreBounded(t *testing.T) { | 195 | func TestMalformedTruncatedInterleavedAndOversizeAreBounded(t *testing.T) { | ||
| 184 | if _, err := ParseCommand([]byte("a=q,a=t;")); !errors.Is(err, ErrDuplicateControl) { | 196 | if _, err := ParseCommand([]byte("a=q,a=t;")); !errors.Is(err, ErrDuplicateControl) { | ||
| 185 | t.Fatalf("duplicate control error = %v", err) | 197 | t.Fatalf("duplicate control error = %v", err) | ||
| @@ -286,6 +298,19 @@ func TestChunkedTransmitAndDisplayRetainsDisplayAction(t *testing.T) { | |||||
| 286 | } | 298 | } | ||
| 287 | } | 299 | } | ||
| 288 | 300 | ||||
| 301 | + | func TestControlsIgnoreExtensionsAndBoundIntegerFields(t *testing.T) { | |||
| 302 | + | controls, err := ParseControls([]byte("a=p,i=1,N=1,S=2,O=3")) | |||
| 303 | + | if err != nil || controls.ImageID != 1 { | |||
| 304 | + | t.Fatalf("extension controls = %#v, %v", controls, err) | |||
| 305 | + | } | |||
| 306 | + | if _, err := ParseControls([]byte("a=p,s=4294967296")); !errors.Is(err, ErrIntegerOverflow) { | |||
| 307 | + | t.Fatalf("out-of-range uint32 control error = %v", err) | |||
| 308 | + | } | |||
| 309 | + | if _, err := ParseControls([]byte("a=p,z=2147483648")); !errors.Is(err, ErrIntegerOverflow) { | |||
| 310 | + | t.Fatalf("out-of-range int32 control error = %v", err) | |||
| 311 | + | } | |||
| 312 | + | } | |||
| 313 | + | ||||
| 289 | func TestImplicitPlacementIDsStopAtKittyLimit(t *testing.T) { | 314 | func TestImplicitPlacementIDsStopAtKittyLimit(t *testing.T) { | ||
| 290 | session := NewSession(nil) | 315 | session := NewSession(nil) | ||
| 291 | session.nextPlacement = MaxKittyID + 1 | 316 | session.nextPlacement = MaxKittyID + 1 | ||
| @@ -294,6 +319,40 @@ func TestImplicitPlacementIDsStopAtKittyLimit(t *testing.T) { | |||||
| 294 | } | 319 | } | ||
| 295 | } | 320 | } | ||
| 296 | 321 | ||||
| 322 | + | func TestUnsupportedRelativeAndCellDeleteControlsFailClosed(t *testing.T) { | |||
| 323 | + | session := NewSession(graphics.NewScene(graphics.Limits{})) | |||
| 324 | + | payload := base64.RawStdEncoding.EncodeToString([]byte{1, 2, 3, 4}) | |||
| 325 | + | if _, err := session.Feed(apc("a=t,i=1,f=32,s=1,v=1,q=2", payload)); err != nil { | |||
| 326 | + | t.Fatal(err) | |||
| 327 | + | } | |||
| 328 | + | for _, header := range []string{"a=p,i=1,P=2", "a=p,i=1,H=1", "a=d,d=p,x=1,y=1", "a=d,d=P,x=1,y=1"} { | |||
| 329 | + | if _, err := session.Feed(apc(header, "")); !errors.Is(err, ErrUnsupported) { | |||
| 330 | + | t.Fatalf("%q error = %v, want ErrUnsupported", header, err) | |||
| 331 | + | } | |||
| 332 | + | } | |||
| 333 | + | } | |||
| 334 | + | ||||
| 335 | + | func TestPNGDecodedByteBudgetAndAssetFormat(t *testing.T) { | |||
| 336 | + | data := pngBytes(t, 2, 2) | |||
| 337 | + | if _, _, err := imageGeometry(data, FormatPNG, Controls{}, normalizeLimits(Limits{MaxDecodedBytes: 15})); !errors.Is(err, ErrPayloadTooLarge) { | |||
| 338 | + | t.Fatalf("PNG byte-budget error = %v", err) | |||
| 339 | + | } | |||
| 340 | + | scene := graphics.NewScene(graphics.Limits{}) | |||
| 341 | + | session := NewSession(scene) | |||
| 342 | + | payload := base64.RawStdEncoding.EncodeToString(data) | |||
| 343 | + | if _, err := session.Feed(apc("a=t,i=1,f=100,q=2", payload)); err != nil { | |||
| 344 | + | t.Fatal(err) | |||
| 345 | + | } | |||
| 346 | + | assetID, ok := session.Image(1) | |||
| 347 | + | if !ok { | |||
| 348 | + | t.Fatal("image mapping missing") | |||
| 349 | + | } | |||
| 350 | + | asset, ok := scene.Snapshot().Asset(assetID) | |||
| 351 | + | if !ok || asset.Format() != graphics.AssetFormatPNG || asset.Blob().Format != graphics.AssetFormatPNG { | |||
| 352 | + | t.Fatalf("asset format = %#v, ok=%v", asset, ok) | |||
| 353 | + | } | |||
| 354 | + | } | |||
| 355 | + | ||||
| 297 | func TestPlacementFailureEmitsStableProtocolError(t *testing.T) { | 356 | func TestPlacementFailureEmitsStableProtocolError(t *testing.T) { | ||
| 298 | scene := graphics.NewScene(graphics.Limits{}) | 357 | scene := graphics.NewScene(graphics.Limits{}) | ||
| 299 | session := NewSession(scene) | 358 | session := NewSession(scene) | ||
M protocol/kittygraphics/parser.go +10 -15
| @@ -1,9 +1,6 @@ | |||
| 1 | 1 | package kittygraphics | |
| 2 | 2 | ||
| 3 | - | import ( | |
| 4 | - | "bytes" | |
| 5 | - | "fmt" | |
| 6 | - | ) | |
| 3 | + | import "bytes" | |
| 7 | 4 | ||
| 8 | 5 | var apcPrefix = [...]byte{0x1b, '_', 'G'} | |
| 9 | 6 | ||
| @@ -42,7 +39,6 @@ func (p *Parser) Feed(data []byte) []Event { | |||
| 42 | 39 | flushText() | |
| 43 | 40 | p.candidate = p.candidate[:0] | |
| 44 | 41 | p.inAPC = true | |
| 45 | - | p.discard = false | |
| 46 | 42 | p.escaped = false | |
| 47 | 43 | p.oversize = false | |
| 48 | 44 | p.apc = p.apc[:0] | |
| @@ -52,7 +48,7 @@ func (p *Parser) Feed(data []byte) []Event { | |||
| 52 | 48 | ||
| 53 | 49 | if p.escaped { | |
| 54 | 50 | if b == '\\' { | |
| 55 | - | if p.discard || p.oversize { | |
| 51 | + | if p.oversize { | |
| 56 | 52 | events = append(events, p.errorEvent()) | |
| 57 | 53 | } else { | |
| 58 | 54 | events = append(events, p.commandEvent()) | |
| @@ -67,7 +63,7 @@ func (p *Parser) Feed(data []byte) []Event { | |||
| 67 | 63 | p.escaped = false | |
| 68 | 64 | } | |
| 69 | 65 | if b == 0x9c { // C1 ST is also a valid APC terminator. | |
| 70 | - | if p.discard || p.oversize { | |
| 66 | + | if p.oversize { | |
| 71 | 67 | events = append(events, p.errorEvent()) | |
| 72 | 68 | } else { | |
| 73 | 69 | events = append(events, p.commandEvent()) | |
| @@ -127,7 +123,7 @@ func (p *Parser) Finish() []Event { | |||
| 127 | 123 | func (p *Parser) Flush() []Event { return p.Finish() } | |
| 128 | 124 | ||
| 129 | 125 | func (p *Parser) appendAPC(b byte) { | |
| 130 | - | if p.discard || p.oversize { | |
| 126 | + | if p.oversize { | |
| 131 | 127 | return | |
| 132 | 128 | } | |
| 133 | 129 | if uint64(len(p.apc))+1 > p.limits.MaxAPCBytes { | |
| @@ -139,7 +135,6 @@ func (p *Parser) appendAPC(b byte) { | |||
| 139 | 135 | ||
| 140 | 136 | func (p *Parser) resetAPC() { | |
| 141 | 137 | p.inAPC = false | |
| 142 | - | p.discard = false | |
| 143 | 138 | p.escaped = false | |
| 144 | 139 | p.oversize = false | |
| 145 | 140 | p.apc = p.apc[:0] | |
| @@ -172,15 +167,15 @@ func ParseCommand(body []byte, config ...Limits) (Command, error) { | |||
| 172 | 167 | if uint64(len(body)) > l.MaxAPCBytes { | |
| 173 | 168 | return Command{}, ErrAPCTooLarge | |
| 174 | 169 | } | |
| 175 | - | sep := bytes.IndexByte(body, ';') | |
| 176 | - | if sep < 0 { | |
| 177 | - | return Command{}, fmt.Errorf("%w: missing payload separator", ErrInvalidCommand) | |
| 170 | + | controlEnd, payloadStart := len(body), len(body) | |
| 171 | + | if sep := bytes.IndexByte(body, ';'); sep >= 0 { | |
| 172 | + | controlEnd, payloadStart = sep, sep+1 | |
| 178 | 173 | } | |
| 179 | - | controls, err := parseControls(body[:sep]) | |
| 174 | + | controls, err := parseControls(body[:controlEnd]) | |
| 180 | 175 | if err != nil { | |
| 181 | 176 | return Command{}, err | |
| 182 | 177 | } | |
| 183 | - | payload := body[sep+1:] | |
| 178 | + | payload := body[payloadStart:] | |
| 184 | 179 | if uint64(len(payload)) > l.MaxPayloadBytes { | |
| 185 | 180 | return Command{}, ErrPayloadTooLarge | |
| 186 | 181 | } | |
| @@ -189,7 +184,7 @@ func ParseCommand(body []byte, config ...Limits) (Command, error) { | |||
| 189 | 184 | ||
| 190 | 185 | // ParseAPC parses a complete ESC _ G ... ST sequence. | |
| 191 | 186 | func ParseAPC(apc []byte, config ...Limits) (Command, error) { | |
| 192 | - | if len(apc) < 5 || apc[0] != 0x1b || apc[1] != '_' || apc[2] != 'G' { | |
| 187 | + | if len(apc) < 4 || apc[0] != 0x1b || apc[1] != '_' || apc[2] != 'G' { | |
| 193 | 188 | return Command{}, ErrInvalidAPC | |
| 194 | 189 | } | |
| 195 | 190 | body := apc[3:] | |
| @@ -1,9 +1,6 @@ | |||||
| 1 | package kittygraphics | 1 | package kittygraphics | ||
| 2 | 2 | ||||
| 3 | - | import ( | 3 | + | import "bytes" |
| 4 | - | "bytes" | |||
| 5 | - | "fmt" | |||
| 6 | - | ) | |||
| 7 | 4 | ||||
| 8 | var apcPrefix = [...]byte{0x1b, '_', 'G'} | 5 | var apcPrefix = [...]byte{0x1b, '_', 'G'} | ||
| 9 | 6 | ||||
| @@ -42,7 +39,6 @@ func (p *Parser) Feed(data []byte) []Event { | |||||
| 42 | flushText() | 39 | flushText() | ||
| 43 | p.candidate = p.candidate[:0] | 40 | p.candidate = p.candidate[:0] | ||
| 44 | p.inAPC = true | 41 | p.inAPC = true | ||
| 45 | - | p.discard = false | |||
| 46 | p.escaped = false | 42 | p.escaped = false | ||
| 47 | p.oversize = false | 43 | p.oversize = false | ||
| 48 | p.apc = p.apc[:0] | 44 | p.apc = p.apc[:0] | ||
| @@ -52,7 +48,7 @@ func (p *Parser) Feed(data []byte) []Event { | |||||
| 52 | 48 | ||||
| 53 | if p.escaped { | 49 | if p.escaped { | ||
| 54 | if b == '\\' { | 50 | if b == '\\' { | ||
| 55 | - | if p.discard || p.oversize { | 51 | + | if p.oversize { |
| 56 | events = append(events, p.errorEvent()) | 52 | events = append(events, p.errorEvent()) | ||
| 57 | } else { | 53 | } else { | ||
| 58 | events = append(events, p.commandEvent()) | 54 | events = append(events, p.commandEvent()) | ||
| @@ -67,7 +63,7 @@ func (p *Parser) Feed(data []byte) []Event { | |||||
| 67 | p.escaped = false | 63 | p.escaped = false | ||
| 68 | } | 64 | } | ||
| 69 | if b == 0x9c { // C1 ST is also a valid APC terminator. | 65 | if b == 0x9c { // C1 ST is also a valid APC terminator. | ||
| 70 | - | if p.discard || p.oversize { | 66 | + | if p.oversize { |
| 71 | events = append(events, p.errorEvent()) | 67 | events = append(events, p.errorEvent()) | ||
| 72 | } else { | 68 | } else { | ||
| 73 | events = append(events, p.commandEvent()) | 69 | events = append(events, p.commandEvent()) | ||
| @@ -127,7 +123,7 @@ func (p *Parser) Finish() []Event { | |||||
| 127 | func (p *Parser) Flush() []Event { return p.Finish() } | 123 | func (p *Parser) Flush() []Event { return p.Finish() } | ||
| 128 | 124 | ||||
| 129 | func (p *Parser) appendAPC(b byte) { | 125 | func (p *Parser) appendAPC(b byte) { | ||
| 130 | - | if p.discard || p.oversize { | 126 | + | if p.oversize { |
| 131 | return | 127 | return | ||
| 132 | } | 128 | } | ||
| 133 | if uint64(len(p.apc))+1 > p.limits.MaxAPCBytes { | 129 | if uint64(len(p.apc))+1 > p.limits.MaxAPCBytes { | ||
| @@ -139,7 +135,6 @@ func (p *Parser) appendAPC(b byte) { | |||||
| 139 | 135 | ||||
| 140 | func (p *Parser) resetAPC() { | 136 | func (p *Parser) resetAPC() { | ||
| 141 | p.inAPC = false | 137 | p.inAPC = false | ||
| 142 | - | p.discard = false | |||
| 143 | p.escaped = false | 138 | p.escaped = false | ||
| 144 | p.oversize = false | 139 | p.oversize = false | ||
| 145 | p.apc = p.apc[:0] | 140 | p.apc = p.apc[:0] | ||
| @@ -172,15 +167,15 @@ func ParseCommand(body []byte, config ...Limits) (Command, error) { | |||||
| 172 | if uint64(len(body)) > l.MaxAPCBytes { | 167 | if uint64(len(body)) > l.MaxAPCBytes { | ||
| 173 | return Command{}, ErrAPCTooLarge | 168 | return Command{}, ErrAPCTooLarge | ||
| 174 | } | 169 | } | ||
| 175 | - | sep := bytes.IndexByte(body, ';') | 170 | + | controlEnd, payloadStart := len(body), len(body) |
| 176 | - | if sep < 0 { | 171 | + | if sep := bytes.IndexByte(body, ';'); sep >= 0 { |
| 177 | - | return Command{}, fmt.Errorf("%w: missing payload separator", ErrInvalidCommand) | 172 | + | controlEnd, payloadStart = sep, sep+1 |
| 178 | } | 173 | } | ||
| 179 | - | controls, err := parseControls(body[:sep]) | 174 | + | controls, err := parseControls(body[:controlEnd]) |
| 180 | if err != nil { | 175 | if err != nil { | ||
| 181 | return Command{}, err | 176 | return Command{}, err | ||
| 182 | } | 177 | } | ||
| 183 | - | payload := body[sep+1:] | 178 | + | payload := body[payloadStart:] |
| 184 | if uint64(len(payload)) > l.MaxPayloadBytes { | 179 | if uint64(len(payload)) > l.MaxPayloadBytes { | ||
| 185 | return Command{}, ErrPayloadTooLarge | 180 | return Command{}, ErrPayloadTooLarge | ||
| 186 | } | 181 | } | ||
| @@ -189,7 +184,7 @@ func ParseCommand(body []byte, config ...Limits) (Command, error) { | |||||
| 189 | 184 | ||||
| 190 | // ParseAPC parses a complete ESC _ G ... ST sequence. | 185 | // ParseAPC parses a complete ESC _ G ... ST sequence. | ||
| 191 | func ParseAPC(apc []byte, config ...Limits) (Command, error) { | 186 | func ParseAPC(apc []byte, config ...Limits) (Command, error) { | ||
| 192 | - | if len(apc) < 5 || apc[0] != 0x1b || apc[1] != '_' || apc[2] != 'G' { | 187 | + | if len(apc) < 4 || apc[0] != 0x1b || apc[1] != '_' || apc[2] != 'G' { |
| 193 | return Command{}, ErrInvalidAPC | 188 | return Command{}, ErrInvalidAPC | ||
| 194 | } | 189 | } | ||
| 195 | body := apc[3:] | 190 | body := apc[3:] | ||
M protocol/kittygraphics/session.go +33 -33
| @@ -243,12 +243,10 @@ func (s *Session) apply(command Command) ([][]byte, *Mutation, error) { | |||
| 243 | 243 | func (s *Session) transmit(command Command, display bool) ([][]byte, *Mutation, error) { | |
| 244 | 244 | c := command.Controls | |
| 245 | 245 | imageID := c.ImageID | |
| 246 | - | persistent := c.HasImageID && imageID != 0 | |
| 247 | - | if !persistent { | |
| 246 | + | if !c.HasImageID || imageID == 0 { | |
| 248 | 247 | imageID = 0 | |
| 249 | 248 | if c.HasImageNumber { | |
| 250 | 249 | imageID = s.nextImageID() | |
| 251 | - | persistent = true | |
| 252 | 250 | } | |
| 253 | 251 | } | |
| 254 | 252 | if s.upload != nil { | |
| @@ -338,12 +336,14 @@ func (s *Session) commitUpload(value upload, display bool) ([][]byte, *Mutation, | |||
| 338 | 336 | // old asset and every placement that references it untouched. | |
| 339 | 337 | assetID, err = s.scene.ReplaceAsset(old, graphics.AssetBlob{ | |
| 340 | 338 | Encoded: decoded, | |
| 339 | + | Format: assetFormat(format), | |
| 341 | 340 | Width: width, | |
| 342 | 341 | Height: height, | |
| 343 | 342 | }) | |
| 344 | 343 | } else { | |
| 345 | 344 | assetID, err = s.scene.AddAsset(graphics.AssetBlob{ | |
| 346 | 345 | Encoded: decoded, | |
| 346 | + | Format: assetFormat(format), | |
| 347 | 347 | Width: width, | |
| 348 | 348 | Height: height, | |
| 349 | 349 | }) | |
| @@ -390,6 +390,19 @@ func (s *Session) commitUpload(value upload, display bool) ([][]byte, *Mutation, | |||
| 390 | 390 | return s.success(value.controls, value.imageID), mutation, nil | |
| 391 | 391 | } | |
| 392 | 392 | ||
| 393 | + | func assetFormat(format Format) graphics.AssetFormat { | |
| 394 | + | switch format { | |
| 395 | + | case FormatRGB: | |
| 396 | + | return graphics.AssetFormatRGB | |
| 397 | + | case FormatRGBA: | |
| 398 | + | return graphics.AssetFormatRGBA | |
| 399 | + | case FormatPNG: | |
| 400 | + | return graphics.AssetFormatPNG | |
| 401 | + | default: | |
| 402 | + | return graphics.AssetFormatUnknown | |
| 403 | + | } | |
| 404 | + | } | |
| 405 | + | ||
| 393 | 406 | func imageGeometry(data []byte, format Format, c Controls, limits Limits) (int64, int64, error) { | |
| 394 | 407 | if format == FormatPNG { | |
| 395 | 408 | config, err := png.DecodeConfig(bytes.NewReader(data)) | |
| @@ -403,7 +416,8 @@ func imageGeometry(data []byte, format Format, c Controls, limits Limits) (int64 | |||
| 403 | 416 | return 0, 0, ErrPayloadTooLarge | |
| 404 | 417 | } | |
| 405 | 418 | pixels, ok := checkedProduct(uint64(config.Width), uint64(config.Height)) | |
| 406 | - | if !ok || pixels > limits.MaxDecodedPixels { | |
| 419 | + | decodedBytes, bytesOK := checkedProduct(pixels, 4) | |
| 420 | + | if !ok || !bytesOK || pixels > limits.MaxDecodedPixels || decodedBytes > limits.MaxDecodedBytes { | |
| 407 | 421 | return 0, 0, ErrPayloadTooLarge | |
| 408 | 422 | } | |
| 409 | 423 | if c.HasWidth && c.Width != uint64(config.Width) || c.HasHeight && c.Height != uint64(config.Height) { | |
| @@ -558,6 +572,9 @@ func placementSpec(c Controls, assetID graphics.AssetID, width, height int64, or | |||
| 558 | 572 | } | |
| 559 | 573 | ||
| 560 | 574 | func (s *Session) place(c Controls, imageID uint64, assetID graphics.AssetID, origin placementOrigin) ([][]byte, *Mutation, error) { | |
| 575 | + | if c.HasParent || c.HasCellOffsetX || c.HasCellOffsetY { | |
| 576 | + | return nil, nil, ErrUnsupported | |
| 577 | + | } | |
| 561 | 578 | if s.scene == nil { | |
| 562 | 579 | return nil, nil, ErrNoScene | |
| 563 | 580 | } | |
| @@ -658,39 +675,22 @@ func (s *Session) delete(c Controls) ([][]byte, *Mutation, error) { | |||
| 658 | 675 | return s.failure(c, responseID, err) | |
| 659 | 676 | } | |
| 660 | 677 | return s.success(c, responseID), &Mutation{Kind: MutationDeletePlacements}, nil | |
| 661 | - | case DeletePlacement: | |
| 662 | - | placementID := c.PlacementID | |
| 663 | - | key := placementKey{imageID: id, placementID: placementID} | |
| 664 | - | mapped, ok := s.placements[key] | |
| 665 | - | if !selected || placementID == 0 || !ok || s.scene == nil { | |
| 666 | - | return s.failure(c, responseID, ErrPlacementNotFound) | |
| 667 | - | } | |
| 668 | - | if err := s.scene.RemovePlacement(mapped); err != nil { | |
| 669 | - | return s.failure(c, responseID, err) | |
| 670 | - | } | |
| 671 | - | delete(s.placements, key) | |
| 672 | - | return s.success(c, responseID), &Mutation{Kind: MutationDeletePlacement, ImageID: id, PlacementID: placementID, ScenePlacementID: mapped}, nil | |
| 673 | - | case DeleteAllImages, DeleteAllPlacements: | |
| 678 | + | case DeleteCell, DeleteCellAll: | |
| 679 | + | // Kitty d=p/P deletes by cell intersection, not by placement ID or | |
| 680 | + | // all placements. Those selectors remain unsupported until the scene | |
| 681 | + | // tracks terminal cell anchors. | |
| 682 | + | return s.failure(c, responseID, ErrUnsupported) | |
| 683 | + | case DeleteAllImages: | |
| 674 | 684 | if s.scene == nil { | |
| 675 | 685 | return s.failure(c, responseID, ErrNoScene) | |
| 676 | 686 | } | |
| 677 | - | if target == DeleteAllPlacements { | |
| 678 | - | placementIDs := make([]placementKey, 0, len(s.placements)) | |
| 679 | - | for placementID := range s.placements { | |
| 680 | - | placementIDs = append(placementIDs, placementID) | |
| 681 | - | } | |
| 682 | - | if err := s.removePlacements(placementIDs); err != nil { | |
| 683 | - | return s.failure(c, responseID, err) | |
| 684 | - | } | |
| 685 | - | } else { | |
| 686 | - | if err := s.scene.Clear(); err != nil { | |
| 687 | - | return s.failure(c, responseID, err) | |
| 688 | - | } | |
| 689 | - | s.images = make(map[uint64]graphics.AssetID) | |
| 690 | - | s.imageNumbers = make(map[uint64]uint64) | |
| 691 | - | s.placements = make(map[placementKey]graphics.PlacementID) | |
| 692 | - | s.children = make(map[uint64]Child) | |
| 687 | + | if err := s.scene.Clear(); err != nil { | |
| 688 | + | return s.failure(c, responseID, err) | |
| 693 | 689 | } | |
| 690 | + | s.images = make(map[uint64]graphics.AssetID) | |
| 691 | + | s.imageNumbers = make(map[uint64]uint64) | |
| 692 | + | s.placements = make(map[placementKey]graphics.PlacementID) | |
| 693 | + | s.children = make(map[uint64]Child) | |
| 694 | 694 | return s.success(c, responseID), &Mutation{Kind: MutationClear}, nil | |
| 695 | 695 | default: | |
| 696 | 696 | return s.failure(c, responseID, ErrUnsupported) | |
| @@ -243,12 +243,10 @@ func (s *Session) apply(command Command) ([][]byte, *Mutation, error) { | |||||
| 243 | func (s *Session) transmit(command Command, display bool) ([][]byte, *Mutation, error) { | 243 | func (s *Session) transmit(command Command, display bool) ([][]byte, *Mutation, error) { | ||
| 244 | c := command.Controls | 244 | c := command.Controls | ||
| 245 | imageID := c.ImageID | 245 | imageID := c.ImageID | ||
| 246 | - | persistent := c.HasImageID && imageID != 0 | 246 | + | if !c.HasImageID || imageID == 0 { |
| 247 | - | if !persistent { | |||
| 248 | imageID = 0 | 247 | imageID = 0 | ||
| 249 | if c.HasImageNumber { | 248 | if c.HasImageNumber { | ||
| 250 | imageID = s.nextImageID() | 249 | imageID = s.nextImageID() | ||
| 251 | - | persistent = true | |||
| 252 | } | 250 | } | ||
| 253 | } | 251 | } | ||
| 254 | if s.upload != nil { | 252 | if s.upload != nil { | ||
| @@ -338,12 +336,14 @@ func (s *Session) commitUpload(value upload, display bool) ([][]byte, *Mutation, | |||||
| 338 | // old asset and every placement that references it untouched. | 336 | // old asset and every placement that references it untouched. | ||
| 339 | assetID, err = s.scene.ReplaceAsset(old, graphics.AssetBlob{ | 337 | assetID, err = s.scene.ReplaceAsset(old, graphics.AssetBlob{ | ||
| 340 | Encoded: decoded, | 338 | Encoded: decoded, | ||
| 339 | + | Format: assetFormat(format), | |||
| 341 | Width: width, | 340 | Width: width, | ||
| 342 | Height: height, | 341 | Height: height, | ||
| 343 | }) | 342 | }) | ||
| 344 | } else { | 343 | } else { | ||
| 345 | assetID, err = s.scene.AddAsset(graphics.AssetBlob{ | 344 | assetID, err = s.scene.AddAsset(graphics.AssetBlob{ | ||
| 346 | Encoded: decoded, | 345 | Encoded: decoded, | ||
| 346 | + | Format: assetFormat(format), | |||
| 347 | Width: width, | 347 | Width: width, | ||
| 348 | Height: height, | 348 | Height: height, | ||
| 349 | }) | 349 | }) | ||
| @@ -390,6 +390,19 @@ func (s *Session) commitUpload(value upload, display bool) ([][]byte, *Mutation, | |||||
| 390 | return s.success(value.controls, value.imageID), mutation, nil | 390 | return s.success(value.controls, value.imageID), mutation, nil | ||
| 391 | } | 391 | } | ||
| 392 | 392 | ||||
| 393 | + | func assetFormat(format Format) graphics.AssetFormat { | |||
| 394 | + | switch format { | |||
| 395 | + | case FormatRGB: | |||
| 396 | + | return graphics.AssetFormatRGB | |||
| 397 | + | case FormatRGBA: | |||
| 398 | + | return graphics.AssetFormatRGBA | |||
| 399 | + | case FormatPNG: | |||
| 400 | + | return graphics.AssetFormatPNG | |||
| 401 | + | default: | |||
| 402 | + | return graphics.AssetFormatUnknown | |||
| 403 | + | } | |||
| 404 | + | } | |||
| 405 | + | ||||
| 393 | func imageGeometry(data []byte, format Format, c Controls, limits Limits) (int64, int64, error) { | 406 | func imageGeometry(data []byte, format Format, c Controls, limits Limits) (int64, int64, error) { | ||
| 394 | if format == FormatPNG { | 407 | if format == FormatPNG { | ||
| 395 | config, err := png.DecodeConfig(bytes.NewReader(data)) | 408 | config, err := png.DecodeConfig(bytes.NewReader(data)) | ||
| @@ -403,7 +416,8 @@ func imageGeometry(data []byte, format Format, c Controls, limits Limits) (int64 | |||||
| 403 | return 0, 0, ErrPayloadTooLarge | 416 | return 0, 0, ErrPayloadTooLarge | ||
| 404 | } | 417 | } | ||
| 405 | pixels, ok := checkedProduct(uint64(config.Width), uint64(config.Height)) | 418 | pixels, ok := checkedProduct(uint64(config.Width), uint64(config.Height)) | ||
| 406 | - | if !ok || pixels > limits.MaxDecodedPixels { | 419 | + | decodedBytes, bytesOK := checkedProduct(pixels, 4) |
| 420 | + | if !ok || !bytesOK || pixels > limits.MaxDecodedPixels || decodedBytes > limits.MaxDecodedBytes { | |||
| 407 | return 0, 0, ErrPayloadTooLarge | 421 | return 0, 0, ErrPayloadTooLarge | ||
| 408 | } | 422 | } | ||
| 409 | if c.HasWidth && c.Width != uint64(config.Width) || c.HasHeight && c.Height != uint64(config.Height) { | 423 | if c.HasWidth && c.Width != uint64(config.Width) || c.HasHeight && c.Height != uint64(config.Height) { | ||
| @@ -558,6 +572,9 @@ func placementSpec(c Controls, assetID graphics.AssetID, width, height int64, or | |||||
| 558 | } | 572 | } | ||
| 559 | 573 | ||||
| 560 | func (s *Session) place(c Controls, imageID uint64, assetID graphics.AssetID, origin placementOrigin) ([][]byte, *Mutation, error) { | 574 | func (s *Session) place(c Controls, imageID uint64, assetID graphics.AssetID, origin placementOrigin) ([][]byte, *Mutation, error) { | ||
| 575 | + | if c.HasParent || c.HasCellOffsetX || c.HasCellOffsetY { | |||
| 576 | + | return nil, nil, ErrUnsupported | |||
| 577 | + | } | |||
| 561 | if s.scene == nil { | 578 | if s.scene == nil { | ||
| 562 | return nil, nil, ErrNoScene | 579 | return nil, nil, ErrNoScene | ||
| 563 | } | 580 | } | ||
| @@ -658,39 +675,22 @@ func (s *Session) delete(c Controls) ([][]byte, *Mutation, error) { | |||||
| 658 | return s.failure(c, responseID, err) | 675 | return s.failure(c, responseID, err) | ||
| 659 | } | 676 | } | ||
| 660 | return s.success(c, responseID), &Mutation{Kind: MutationDeletePlacements}, nil | 677 | return s.success(c, responseID), &Mutation{Kind: MutationDeletePlacements}, nil | ||
| 661 | - | case DeletePlacement: | 678 | + | case DeleteCell, DeleteCellAll: |
| 662 | - | placementID := c.PlacementID | 679 | + | // Kitty d=p/P deletes by cell intersection, not by placement ID or |
| 663 | - | key := placementKey{imageID: id, placementID: placementID} | 680 | + | // all placements. Those selectors remain unsupported until the scene |
| 664 | - | mapped, ok := s.placements[key] | 681 | + | // tracks terminal cell anchors. |
| 665 | - | if !selected || placementID == 0 || !ok || s.scene == nil { | 682 | + | return s.failure(c, responseID, ErrUnsupported) |
| 666 | - | return s.failure(c, responseID, ErrPlacementNotFound) | 683 | + | case DeleteAllImages: |
| 667 | - | } | |||
| 668 | - | if err := s.scene.RemovePlacement(mapped); err != nil { | |||
| 669 | - | return s.failure(c, responseID, err) | |||
| 670 | - | } | |||
| 671 | - | delete(s.placements, key) | |||
| 672 | - | return s.success(c, responseID), &Mutation{Kind: MutationDeletePlacement, ImageID: id, PlacementID: placementID, ScenePlacementID: mapped}, nil | |||
| 673 | - | case DeleteAllImages, DeleteAllPlacements: | |||
| 674 | if s.scene == nil { | 684 | if s.scene == nil { | ||
| 675 | return s.failure(c, responseID, ErrNoScene) | 685 | return s.failure(c, responseID, ErrNoScene) | ||
| 676 | } | 686 | } | ||
| 677 | - | if target == DeleteAllPlacements { | 687 | + | if err := s.scene.Clear(); err != nil { |
| 678 | - | placementIDs := make([]placementKey, 0, len(s.placements)) | 688 | + | return s.failure(c, responseID, err) |
| 679 | - | for placementID := range s.placements { | |||
| 680 | - | placementIDs = append(placementIDs, placementID) | |||
| 681 | - | } | |||
| 682 | - | if err := s.removePlacements(placementIDs); err != nil { | |||
| 683 | - | return s.failure(c, responseID, err) | |||
| 684 | - | } | |||
| 685 | - | } else { | |||
| 686 | - | if err := s.scene.Clear(); err != nil { | |||
| 687 | - | return s.failure(c, responseID, err) | |||
| 688 | - | } | |||
| 689 | - | s.images = make(map[uint64]graphics.AssetID) | |||
| 690 | - | s.imageNumbers = make(map[uint64]uint64) | |||
| 691 | - | s.placements = make(map[placementKey]graphics.PlacementID) | |||
| 692 | - | s.children = make(map[uint64]Child) | |||
| 693 | } | 689 | } | ||
| 690 | + | s.images = make(map[uint64]graphics.AssetID) | |||
| 691 | + | s.imageNumbers = make(map[uint64]uint64) | |||
| 692 | + | s.placements = make(map[placementKey]graphics.PlacementID) | |||
| 693 | + | s.children = make(map[uint64]Child) | |||
| 694 | return s.success(c, responseID), &Mutation{Kind: MutationClear}, nil | 694 | return s.success(c, responseID), &Mutation{Kind: MutationClear}, nil | ||
| 695 | default: | 695 | default: | ||
| 696 | return s.failure(c, responseID, ErrUnsupported) | 696 | return s.failure(c, responseID, ErrUnsupported) | ||
M protocol/terminalquery/query.go +22 -2
| @@ -34,8 +34,17 @@ func (p *Probe) Feed(data []byte) []byte { | |||
| 34 | 34 | if p == nil || len(data) == 0 { | |
| 35 | 35 | return append([]byte(nil), data...) | |
| 36 | 36 | } | |
| 37 | - | p.pending = append(p.pending, data...) | |
| 38 | - | return p.scan(false) | |
| 37 | + | // Never retain or copy an unbounded caller fragment into pending. scan | |
| 38 | + | // leaves at most a bounded incomplete probe prefix, so feeding bounded | |
| 39 | + | // chunks also prevents a small suffix from retaining a large backing array. | |
| 40 | + | var unrelated []byte | |
| 41 | + | for len(data) != 0 { | |
| 42 | + | n := min(len(data), maxProbeResponseBytes+2) | |
| 43 | + | p.pending = append(p.pending, data[:n]...) | |
| 44 | + | unrelated = append(unrelated, p.scan(false)...) | |
| 45 | + | data = data[n:] | |
| 46 | + | } | |
| 47 | + | return unrelated | |
| 39 | 48 | } | |
| 40 | 49 | ||
| 41 | 50 | // Finish flushes an incomplete or unrelated prefix without recognizing any | |
| @@ -79,6 +88,7 @@ func (p *Probe) scan(flush bool) []byte { | |||
| 79 | 88 | p.pending = p.pending[len(p.pending)-keep:] | |
| 80 | 89 | } | |
| 81 | 90 | } | |
| 91 | + | p.retainPending() | |
| 82 | 92 | break | |
| 83 | 93 | } | |
| 84 | 94 | if start > 0 { | |
| @@ -93,6 +103,7 @@ func (p *Probe) scan(flush bool) []byte { | |||
| 93 | 103 | p.pending = p.pending[1:] | |
| 94 | 104 | continue | |
| 95 | 105 | } | |
| 106 | + | p.retainPending() | |
| 96 | 107 | break | |
| 97 | 108 | } | |
| 98 | 109 | response := p.pending[:end] | |
| @@ -112,6 +123,7 @@ func (p *Probe) scan(flush bool) []byte { | |||
| 112 | 123 | p.pending = p.pending[1:] | |
| 113 | 124 | continue | |
| 114 | 125 | } | |
| 126 | + | p.retainPending() | |
| 115 | 127 | break | |
| 116 | 128 | } | |
| 117 | 129 | response := p.pending[:end] | |
| @@ -126,6 +138,14 @@ func (p *Probe) scan(flush bool) []byte { | |||
| 126 | 138 | return unrelated | |
| 127 | 139 | } | |
| 128 | 140 | ||
| 141 | + | func (p *Probe) retainPending() { | |
| 142 | + | if len(p.pending) == 0 { | |
| 143 | + | p.pending = nil | |
| 144 | + | return | |
| 145 | + | } | |
| 146 | + | p.pending = append([]byte(nil), p.pending...) | |
| 147 | + | } | |
| 148 | + | ||
| 129 | 149 | func probeTerminator(data []byte) (int, bool) { | |
| 130 | 150 | for i := 3; i+1 < len(data) && i <= maxProbeResponseBytes; i++ { | |
| 131 | 151 | if data[i] == '\x1b' && data[i+1] == '\\' { | |
| @@ -34,8 +34,17 @@ func (p *Probe) Feed(data []byte) []byte { | |||||
| 34 | if p == nil || len(data) == 0 { | 34 | if p == nil || len(data) == 0 { | ||
| 35 | return append([]byte(nil), data...) | 35 | return append([]byte(nil), data...) | ||
| 36 | } | 36 | } | ||
| 37 | - | p.pending = append(p.pending, data...) | 37 | + | // Never retain or copy an unbounded caller fragment into pending. scan |
| 38 | - | return p.scan(false) | 38 | + | // leaves at most a bounded incomplete probe prefix, so feeding bounded |
| 39 | + | // chunks also prevents a small suffix from retaining a large backing array. | |||
| 40 | + | var unrelated []byte | |||
| 41 | + | for len(data) != 0 { | |||
| 42 | + | n := min(len(data), maxProbeResponseBytes+2) | |||
| 43 | + | p.pending = append(p.pending, data[:n]...) | |||
| 44 | + | unrelated = append(unrelated, p.scan(false)...) | |||
| 45 | + | data = data[n:] | |||
| 46 | + | } | |||
| 47 | + | return unrelated | |||
| 39 | } | 48 | } | ||
| 40 | 49 | ||||
| 41 | // Finish flushes an incomplete or unrelated prefix without recognizing any | 50 | // Finish flushes an incomplete or unrelated prefix without recognizing any | ||
| @@ -79,6 +88,7 @@ func (p *Probe) scan(flush bool) []byte { | |||||
| 79 | p.pending = p.pending[len(p.pending)-keep:] | 88 | p.pending = p.pending[len(p.pending)-keep:] | ||
| 80 | } | 89 | } | ||
| 81 | } | 90 | } | ||
| 91 | + | p.retainPending() | |||
| 82 | break | 92 | break | ||
| 83 | } | 93 | } | ||
| 84 | if start > 0 { | 94 | if start > 0 { | ||
| @@ -93,6 +103,7 @@ func (p *Probe) scan(flush bool) []byte { | |||||
| 93 | p.pending = p.pending[1:] | 103 | p.pending = p.pending[1:] | ||
| 94 | continue | 104 | continue | ||
| 95 | } | 105 | } | ||
| 106 | + | p.retainPending() | |||
| 96 | break | 107 | break | ||
| 97 | } | 108 | } | ||
| 98 | response := p.pending[:end] | 109 | response := p.pending[:end] | ||
| @@ -112,6 +123,7 @@ func (p *Probe) scan(flush bool) []byte { | |||||
| 112 | p.pending = p.pending[1:] | 123 | p.pending = p.pending[1:] | ||
| 113 | continue | 124 | continue | ||
| 114 | } | 125 | } | ||
| 126 | + | p.retainPending() | |||
| 115 | break | 127 | break | ||
| 116 | } | 128 | } | ||
| 117 | response := p.pending[:end] | 129 | response := p.pending[:end] | ||
| @@ -126,6 +138,14 @@ func (p *Probe) scan(flush bool) []byte { | |||||
| 126 | return unrelated | 138 | return unrelated | ||
| 127 | } | 139 | } | ||
| 128 | 140 | ||||
| 141 | + | func (p *Probe) retainPending() { | |||
| 142 | + | if len(p.pending) == 0 { | |||
| 143 | + | p.pending = nil | |||
| 144 | + | return | |||
| 145 | + | } | |||
| 146 | + | p.pending = append([]byte(nil), p.pending...) | |||
| 147 | + | } | |||
| 148 | + | ||||
| 129 | func probeTerminator(data []byte) (int, bool) { | 149 | func probeTerminator(data []byte) (int, bool) { | ||
| 130 | for i := 3; i+1 < len(data) && i <= maxProbeResponseBytes; i++ { | 150 | for i := 3; i+1 < len(data) && i <= maxProbeResponseBytes; i++ { | ||
| 131 | if data[i] == '\x1b' && data[i+1] == '\\' { | 151 | if data[i] == '\x1b' && data[i+1] == '\\' { | ||
M protocol/terminalquery/query_test.go +8 -0
| @@ -64,6 +64,14 @@ func FuzzProbeNeverPanics(f *testing.F) { | |||
| 64 | 64 | }) | |
| 65 | 65 | } | |
| 66 | 66 | ||
| 67 | + | func TestProbeDoesNotRetainLargeInputFragments(t *testing.T) { | |
| 68 | + | var p Probe | |
| 69 | + | input := append(make([]byte, 1<<20), []byte("\x1b_G")...) | |
| 70 | + | got := append(p.Feed(input), p.Finish()...) | |
| 71 | + | require.Equal(t, input, got) | |
| 72 | + | require.LessOrEqual(t, cap(p.pending), maxProbeResponseBytes+2) | |
| 73 | + | } | |
| 74 | + | ||
| 67 | 75 | func TestProbeBoundsUnterminatedResponse(t *testing.T) { | |
| 68 | 76 | var p Probe | |
| 69 | 77 | input := append([]byte("prefix"), []byte("\x1b_G")...) | |
| @@ -64,6 +64,14 @@ func FuzzProbeNeverPanics(f *testing.F) { | |||||
| 64 | }) | 64 | }) | ||
| 65 | } | 65 | } | ||
| 66 | 66 | ||||
| 67 | + | func TestProbeDoesNotRetainLargeInputFragments(t *testing.T) { | |||
| 68 | + | var p Probe | |||
| 69 | + | input := append(make([]byte, 1<<20), []byte("\x1b_G")...) | |||
| 70 | + | got := append(p.Feed(input), p.Finish()...) | |||
| 71 | + | require.Equal(t, input, got) | |||
| 72 | + | require.LessOrEqual(t, cap(p.pending), maxProbeResponseBytes+2) | |||
| 73 | + | } | |||
| 74 | + | ||||
| 67 | func TestProbeBoundsUnterminatedResponse(t *testing.T) { | 75 | func TestProbeBoundsUnterminatedResponse(t *testing.T) { | ||
| 68 | var p Probe | 76 | var p Probe | ||
| 69 | input := append([]byte("prefix"), []byte("\x1b_G")...) | 77 | input := append([]byte("prefix"), []byte("\x1b_G")...) | ||
M screen_geometry.go +1 -4
| @@ -31,7 +31,7 @@ func (s *Screen) SetGeometry(geometry Geometry) { | |||
| 31 | 31 | // reportWindowSize answers the CSI 14 t and CSI 16 t pixel-geometry queries. | |
| 32 | 32 | // Unknown pixel geometry is intentionally not fabricated. | |
| 33 | 33 | func (s *Screen) reportWindowSize(parts []int) { | |
| 34 | - | if s == nil || len(parts) == 0 || !s.geometry.PixelsKnown() { | |
| 34 | + | if s == nil || len(parts) == 0 || !s.geometry.PixelsKnown() || s.geometry.Rows <= 0 || s.geometry.Cols <= 0 { | |
| 35 | 35 | return | |
| 36 | 36 | } | |
| 37 | 37 | kind := parts[0] | |
| @@ -40,9 +40,6 @@ func (s *Screen) reportWindowSize(parts []int) { | |||
| 40 | 40 | case 14: | |
| 41 | 41 | rows, cols = s.geometry.PixelHeight, s.geometry.PixelWidth | |
| 42 | 42 | case 16: | |
| 43 | - | if s.geometry.Rows <= 0 || s.geometry.Cols <= 0 { | |
| 44 | - | return | |
| 45 | - | } | |
| 46 | 43 | rows, cols = s.geometry.PixelHeight/s.geometry.Rows, s.geometry.PixelWidth/s.geometry.Cols | |
| 47 | 44 | default: | |
| 48 | 45 | return | |
| @@ -31,7 +31,7 @@ func (s *Screen) SetGeometry(geometry Geometry) { | |||||
| 31 | // reportWindowSize answers the CSI 14 t and CSI 16 t pixel-geometry queries. | 31 | // reportWindowSize answers the CSI 14 t and CSI 16 t pixel-geometry queries. | ||
| 32 | // Unknown pixel geometry is intentionally not fabricated. | 32 | // Unknown pixel geometry is intentionally not fabricated. | ||
| 33 | func (s *Screen) reportWindowSize(parts []int) { | 33 | func (s *Screen) reportWindowSize(parts []int) { | ||
| 34 | - | if s == nil || len(parts) == 0 || !s.geometry.PixelsKnown() { | 34 | + | if s == nil || len(parts) == 0 || !s.geometry.PixelsKnown() || s.geometry.Rows <= 0 || s.geometry.Cols <= 0 { |
| 35 | return | 35 | return | ||
| 36 | } | 36 | } | ||
| 37 | kind := parts[0] | 37 | kind := parts[0] | ||
| @@ -40,9 +40,6 @@ func (s *Screen) reportWindowSize(parts []int) { | |||||
| 40 | case 14: | 40 | case 14: | ||
| 41 | rows, cols = s.geometry.PixelHeight, s.geometry.PixelWidth | 41 | rows, cols = s.geometry.PixelHeight, s.geometry.PixelWidth | ||
| 42 | case 16: | 42 | case 16: | ||
| 43 | - | if s.geometry.Rows <= 0 || s.geometry.Cols <= 0 { | |||
| 44 | - | return | |||
| 45 | - | } | |||
| 46 | rows, cols = s.geometry.PixelHeight/s.geometry.Rows, s.geometry.PixelWidth/s.geometry.Cols | 43 | rows, cols = s.geometry.PixelHeight/s.geometry.Rows, s.geometry.PixelWidth/s.geometry.Cols | ||
| 47 | default: | 44 | default: | ||
| 48 | return | 45 | return | ||
M screen_geometry_test.go +13 -0
| @@ -15,6 +15,7 @@ func TestScreenWindowSizeReports(t *testing.T) { | |||
| 15 | 15 | {name: "unknown pixels", geometry: Geometry{Cols: 80, Rows: 24}, query: "\x1b[14t"}, | |
| 16 | 16 | {name: "partial pixels are unknown", geometry: Geometry{Cols: 80, Rows: 24, PixelWidth: 640}, query: "\x1b[16t"}, | |
| 17 | 17 | {name: "unrelated window query", geometry: Geometry{Cols: 80, Rows: 24, PixelWidth: 640, PixelHeight: 384}, query: "\x1b[18t"}, | |
| 18 | + | {name: "private parameter is ignored", geometry: Geometry{Cols: 80, Rows: 24, PixelWidth: 640, PixelHeight: 384}, query: "\x1b[?14t"}, | |
| 18 | 19 | } | |
| 19 | 20 | for _, tt := range tests { | |
| 20 | 21 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -29,3 +30,15 @@ func TestScreenWindowSizeReports(t *testing.T) { | |||
| 29 | 30 | }) | |
| 30 | 31 | } | |
| 31 | 32 | } | |
| 33 | + | ||
| 34 | + | func TestScreenWindowSizeReportsRequireUsableGeometry(t *testing.T) { | |
| 35 | + | s := NewScreen(80, 24) | |
| 36 | + | s.SetGeometry(Geometry{Cols: 80, Rows: 24, PixelWidth: 640, PixelHeight: 384}) | |
| 37 | + | s.geometry.Cols = 0 | |
| 38 | + | var got string | |
| 39 | + | s.OnResponse = func(response []byte) { got += string(response) } | |
| 40 | + | s.Write([]byte("\x1b[14t")) | |
| 41 | + | if got != "" { | |
| 42 | + | t.Fatalf("response = %q", got) | |
| 43 | + | } | |
| 44 | + | } | |
| @@ -15,6 +15,7 @@ func TestScreenWindowSizeReports(t *testing.T) { | |||||
| 15 | {name: "unknown pixels", geometry: Geometry{Cols: 80, Rows: 24}, query: "\x1b[14t"}, | 15 | {name: "unknown pixels", geometry: Geometry{Cols: 80, Rows: 24}, query: "\x1b[14t"}, | ||
| 16 | {name: "partial pixels are unknown", geometry: Geometry{Cols: 80, Rows: 24, PixelWidth: 640}, query: "\x1b[16t"}, | 16 | {name: "partial pixels are unknown", geometry: Geometry{Cols: 80, Rows: 24, PixelWidth: 640}, query: "\x1b[16t"}, | ||
| 17 | {name: "unrelated window query", geometry: Geometry{Cols: 80, Rows: 24, PixelWidth: 640, PixelHeight: 384}, query: "\x1b[18t"}, | 17 | {name: "unrelated window query", geometry: Geometry{Cols: 80, Rows: 24, PixelWidth: 640, PixelHeight: 384}, query: "\x1b[18t"}, | ||
| 18 | + | {name: "private parameter is ignored", geometry: Geometry{Cols: 80, Rows: 24, PixelWidth: 640, PixelHeight: 384}, query: "\x1b[?14t"}, | |||
| 18 | } | 19 | } | ||
| 19 | for _, tt := range tests { | 20 | for _, tt := range tests { | ||
| 20 | t.Run(tt.name, func(t *testing.T) { | 21 | t.Run(tt.name, func(t *testing.T) { | ||
| @@ -29,3 +30,15 @@ func TestScreenWindowSizeReports(t *testing.T) { | |||||
| 29 | }) | 30 | }) | ||
| 30 | } | 31 | } | ||
| 31 | } | 32 | } | ||
| 33 | + | ||||
| 34 | + | func TestScreenWindowSizeReportsRequireUsableGeometry(t *testing.T) { | |||
| 35 | + | s := NewScreen(80, 24) | |||
| 36 | + | s.SetGeometry(Geometry{Cols: 80, Rows: 24, PixelWidth: 640, PixelHeight: 384}) | |||
| 37 | + | s.geometry.Cols = 0 | |||
| 38 | + | var got string | |||
| 39 | + | s.OnResponse = func(response []byte) { got += string(response) } | |||
| 40 | + | s.Write([]byte("\x1b[14t")) | |||
| 41 | + | if got != "" { | |||
| 42 | + | t.Fatalf("response = %q", got) | |||
| 43 | + | } | |||
| 44 | + | } | |||
M screen_graphics_test.go +13 -12
| @@ -2,6 +2,7 @@ package vt | |||
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | 4 | "encoding/base64" | |
| 5 | + | "strconv" | |
| 5 | 6 | "testing" | |
| 6 | 7 | ||
| 7 | 8 | "github.com/bnema/vev-vt/graphics" | |
| @@ -13,13 +14,13 @@ func screenKittyImageAPC(action string) []byte { | |||
| 13 | 14 | return []byte("\x1b_Ga=" + action + ",i=1,f=32,s=1,v=1,C=1;" + payload + "\x1b\\") | |
| 14 | 15 | } | |
| 15 | 16 | ||
| 16 | - | func screenKittyImageAPCWithID(action string, id byte) []byte { | |
| 17 | + | func screenKittyImageAPCWithID(action string, id uint64) []byte { | |
| 17 | 18 | payload := base64.RawStdEncoding.EncodeToString([]byte{1, 2, 3, 4}) | |
| 18 | - | return []byte("\x1b_Ga=" + action + ",i=" + string(id) + ",f=32,s=1,v=1,C=1;" + payload + "\x1b\\") | |
| 19 | + | return []byte("\x1b_Ga=" + action + ",i=" + strconv.FormatUint(id, 10) + ",f=32,s=1,v=1,C=1;" + payload + "\x1b\\") | |
| 19 | 20 | } | |
| 20 | 21 | ||
| 21 | - | func screenKittyPutAPC(id byte) []byte { | |
| 22 | - | return []byte("\x1b_Ga=p,i=" + string(id) + ";\x1b\\") | |
| 22 | + | func screenKittyPutAPC(id uint64) []byte { | |
| 23 | + | return []byte("\x1b_Ga=p,i=" + strconv.FormatUint(id, 10) + ";\x1b\\") | |
| 23 | 24 | } | |
| 24 | 25 | ||
| 25 | 26 | func TestScreenKittyGraphicsFragmentedAPCAndResponse(t *testing.T) { | |
| @@ -51,7 +52,7 @@ func TestScreenKittyGraphicsPrimaryAndAlternateState(t *testing.T) { | |||
| 51 | 52 | screen.Write([]byte("\x1b[?1049halt")) | |
| 52 | 53 | require.True(t, screen.AltScreenActive()) | |
| 53 | 54 | require.Nil(t, screen.GraphicsSnapshot()) | |
| 54 | - | screen.Write(screenKittyImageAPCWithID("T", '2')) | |
| 55 | + | screen.Write(screenKittyImageAPCWithID("T", 2)) | |
| 55 | 56 | require.Equal(t, uint64(1), screen.GraphicsSnapshot().Usage().Placements) | |
| 56 | 57 | ||
| 57 | 58 | screen.Write([]byte("\x1b[?1049l")) | |
| @@ -59,7 +60,7 @@ func TestScreenKittyGraphicsPrimaryAndAlternateState(t *testing.T) { | |||
| 59 | 60 | require.Equal(t, primary.Usage(), screen.GraphicsSnapshot().Usage()) | |
| 60 | 61 | } | |
| 61 | 62 | ||
| 62 | - | func TestScreenReenteringActiveAlternatePreservesGraphicsState(t *testing.T) { | |
| 63 | + | func TestScreenReenteringActiveAlternateClearsGraphicsState(t *testing.T) { | |
| 63 | 64 | screen := NewScreen(8, 3) | |
| 64 | 65 | screen.Write([]byte("\x1b[?1049h")) | |
| 65 | 66 | screen.Write(screenKittyImageAPC("T")) | |
| @@ -69,8 +70,8 @@ func TestScreenReenteringActiveAlternatePreservesGraphicsState(t *testing.T) { | |||
| 69 | 70 | ||
| 70 | 71 | screen.Write([]byte("\x1b[?1049h")) | |
| 71 | 72 | after := screen.GraphicsSnapshot() | |
| 72 | - | require.NotNil(t, after) | |
| 73 | - | require.Equal(t, before.Usage(), after.Usage()) | |
| 73 | + | require.Nil(t, after) | |
| 74 | + | require.Equal(t, uint64(1), before.Usage().Placements) | |
| 74 | 75 | } | |
| 75 | 76 | ||
| 76 | 77 | func TestScreenFullClearAbortsPendingUploadAndClearsPlacements(t *testing.T) { | |
| @@ -100,7 +101,7 @@ func TestScreenPendingTransmitDisplayFailsClosedAcrossErrorAndResize(t *testing. | |||
| 100 | 101 | require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements) | |
| 101 | 102 | ||
| 102 | 103 | screen.Write(pending) | |
| 103 | - | screen.Resize(4, 2) | |
| 104 | + | screen.Resize(5, 2) | |
| 104 | 105 | screen.Write(continuation) | |
| 105 | 106 | require.NotNil(t, screen.GraphicsSnapshot()) | |
| 106 | 107 | require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements) | |
| @@ -158,7 +159,7 @@ func TestScreenKittyGraphicsOrdinaryResetClearScrollAndResize(t *testing.T) { | |||
| 158 | 159 | require.Equal(t, uint64(1), graphicsSnapshot.Usage().Assets) | |
| 159 | 160 | require.Equal(t, uint64(0), graphicsSnapshot.Usage().Placements) | |
| 160 | 161 | ||
| 161 | - | screen.Write(screenKittyPutAPC('1')) | |
| 162 | + | screen.Write(screenKittyPutAPC(1)) | |
| 162 | 163 | require.Equal(t, uint64(1), screen.GraphicsSnapshot().Usage().Placements) | |
| 163 | 164 | screen.Write([]byte("\x1bc")) | |
| 164 | 165 | require.Nil(t, screen.GraphicsSnapshot()) | |
| @@ -170,7 +171,7 @@ func TestScreenSnapshotCapturesOwnedActiveGraphicsSnapshot(t *testing.T) { | |||
| 170 | 171 | captured := screen.Snapshot() | |
| 171 | 172 | require.NotNil(t, captured.Graphics()) | |
| 172 | 173 | require.Equal(t, uint64(1), captured.Graphics().Usage().Placements) | |
| 173 | - | require.Equal(t, captured.Graphics().Usage(), screen.CaptureGraphicsSnapshot().Usage(), "capture returns the immutable active-scene reference") | |
| 174 | + | require.Equal(t, captured.Graphics().Usage(), screen.GraphicsSnapshot().Usage(), "screen graphics snapshot returns the immutable active-scene reference") | |
| 174 | 175 | ||
| 175 | 176 | screen.Write([]byte("\x1b[2J")) | |
| 176 | 177 | require.Equal(t, uint64(1), captured.Graphics().Usage().Placements) | |
| @@ -185,7 +186,7 @@ func TestScreenOrdinaryTextPathDoesNotAllocateGraphicsState(t *testing.T) { | |||
| 185 | 186 | screen := NewScreen(4, 2) | |
| 186 | 187 | screen.Write([]byte("text")) | |
| 187 | 188 | require.Nil(t, screen.GraphicsSnapshot()) | |
| 188 | - | require.Nil(t, screen.Snapshot().GraphicsSnapshot()) | |
| 189 | + | require.Nil(t, screen.Snapshot().Graphics()) | |
| 189 | 190 | require.Equal(t, "text", rowText(screen.Snapshot().Row(0))) | |
| 190 | 191 | } | |
| 191 | 192 | ||
| @@ -2,6 +2,7 @@ package vt | |||||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | "encoding/base64" | 4 | "encoding/base64" | ||
| 5 | + | "strconv" | |||
| 5 | "testing" | 6 | "testing" | ||
| 6 | 7 | ||||
| 7 | "github.com/bnema/vev-vt/graphics" | 8 | "github.com/bnema/vev-vt/graphics" | ||
| @@ -13,13 +14,13 @@ func screenKittyImageAPC(action string) []byte { | |||||
| 13 | return []byte("\x1b_Ga=" + action + ",i=1,f=32,s=1,v=1,C=1;" + payload + "\x1b\\") | 14 | return []byte("\x1b_Ga=" + action + ",i=1,f=32,s=1,v=1,C=1;" + payload + "\x1b\\") | ||
| 14 | } | 15 | } | ||
| 15 | 16 | ||||
| 16 | - | func screenKittyImageAPCWithID(action string, id byte) []byte { | 17 | + | func screenKittyImageAPCWithID(action string, id uint64) []byte { |
| 17 | payload := base64.RawStdEncoding.EncodeToString([]byte{1, 2, 3, 4}) | 18 | payload := base64.RawStdEncoding.EncodeToString([]byte{1, 2, 3, 4}) | ||
| 18 | - | return []byte("\x1b_Ga=" + action + ",i=" + string(id) + ",f=32,s=1,v=1,C=1;" + payload + "\x1b\\") | 19 | + | return []byte("\x1b_Ga=" + action + ",i=" + strconv.FormatUint(id, 10) + ",f=32,s=1,v=1,C=1;" + payload + "\x1b\\") |
| 19 | } | 20 | } | ||
| 20 | 21 | ||||
| 21 | - | func screenKittyPutAPC(id byte) []byte { | 22 | + | func screenKittyPutAPC(id uint64) []byte { |
| 22 | - | return []byte("\x1b_Ga=p,i=" + string(id) + ";\x1b\\") | 23 | + | return []byte("\x1b_Ga=p,i=" + strconv.FormatUint(id, 10) + ";\x1b\\") |
| 23 | } | 24 | } | ||
| 24 | 25 | ||||
| 25 | func TestScreenKittyGraphicsFragmentedAPCAndResponse(t *testing.T) { | 26 | func TestScreenKittyGraphicsFragmentedAPCAndResponse(t *testing.T) { | ||
| @@ -51,7 +52,7 @@ func TestScreenKittyGraphicsPrimaryAndAlternateState(t *testing.T) { | |||||
| 51 | screen.Write([]byte("\x1b[?1049halt")) | 52 | screen.Write([]byte("\x1b[?1049halt")) | ||
| 52 | require.True(t, screen.AltScreenActive()) | 53 | require.True(t, screen.AltScreenActive()) | ||
| 53 | require.Nil(t, screen.GraphicsSnapshot()) | 54 | require.Nil(t, screen.GraphicsSnapshot()) | ||
| 54 | - | screen.Write(screenKittyImageAPCWithID("T", '2')) | 55 | + | screen.Write(screenKittyImageAPCWithID("T", 2)) |
| 55 | require.Equal(t, uint64(1), screen.GraphicsSnapshot().Usage().Placements) | 56 | require.Equal(t, uint64(1), screen.GraphicsSnapshot().Usage().Placements) | ||
| 56 | 57 | ||||
| 57 | screen.Write([]byte("\x1b[?1049l")) | 58 | screen.Write([]byte("\x1b[?1049l")) | ||
| @@ -59,7 +60,7 @@ func TestScreenKittyGraphicsPrimaryAndAlternateState(t *testing.T) { | |||||
| 59 | require.Equal(t, primary.Usage(), screen.GraphicsSnapshot().Usage()) | 60 | require.Equal(t, primary.Usage(), screen.GraphicsSnapshot().Usage()) | ||
| 60 | } | 61 | } | ||
| 61 | 62 | ||||
| 62 | - | func TestScreenReenteringActiveAlternatePreservesGraphicsState(t *testing.T) { | 63 | + | func TestScreenReenteringActiveAlternateClearsGraphicsState(t *testing.T) { |
| 63 | screen := NewScreen(8, 3) | 64 | screen := NewScreen(8, 3) | ||
| 64 | screen.Write([]byte("\x1b[?1049h")) | 65 | screen.Write([]byte("\x1b[?1049h")) | ||
| 65 | screen.Write(screenKittyImageAPC("T")) | 66 | screen.Write(screenKittyImageAPC("T")) | ||
| @@ -69,8 +70,8 @@ func TestScreenReenteringActiveAlternatePreservesGraphicsState(t *testing.T) { | |||||
| 69 | 70 | ||||
| 70 | screen.Write([]byte("\x1b[?1049h")) | 71 | screen.Write([]byte("\x1b[?1049h")) | ||
| 71 | after := screen.GraphicsSnapshot() | 72 | after := screen.GraphicsSnapshot() | ||
| 72 | - | require.NotNil(t, after) | 73 | + | require.Nil(t, after) |
| 73 | - | require.Equal(t, before.Usage(), after.Usage()) | 74 | + | require.Equal(t, uint64(1), before.Usage().Placements) |
| 74 | } | 75 | } | ||
| 75 | 76 | ||||
| 76 | func TestScreenFullClearAbortsPendingUploadAndClearsPlacements(t *testing.T) { | 77 | func TestScreenFullClearAbortsPendingUploadAndClearsPlacements(t *testing.T) { | ||
| @@ -100,7 +101,7 @@ func TestScreenPendingTransmitDisplayFailsClosedAcrossErrorAndResize(t *testing. | |||||
| 100 | require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements) | 101 | require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements) | ||
| 101 | 102 | ||||
| 102 | screen.Write(pending) | 103 | screen.Write(pending) | ||
| 103 | - | screen.Resize(4, 2) | 104 | + | screen.Resize(5, 2) |
| 104 | screen.Write(continuation) | 105 | screen.Write(continuation) | ||
| 105 | require.NotNil(t, screen.GraphicsSnapshot()) | 106 | require.NotNil(t, screen.GraphicsSnapshot()) | ||
| 106 | require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements) | 107 | require.Equal(t, uint64(0), screen.GraphicsSnapshot().Usage().Placements) | ||
| @@ -158,7 +159,7 @@ func TestScreenKittyGraphicsOrdinaryResetClearScrollAndResize(t *testing.T) { | |||||
| 158 | require.Equal(t, uint64(1), graphicsSnapshot.Usage().Assets) | 159 | require.Equal(t, uint64(1), graphicsSnapshot.Usage().Assets) | ||
| 159 | require.Equal(t, uint64(0), graphicsSnapshot.Usage().Placements) | 160 | require.Equal(t, uint64(0), graphicsSnapshot.Usage().Placements) | ||
| 160 | 161 | ||||
| 161 | - | screen.Write(screenKittyPutAPC('1')) | 162 | + | screen.Write(screenKittyPutAPC(1)) |
| 162 | require.Equal(t, uint64(1), screen.GraphicsSnapshot().Usage().Placements) | 163 | require.Equal(t, uint64(1), screen.GraphicsSnapshot().Usage().Placements) | ||
| 163 | screen.Write([]byte("\x1bc")) | 164 | screen.Write([]byte("\x1bc")) | ||
| 164 | require.Nil(t, screen.GraphicsSnapshot()) | 165 | require.Nil(t, screen.GraphicsSnapshot()) | ||
| @@ -170,7 +171,7 @@ func TestScreenSnapshotCapturesOwnedActiveGraphicsSnapshot(t *testing.T) { | |||||
| 170 | captured := screen.Snapshot() | 171 | captured := screen.Snapshot() | ||
| 171 | require.NotNil(t, captured.Graphics()) | 172 | require.NotNil(t, captured.Graphics()) | ||
| 172 | require.Equal(t, uint64(1), captured.Graphics().Usage().Placements) | 173 | require.Equal(t, uint64(1), captured.Graphics().Usage().Placements) | ||
| 173 | - | require.Equal(t, captured.Graphics().Usage(), screen.CaptureGraphicsSnapshot().Usage(), "capture returns the immutable active-scene reference") | 174 | + | require.Equal(t, captured.Graphics().Usage(), screen.GraphicsSnapshot().Usage(), "screen graphics snapshot returns the immutable active-scene reference") |
| 174 | 175 | ||||
| 175 | screen.Write([]byte("\x1b[2J")) | 176 | screen.Write([]byte("\x1b[2J")) | ||
| 176 | require.Equal(t, uint64(1), captured.Graphics().Usage().Placements) | 177 | require.Equal(t, uint64(1), captured.Graphics().Usage().Placements) | ||
| @@ -185,7 +186,7 @@ func TestScreenOrdinaryTextPathDoesNotAllocateGraphicsState(t *testing.T) { | |||||
| 185 | screen := NewScreen(4, 2) | 186 | screen := NewScreen(4, 2) | ||
| 186 | screen.Write([]byte("text")) | 187 | screen.Write([]byte("text")) | ||
| 187 | require.Nil(t, screen.GraphicsSnapshot()) | 188 | require.Nil(t, screen.GraphicsSnapshot()) | ||
| 188 | - | require.Nil(t, screen.Snapshot().GraphicsSnapshot()) | 189 | + | require.Nil(t, screen.Snapshot().Graphics()) |
| 189 | require.Equal(t, "text", rowText(screen.Snapshot().Row(0))) | 190 | require.Equal(t, "text", rowText(screen.Snapshot().Row(0))) | ||
| 190 | } | 191 | } | ||
| 191 | 192 | ||||
M screen_modes.go +15 -16
| @@ -198,22 +198,21 @@ func (s *Screen) setMode(private bool, parts []int, enabled bool) { | |||
| 198 | 198 | } | |
| 199 | 199 | ||
| 200 | 200 | func (s *Screen) enterAlternateScreen() { | |
| 201 | - | if s.alternate != nil { | |
| 202 | - | return | |
| 203 | - | } | |
| 204 | - | s.alternate = &screenState{ | |
| 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, | |
| 201 | + | if s.alternate == nil { | |
| 202 | + | s.alternate = &screenState{ | |
| 203 | + | frame: cloneFrame(s.Frame), | |
| 204 | + | buffer: s.buffer.clone(), | |
| 205 | + | row: s.Row, | |
| 206 | + | col: s.Col, | |
| 207 | + | style: s.Style, | |
| 208 | + | scrollTop: s.scrollTop, | |
| 209 | + | scrollBottom: s.scrollBottom, | |
| 210 | + | savedCursor: s.savedCursor, | |
| 211 | + | graphics: s.graphics, | |
| 212 | + | kittyPendingDisplay: s.kittyPendingDisplay, | |
| 213 | + | originMode: s.originMode, | |
| 214 | + | insertMode: s.insertMode, | |
| 215 | + | } | |
| 217 | 216 | } | |
| 218 | 217 | s.graphics = nil | |
| 219 | 218 | s.kittyPendingDisplay = nil | |
| @@ -198,22 +198,21 @@ func (s *Screen) setMode(private bool, parts []int, enabled bool) { | |||||
| 198 | } | 198 | } | ||
| 199 | 199 | ||||
| 200 | func (s *Screen) enterAlternateScreen() { | 200 | func (s *Screen) enterAlternateScreen() { | ||
| 201 | - | if s.alternate != nil { | 201 | + | if s.alternate == nil { |
| 202 | - | return | 202 | + | s.alternate = &screenState{ |
| 203 | - | } | 203 | + | frame: cloneFrame(s.Frame), |
| 204 | - | s.alternate = &screenState{ | 204 | + | buffer: s.buffer.clone(), |
| 205 | - | frame: cloneFrame(s.Frame), | 205 | + | row: s.Row, |
| 206 | - | buffer: s.buffer.clone(), | 206 | + | col: s.Col, |
| 207 | - | row: s.Row, | 207 | + | style: s.Style, |
| 208 | - | col: s.Col, | 208 | + | scrollTop: s.scrollTop, |
| 209 | - | style: s.Style, | 209 | + | scrollBottom: s.scrollBottom, |
| 210 | - | scrollTop: s.scrollTop, | 210 | + | savedCursor: s.savedCursor, |
| 211 | - | scrollBottom: s.scrollBottom, | 211 | + | graphics: s.graphics, |
| 212 | - | savedCursor: s.savedCursor, | 212 | + | kittyPendingDisplay: s.kittyPendingDisplay, |
| 213 | - | graphics: s.graphics, | 213 | + | originMode: s.originMode, |
| 214 | - | kittyPendingDisplay: s.kittyPendingDisplay, | 214 | + | insertMode: s.insertMode, |
| 215 | - | originMode: s.originMode, | 215 | + | } |
| 216 | - | insertMode: s.insertMode, | |||
| 217 | } | 216 | } | ||
| 218 | s.graphics = nil | 217 | s.graphics = nil | ||
| 219 | s.kittyPendingDisplay = nil | 218 | s.kittyPendingDisplay = nil | ||
M screen_resize.go +0 -4
| @@ -5,10 +5,6 @@ import renderer "github.com/bnema/vev-vt/core" | |||
| 5 | 5 | func (s *Screen) Resize(width, height int) { | |
| 6 | 6 | if width == s.Frame.Width && height == s.Frame.Height { | |
| 7 | 7 | 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() | |
| 12 | 8 | return | |
| 13 | 9 | } | |
| 14 | 10 | if width <= 0 || height <= 0 { | |
| @@ -5,10 +5,6 @@ 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, height | 7 | 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() | |||
| 12 | return | 8 | return | ||
| 13 | } | 9 | } | ||
| 14 | if width <= 0 || height <= 0 { | 10 | if width <= 0 || height <= 0 { | ||
M screen_snapshot.go +0 -3
| @@ -123,6 +123,3 @@ func (s ScreenSnapshot) Title() string { return s.title } | |||
| 123 | 123 | // Graphics returns the immutable graphics scene snapshot for the active | |
| 124 | 124 | // screen buffer, or nil when that buffer has not used Kitty graphics. | |
| 125 | 125 | func (s ScreenSnapshot) Graphics() *graphics.Snapshot { return s.graphics } | |
| 126 | - | ||
| 127 | - | // GraphicsSnapshot is an explicit alias for Graphics. | |
| 128 | - | func (s ScreenSnapshot) GraphicsSnapshot() *graphics.Snapshot { return s.graphics } | |
| @@ -123,6 +123,3 @@ func (s ScreenSnapshot) Title() string { return s.title } | |||||
| 123 | // Graphics returns the immutable graphics scene snapshot for the active | 123 | // Graphics returns the immutable graphics scene snapshot for the active | ||
| 124 | // screen buffer, or nil when that buffer has not used Kitty graphics. | 124 | // screen buffer, or nil when that buffer has not used Kitty graphics. | ||
| 125 | func (s ScreenSnapshot) Graphics() *graphics.Snapshot { return s.graphics } | 125 | func (s ScreenSnapshot) Graphics() *graphics.Snapshot { return s.graphics } | ||
| 126 | - | ||||
| 127 | - | // GraphicsSnapshot is an explicit alias for Graphics. | |||
| 128 | - | func (s ScreenSnapshot) GraphicsSnapshot() *graphics.Snapshot { return s.graphics } | |||