merge(perf): screen eviction scratch, kitty payload ownership and escape buffers
10 files · +411 -39
M graphics/graphics.go +2 -1
| @@ -111,7 +111,8 @@ const ( | |||
| 111 | 111 | // scene or any snapshot. Format preserves the encoded pixel layout for the | |
| 112 | 112 | // renderer. Width and Height describe the decoded pixel extent used for | |
| 113 | 113 | // clipping and quota accounting. DecodedPixels, when non-zero, must equal | |
| 114 | - | // Width*Height; accounting always uses the checked product. | |
| 114 | + | // Width*Height; accounting always uses the checked product. Scene.AddAssetOwned | |
| 115 | + | // and ReplaceAssetOwned instead take ownership of Encoded without copying it. | |
| 115 | 116 | type AssetBlob struct { | |
| 116 | 117 | Encoded []byte | |
| 117 | 118 | Format AssetFormat | |
| @@ -111,7 +111,8 @@ const ( | |||||
| 111 | // scene or any snapshot. Format preserves the encoded pixel layout for the | 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 | 112 | // renderer. Width and Height describe the decoded pixel extent used for | ||
| 113 | // clipping and quota accounting. DecodedPixels, when non-zero, must equal | 113 | // clipping and quota accounting. DecodedPixels, when non-zero, must equal | ||
| 114 | - | // Width*Height; accounting always uses the checked product. | 114 | + | // Width*Height; accounting always uses the checked product. Scene.AddAssetOwned |
| 115 | + | // and ReplaceAssetOwned instead take ownership of Encoded without copying it. | |||
| 115 | type AssetBlob struct { | 116 | type AssetBlob struct { | ||
| 116 | Encoded []byte | 117 | Encoded []byte | ||
| 117 | Format AssetFormat | 118 | Format AssetFormat | ||
M graphics/scene.go +30 -5
| @@ -101,13 +101,25 @@ func (s *Scene) Limits() Limits { | |||
| 101 | 101 | ||
| 102 | 102 | // AddAsset copies and registers an encoded asset, returning an opaque ID. | |
| 103 | 103 | func (s *Scene) AddAsset(blob AssetBlob) (AssetID, error) { | |
| 104 | + | return s.addAsset(blob, true) | |
| 105 | + | } | |
| 106 | + | ||
| 107 | + | // AddAssetOwned is AddAsset for callers that hand over blob.Encoded: the scene | |
| 108 | + | // keeps the slice without copying, so the caller must not read or modify it | |
| 109 | + | // afterwards. Snapshots and AssetView still return copies. On error the scene | |
| 110 | + | // does not retain blob.Encoded, so the caller keeps ownership of it. | |
| 111 | + | func (s *Scene) AddAssetOwned(blob AssetBlob) (AssetID, error) { | |
| 112 | + | return s.addAsset(blob, false) | |
| 113 | + | } | |
| 114 | + | ||
| 115 | + | func (s *Scene) addAsset(blob AssetBlob, copyEncoded bool) (AssetID, error) { | |
| 104 | 116 | if s == nil { | |
| 105 | 117 | return AssetID{}, fmt.Errorf("add asset: %w", ErrInvalidAsset) | |
| 106 | 118 | } | |
| 107 | 119 | s.mu.Lock() | |
| 108 | 120 | defer s.mu.Unlock() | |
| 109 | 121 | candidate := cloneState(s.state) | |
| 110 | - | record, err := s.prepareAsset(blob, candidate) | |
| 122 | + | record, err := s.prepareAsset(blob, candidate, copyEncoded) | |
| 111 | 123 | if err != nil { | |
| 112 | 124 | return AssetID{}, err | |
| 113 | 125 | } | |
| @@ -134,6 +146,16 @@ func (s *Scene) RegisterAsset(blob AssetBlob) (AssetID, error) { return s.AddAss | |||
| 134 | 146 | // reference it. If validation or resource limits reject the new asset, the | |
| 135 | 147 | // existing asset and placements remain unchanged. | |
| 136 | 148 | func (s *Scene) ReplaceAsset(id AssetID, blob AssetBlob) (AssetID, error) { | |
| 149 | + | return s.replaceAsset(id, blob, true) | |
| 150 | + | } | |
| 151 | + | ||
| 152 | + | // ReplaceAssetOwned is ReplaceAsset for callers that hand over blob.Encoded; | |
| 153 | + | // see AddAssetOwned. On error blob.Encoded is not retained. | |
| 154 | + | func (s *Scene) ReplaceAssetOwned(id AssetID, blob AssetBlob) (AssetID, error) { | |
| 155 | + | return s.replaceAsset(id, blob, false) | |
| 156 | + | } | |
| 157 | + | ||
| 158 | + | func (s *Scene) replaceAsset(id AssetID, blob AssetBlob, copyEncoded bool) (AssetID, error) { | |
| 137 | 159 | if s == nil { | |
| 138 | 160 | return AssetID{}, fmt.Errorf("replace asset: %w", ErrAssetNotFound) | |
| 139 | 161 | } | |
| @@ -155,7 +177,7 @@ func (s *Scene) ReplaceAsset(id AssetID, blob AssetBlob) (AssetID, error) { | |||
| 155 | 177 | candidate.usage.Assets-- | |
| 156 | 178 | candidate.usage.EncodedBytes -= uint64(len(old.encoded)) | |
| 157 | 179 | candidate.usage.DecodedPixels -= old.pixels | |
| 158 | - | replacement, err := s.prepareAsset(blob, candidate) | |
| 180 | + | replacement, err := s.prepareAsset(blob, candidate, copyEncoded) | |
| 159 | 181 | if err != nil { | |
| 160 | 182 | return AssetID{}, err | |
| 161 | 183 | } | |
| @@ -389,7 +411,7 @@ func (s *Scene) Usage() Usage { | |||
| 389 | 411 | return s.state.usage | |
| 390 | 412 | } | |
| 391 | 413 | ||
| 392 | - | func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState) (assetRecord, error) { | |
| 414 | + | func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState, copyEncoded bool) (assetRecord, error) { | |
| 393 | 415 | data := blob.Encoded | |
| 394 | 416 | if blob.Width <= 0 || blob.Height <= 0 { | |
| 395 | 417 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrInvalidAsset) | |
| @@ -414,7 +436,10 @@ func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState) (assetRecord, er | |||
| 414 | 436 | if !within(state.usage.DecodedPixels, pixels, s.limits.MaxDecodedPixels) { | |
| 415 | 437 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrDecodedPixelBudget) | |
| 416 | 438 | } | |
| 417 | - | return assetRecord{encoded: append([]byte(nil), data...), format: blob.Format, width: blob.Width, height: blob.Height, pixels: pixels}, nil | |
| 439 | + | if copyEncoded { | |
| 440 | + | data = append([]byte(nil), data...) | |
| 441 | + | } | |
| 442 | + | return assetRecord{encoded: data, format: blob.Format, width: blob.Width, height: blob.Height, pixels: pixels}, nil | |
| 418 | 443 | } | |
| 419 | 444 | ||
| 420 | 445 | func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing *placementRecord) (placementRecord, error) { | |
| @@ -495,7 +520,7 @@ func removeAsset(state *sceneState, id AssetID, cascade bool) error { | |||
| 495 | 520 | func (s *Scene) applyOperation(state *sceneState, operation Operation) error { | |
| 496 | 521 | switch operation.Kind { | |
| 497 | 522 | case OperationAddAsset: | |
| 498 | - | record, err := s.prepareAsset(operation.Blob, state) | |
| 523 | + | record, err := s.prepareAsset(operation.Blob, state, true) | |
| 499 | 524 | if err != nil { | |
| 500 | 525 | return err | |
| 501 | 526 | } | |
| @@ -101,13 +101,25 @@ func (s *Scene) Limits() Limits { | |||||
| 101 | 101 | ||||
| 102 | // AddAsset copies and registers an encoded asset, returning an opaque ID. | 102 | // AddAsset copies and registers an encoded asset, returning an opaque ID. | ||
| 103 | func (s *Scene) AddAsset(blob AssetBlob) (AssetID, error) { | 103 | func (s *Scene) AddAsset(blob AssetBlob) (AssetID, error) { | ||
| 104 | + | return s.addAsset(blob, true) | |||
| 105 | + | } | |||
| 106 | + | ||||
| 107 | + | // AddAssetOwned is AddAsset for callers that hand over blob.Encoded: the scene | |||
| 108 | + | // keeps the slice without copying, so the caller must not read or modify it | |||
| 109 | + | // afterwards. Snapshots and AssetView still return copies. On error the scene | |||
| 110 | + | // does not retain blob.Encoded, so the caller keeps ownership of it. | |||
| 111 | + | func (s *Scene) AddAssetOwned(blob AssetBlob) (AssetID, error) { | |||
| 112 | + | return s.addAsset(blob, false) | |||
| 113 | + | } | |||
| 114 | + | ||||
| 115 | + | func (s *Scene) addAsset(blob AssetBlob, copyEncoded bool) (AssetID, error) { | |||
| 104 | if s == nil { | 116 | if s == nil { | ||
| 105 | return AssetID{}, fmt.Errorf("add asset: %w", ErrInvalidAsset) | 117 | return AssetID{}, fmt.Errorf("add asset: %w", ErrInvalidAsset) | ||
| 106 | } | 118 | } | ||
| 107 | s.mu.Lock() | 119 | s.mu.Lock() | ||
| 108 | defer s.mu.Unlock() | 120 | defer s.mu.Unlock() | ||
| 109 | candidate := cloneState(s.state) | 121 | candidate := cloneState(s.state) | ||
| 110 | - | record, err := s.prepareAsset(blob, candidate) | 122 | + | record, err := s.prepareAsset(blob, candidate, copyEncoded) |
| 111 | if err != nil { | 123 | if err != nil { | ||
| 112 | return AssetID{}, err | 124 | return AssetID{}, err | ||
| 113 | } | 125 | } | ||
| @@ -134,6 +146,16 @@ func (s *Scene) RegisterAsset(blob AssetBlob) (AssetID, error) { return s.AddAss | |||||
| 134 | // reference it. If validation or resource limits reject the new asset, the | 146 | // reference it. If validation or resource limits reject the new asset, the | ||
| 135 | // existing asset and placements remain unchanged. | 147 | // existing asset and placements remain unchanged. | ||
| 136 | func (s *Scene) ReplaceAsset(id AssetID, blob AssetBlob) (AssetID, error) { | 148 | func (s *Scene) ReplaceAsset(id AssetID, blob AssetBlob) (AssetID, error) { | ||
| 149 | + | return s.replaceAsset(id, blob, true) | |||
| 150 | + | } | |||
| 151 | + | ||||
| 152 | + | // ReplaceAssetOwned is ReplaceAsset for callers that hand over blob.Encoded; | |||
| 153 | + | // see AddAssetOwned. On error blob.Encoded is not retained. | |||
| 154 | + | func (s *Scene) ReplaceAssetOwned(id AssetID, blob AssetBlob) (AssetID, error) { | |||
| 155 | + | return s.replaceAsset(id, blob, false) | |||
| 156 | + | } | |||
| 157 | + | ||||
| 158 | + | func (s *Scene) replaceAsset(id AssetID, blob AssetBlob, copyEncoded bool) (AssetID, error) { | |||
| 137 | if s == nil { | 159 | if s == nil { | ||
| 138 | return AssetID{}, fmt.Errorf("replace asset: %w", ErrAssetNotFound) | 160 | return AssetID{}, fmt.Errorf("replace asset: %w", ErrAssetNotFound) | ||
| 139 | } | 161 | } | ||
| @@ -155,7 +177,7 @@ func (s *Scene) ReplaceAsset(id AssetID, blob AssetBlob) (AssetID, error) { | |||||
| 155 | candidate.usage.Assets-- | 177 | candidate.usage.Assets-- | ||
| 156 | candidate.usage.EncodedBytes -= uint64(len(old.encoded)) | 178 | candidate.usage.EncodedBytes -= uint64(len(old.encoded)) | ||
| 157 | candidate.usage.DecodedPixels -= old.pixels | 179 | candidate.usage.DecodedPixels -= old.pixels | ||
| 158 | - | replacement, err := s.prepareAsset(blob, candidate) | 180 | + | replacement, err := s.prepareAsset(blob, candidate, copyEncoded) |
| 159 | if err != nil { | 181 | if err != nil { | ||
| 160 | return AssetID{}, err | 182 | return AssetID{}, err | ||
| 161 | } | 183 | } | ||
| @@ -389,7 +411,7 @@ func (s *Scene) Usage() Usage { | |||||
| 389 | return s.state.usage | 411 | return s.state.usage | ||
| 390 | } | 412 | } | ||
| 391 | 413 | ||||
| 392 | - | func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState) (assetRecord, error) { | 414 | + | func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState, copyEncoded bool) (assetRecord, error) { |
| 393 | data := blob.Encoded | 415 | data := blob.Encoded | ||
| 394 | if blob.Width <= 0 || blob.Height <= 0 { | 416 | if blob.Width <= 0 || blob.Height <= 0 { | ||
| 395 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrInvalidAsset) | 417 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrInvalidAsset) | ||
| @@ -414,7 +436,10 @@ func (s *Scene) prepareAsset(blob AssetBlob, state *sceneState) (assetRecord, er | |||||
| 414 | if !within(state.usage.DecodedPixels, pixels, s.limits.MaxDecodedPixels) { | 436 | if !within(state.usage.DecodedPixels, pixels, s.limits.MaxDecodedPixels) { | ||
| 415 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrDecodedPixelBudget) | 437 | return assetRecord{}, fmt.Errorf("add asset: %w", ErrDecodedPixelBudget) | ||
| 416 | } | 438 | } | ||
| 417 | - | return assetRecord{encoded: append([]byte(nil), data...), format: blob.Format, width: blob.Width, height: blob.Height, pixels: pixels}, nil | 439 | + | if copyEncoded { |
| 440 | + | data = append([]byte(nil), data...) | |||
| 441 | + | } | |||
| 442 | + | return assetRecord{encoded: data, format: blob.Format, width: blob.Width, height: blob.Height, pixels: pixels}, nil | |||
| 418 | } | 443 | } | ||
| 419 | 444 | ||||
| 420 | func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing *placementRecord) (placementRecord, error) { | 445 | func (s *Scene) preparePlacement(state *sceneState, spec PlacementSpec, existing *placementRecord) (placementRecord, error) { | ||
| @@ -495,7 +520,7 @@ func removeAsset(state *sceneState, id AssetID, cascade bool) error { | |||||
| 495 | func (s *Scene) applyOperation(state *sceneState, operation Operation) error { | 520 | func (s *Scene) applyOperation(state *sceneState, operation Operation) error { | ||
| 496 | switch operation.Kind { | 521 | switch operation.Kind { | ||
| 497 | case OperationAddAsset: | 522 | case OperationAddAsset: | ||
| 498 | - | record, err := s.prepareAsset(operation.Blob, state) | 523 | + | record, err := s.prepareAsset(operation.Blob, state, true) |
| 499 | if err != nil { | 524 | if err != nil { | ||
| 500 | return err | 525 | return err | ||
| 501 | } | 526 | } | ||
M protocol/kittygraphics/base64.go +18 -5
| @@ -1,6 +1,7 @@ | |||
| 1 | 1 | package kittygraphics | |
| 2 | 2 | ||
| 3 | 3 | import ( | |
| 4 | + | "bytes" | |
| 4 | 5 | "encoding/base64" | |
| 5 | 6 | "fmt" | |
| 6 | 7 | ) | |
| @@ -8,14 +9,26 @@ import ( | |||
| 8 | 9 | // DecodeBase64 accepts both padded standard base64 and unpadded raw standard | |
| 9 | 10 | // base64. It deliberately does not accept URL-safe alphabets or non-base64 | |
| 10 | 11 | // whitespace other than the CR/LF ignored by the standard encoding. | |
| 12 | + | // | |
| 13 | + | // The returned slice is freshly allocated and owned by the caller. | |
| 11 | 14 | func DecodeBase64(encoded []byte) ([]byte, error) { | |
| 12 | - | decoded, err := base64.StdEncoding.DecodeString(string(encoded)) | |
| 15 | + | // RawStdEncoding rejects '=' and otherwise accepts every input | |
| 16 | + | // StdEncoding does (padded input has no '=' only when its length is a | |
| 17 | + | // multiple of four, where both encodings agree), so the alphabet used is | |
| 18 | + | // chosen by looking for padding rather than by decoding twice. | |
| 19 | + | enc := base64.RawStdEncoding | |
| 20 | + | if bytes.IndexByte(encoded, '=') >= 0 { | |
| 21 | + | enc = base64.StdEncoding | |
| 22 | + | } | |
| 23 | + | decoded := make([]byte, enc.DecodedLen(len(encoded))) | |
| 24 | + | n, err := enc.Decode(decoded, encoded) | |
| 13 | 25 | if err == nil { | |
| 14 | - | return decoded, nil | |
| 26 | + | return decoded[:n], nil | |
| 15 | 27 | } | |
| 16 | - | decoded, rawErr := base64.RawStdEncoding.DecodeString(string(encoded)) | |
| 17 | - | if rawErr == nil { | |
| 18 | - | return decoded, nil | |
| 28 | + | if enc == base64.RawStdEncoding { | |
| 29 | + | // Report the StdEncoding error, as the padded decoder is the | |
| 30 | + | // documented primary format. This only runs for rejected input. | |
| 31 | + | _, err = base64.StdEncoding.Decode(make([]byte, base64.StdEncoding.DecodedLen(len(encoded))), encoded) | |
| 19 | 32 | } | |
| 20 | 33 | return nil, fmt.Errorf("%w: %v", ErrInvalidBase64, err) | |
| 21 | 34 | } | |
| @@ -1,6 +1,7 @@ | |||||
| 1 | package kittygraphics | 1 | package kittygraphics | ||
| 2 | 2 | ||||
| 3 | import ( | 3 | import ( | ||
| 4 | + | "bytes" | |||
| 4 | "encoding/base64" | 5 | "encoding/base64" | ||
| 5 | "fmt" | 6 | "fmt" | ||
| 6 | ) | 7 | ) | ||
| @@ -8,14 +9,26 @@ import ( | |||||
| 8 | // DecodeBase64 accepts both padded standard base64 and unpadded raw standard | 9 | // DecodeBase64 accepts both padded standard base64 and unpadded raw standard | ||
| 9 | // base64. It deliberately does not accept URL-safe alphabets or non-base64 | 10 | // base64. It deliberately does not accept URL-safe alphabets or non-base64 | ||
| 10 | // whitespace other than the CR/LF ignored by the standard encoding. | 11 | // whitespace other than the CR/LF ignored by the standard encoding. | ||
| 12 | + | // | |||
| 13 | + | // The returned slice is freshly allocated and owned by the caller. | |||
| 11 | func DecodeBase64(encoded []byte) ([]byte, error) { | 14 | func DecodeBase64(encoded []byte) ([]byte, error) { | ||
| 12 | - | decoded, err := base64.StdEncoding.DecodeString(string(encoded)) | 15 | + | // RawStdEncoding rejects '=' and otherwise accepts every input |
| 16 | + | // StdEncoding does (padded input has no '=' only when its length is a | |||
| 17 | + | // multiple of four, where both encodings agree), so the alphabet used is | |||
| 18 | + | // chosen by looking for padding rather than by decoding twice. | |||
| 19 | + | enc := base64.RawStdEncoding | |||
| 20 | + | if bytes.IndexByte(encoded, '=') >= 0 { | |||
| 21 | + | enc = base64.StdEncoding | |||
| 22 | + | } | |||
| 23 | + | decoded := make([]byte, enc.DecodedLen(len(encoded))) | |||
| 24 | + | n, err := enc.Decode(decoded, encoded) | |||
| 13 | if err == nil { | 25 | if err == nil { | ||
| 14 | - | return decoded, nil | 26 | + | return decoded[:n], nil |
| 15 | } | 27 | } | ||
| 16 | - | decoded, rawErr := base64.RawStdEncoding.DecodeString(string(encoded)) | 28 | + | if enc == base64.RawStdEncoding { |
| 17 | - | if rawErr == nil { | 29 | + | // Report the StdEncoding error, as the padded decoder is the |
| 18 | - | return decoded, nil | 30 | + | // documented primary format. This only runs for rejected input. |
| 31 | + | _, err = base64.StdEncoding.Decode(make([]byte, base64.StdEncoding.DecodedLen(len(encoded))), encoded) | |||
| 19 | } | 32 | } | ||
| 20 | return nil, fmt.Errorf("%w: %v", ErrInvalidBase64, err) | 33 | return nil, fmt.Errorf("%w: %v", ErrInvalidBase64, err) | ||
| 21 | } | 34 | } | ||
A protocol/kittygraphics/base64_equiv_test.go +103 -0
| @@ -0,0 +1,103 @@ | |||
| 1 | + | package kittygraphics | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "bytes" | |
| 5 | + | "encoding/base64" | |
| 6 | + | "fmt" | |
| 7 | + | "math/rand" | |
| 8 | + | "testing" | |
| 9 | + | ) | |
| 10 | + | ||
| 11 | + | // legacyDecodeBase64 is the original two-pass implementation, kept as the | |
| 12 | + | // accept/reject and error-text oracle for DecodeBase64. | |
| 13 | + | func legacyDecodeBase64(encoded []byte) ([]byte, error) { | |
| 14 | + | decoded, err := base64.StdEncoding.DecodeString(string(encoded)) | |
| 15 | + | if err == nil { | |
| 16 | + | return decoded, nil | |
| 17 | + | } | |
| 18 | + | decoded, rawErr := base64.RawStdEncoding.DecodeString(string(encoded)) | |
| 19 | + | if rawErr == nil { | |
| 20 | + | return decoded, nil | |
| 21 | + | } | |
| 22 | + | return nil, fmt.Errorf("%w: %v", ErrInvalidBase64, err) | |
| 23 | + | } | |
| 24 | + | ||
| 25 | + | func checkBase64Equivalent(t testing.TB, input []byte) { | |
| 26 | + | t.Helper() | |
| 27 | + | want, wantErr := legacyDecodeBase64(input) | |
| 28 | + | got, gotErr := DecodeBase64(input) | |
| 29 | + | if (wantErr == nil) != (gotErr == nil) { | |
| 30 | + | t.Fatalf("DecodeBase64(%q) error = %v, legacy error = %v", input, gotErr, wantErr) | |
| 31 | + | } | |
| 32 | + | if wantErr != nil { | |
| 33 | + | if gotErr.Error() != wantErr.Error() { | |
| 34 | + | t.Fatalf("DecodeBase64(%q) error = %q, legacy %q", input, gotErr, wantErr) | |
| 35 | + | } | |
| 36 | + | if !errorsIs(gotErr, ErrInvalidBase64) { | |
| 37 | + | t.Fatalf("DecodeBase64(%q) error %v does not wrap ErrInvalidBase64", input, gotErr) | |
| 38 | + | } | |
| 39 | + | return | |
| 40 | + | } | |
| 41 | + | if !bytes.Equal(got, want) || (got == nil) != (want == nil) { | |
| 42 | + | t.Fatalf("DecodeBase64(%q) = %x, legacy %x", input, got, want) | |
| 43 | + | } | |
| 44 | + | } | |
| 45 | + | ||
| 46 | + | func TestDecodeBase64MatchesLegacyRandom(t *testing.T) { | |
| 47 | + | rng := rand.New(rand.NewSource(1)) | |
| 48 | + | alphabets := []string{ | |
| 49 | + | "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/", | |
| 50 | + | "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=\r\n", | |
| 51 | + | "AAAA==\r\n=-_ \t\x00\xff", | |
| 52 | + | } | |
| 53 | + | for i := 0; i < 200000; i++ { | |
| 54 | + | alphabet := alphabets[rng.Intn(len(alphabets))] | |
| 55 | + | input := make([]byte, rng.Intn(24)) | |
| 56 | + | for j := range input { | |
| 57 | + | input[j] = alphabet[rng.Intn(len(alphabet))] | |
| 58 | + | } | |
| 59 | + | checkBase64Equivalent(t, input) | |
| 60 | + | } | |
| 61 | + | // Valid encodings, padded and raw, with and without line breaks. | |
| 62 | + | for n := 0; n < 40; n++ { | |
| 63 | + | raw := make([]byte, n) | |
| 64 | + | rng.Read(raw) | |
| 65 | + | padded := base64.StdEncoding.EncodeToString(raw) | |
| 66 | + | unpadded := base64.RawStdEncoding.EncodeToString(raw) | |
| 67 | + | checkBase64Equivalent(t, []byte(padded)) | |
| 68 | + | checkBase64Equivalent(t, []byte(unpadded)) | |
| 69 | + | checkBase64Equivalent(t, []byte(unpadded+"\r\n")) | |
| 70 | + | if len(padded) > 2 { | |
| 71 | + | checkBase64Equivalent(t, []byte(padded[:2]+"\n"+padded[2:])) | |
| 72 | + | checkBase64Equivalent(t, []byte(padded[:len(padded)-1])) | |
| 73 | + | } | |
| 74 | + | } | |
| 75 | + | checkBase64Equivalent(t, nil) | |
| 76 | + | checkBase64Equivalent(t, []byte{}) | |
| 77 | + | } | |
| 78 | + | ||
| 79 | + | func FuzzDecodeBase64MatchesLegacy(f *testing.F) { | |
| 80 | + | for _, seed := range []string{"", "AAAA", "AAA=", "AAA", "AA==", "AA", "A", "AA=", "=", "AAAA\n", "AA\r\nAA", "-_-_"} { | |
| 81 | + | f.Add([]byte(seed)) | |
| 82 | + | } | |
| 83 | + | f.Fuzz(func(t *testing.T, input []byte) { checkBase64Equivalent(t, input) }) | |
| 84 | + | } | |
| 85 | + | ||
| 86 | + | func TestParseAPCBorrowedAliasesAndParseAPCCopies(t *testing.T) { | |
| 87 | + | apc := []byte("\x1b_Ga=t,i=1;QUJD\x1b\\") | |
| 88 | + | copied, err := ParseAPC(apc) | |
| 89 | + | if err != nil { | |
| 90 | + | t.Fatal(err) | |
| 91 | + | } | |
| 92 | + | borrowed, err := ParseAPCBorrowed(apc) | |
| 93 | + | if err != nil { | |
| 94 | + | t.Fatal(err) | |
| 95 | + | } | |
| 96 | + | apc[len("\x1b_Ga=t,i=1;")] = 'Z' | |
| 97 | + | if string(copied.Payload) != "QUJD" { | |
| 98 | + | t.Fatalf("ParseAPC payload aliased input: %q", copied.Payload) | |
| 99 | + | } | |
| 100 | + | if string(borrowed.Payload) != "ZUJD" { | |
| 101 | + | t.Fatalf("ParseAPCBorrowed payload = %q, want aliased", borrowed.Payload) | |
| 102 | + | } | |
| 103 | + | } | |
| @@ -0,0 +1,103 @@ | |||||
| 1 | + | package kittygraphics | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "bytes" | |||
| 5 | + | "encoding/base64" | |||
| 6 | + | "fmt" | |||
| 7 | + | "math/rand" | |||
| 8 | + | "testing" | |||
| 9 | + | ) | |||
| 10 | + | ||||
| 11 | + | // legacyDecodeBase64 is the original two-pass implementation, kept as the | |||
| 12 | + | // accept/reject and error-text oracle for DecodeBase64. | |||
| 13 | + | func legacyDecodeBase64(encoded []byte) ([]byte, error) { | |||
| 14 | + | decoded, err := base64.StdEncoding.DecodeString(string(encoded)) | |||
| 15 | + | if err == nil { | |||
| 16 | + | return decoded, nil | |||
| 17 | + | } | |||
| 18 | + | decoded, rawErr := base64.RawStdEncoding.DecodeString(string(encoded)) | |||
| 19 | + | if rawErr == nil { | |||
| 20 | + | return decoded, nil | |||
| 21 | + | } | |||
| 22 | + | return nil, fmt.Errorf("%w: %v", ErrInvalidBase64, err) | |||
| 23 | + | } | |||
| 24 | + | ||||
| 25 | + | func checkBase64Equivalent(t testing.TB, input []byte) { | |||
| 26 | + | t.Helper() | |||
| 27 | + | want, wantErr := legacyDecodeBase64(input) | |||
| 28 | + | got, gotErr := DecodeBase64(input) | |||
| 29 | + | if (wantErr == nil) != (gotErr == nil) { | |||
| 30 | + | t.Fatalf("DecodeBase64(%q) error = %v, legacy error = %v", input, gotErr, wantErr) | |||
| 31 | + | } | |||
| 32 | + | if wantErr != nil { | |||
| 33 | + | if gotErr.Error() != wantErr.Error() { | |||
| 34 | + | t.Fatalf("DecodeBase64(%q) error = %q, legacy %q", input, gotErr, wantErr) | |||
| 35 | + | } | |||
| 36 | + | if !errorsIs(gotErr, ErrInvalidBase64) { | |||
| 37 | + | t.Fatalf("DecodeBase64(%q) error %v does not wrap ErrInvalidBase64", input, gotErr) | |||
| 38 | + | } | |||
| 39 | + | return | |||
| 40 | + | } | |||
| 41 | + | if !bytes.Equal(got, want) || (got == nil) != (want == nil) { | |||
| 42 | + | t.Fatalf("DecodeBase64(%q) = %x, legacy %x", input, got, want) | |||
| 43 | + | } | |||
| 44 | + | } | |||
| 45 | + | ||||
| 46 | + | func TestDecodeBase64MatchesLegacyRandom(t *testing.T) { | |||
| 47 | + | rng := rand.New(rand.NewSource(1)) | |||
| 48 | + | alphabets := []string{ | |||
| 49 | + | "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/", | |||
| 50 | + | "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789+/=\r\n", | |||
| 51 | + | "AAAA==\r\n=-_ \t\x00\xff", | |||
| 52 | + | } | |||
| 53 | + | for i := 0; i < 200000; i++ { | |||
| 54 | + | alphabet := alphabets[rng.Intn(len(alphabets))] | |||
| 55 | + | input := make([]byte, rng.Intn(24)) | |||
| 56 | + | for j := range input { | |||
| 57 | + | input[j] = alphabet[rng.Intn(len(alphabet))] | |||
| 58 | + | } | |||
| 59 | + | checkBase64Equivalent(t, input) | |||
| 60 | + | } | |||
| 61 | + | // Valid encodings, padded and raw, with and without line breaks. | |||
| 62 | + | for n := 0; n < 40; n++ { | |||
| 63 | + | raw := make([]byte, n) | |||
| 64 | + | rng.Read(raw) | |||
| 65 | + | padded := base64.StdEncoding.EncodeToString(raw) | |||
| 66 | + | unpadded := base64.RawStdEncoding.EncodeToString(raw) | |||
| 67 | + | checkBase64Equivalent(t, []byte(padded)) | |||
| 68 | + | checkBase64Equivalent(t, []byte(unpadded)) | |||
| 69 | + | checkBase64Equivalent(t, []byte(unpadded+"\r\n")) | |||
| 70 | + | if len(padded) > 2 { | |||
| 71 | + | checkBase64Equivalent(t, []byte(padded[:2]+"\n"+padded[2:])) | |||
| 72 | + | checkBase64Equivalent(t, []byte(padded[:len(padded)-1])) | |||
| 73 | + | } | |||
| 74 | + | } | |||
| 75 | + | checkBase64Equivalent(t, nil) | |||
| 76 | + | checkBase64Equivalent(t, []byte{}) | |||
| 77 | + | } | |||
| 78 | + | ||||
| 79 | + | func FuzzDecodeBase64MatchesLegacy(f *testing.F) { | |||
| 80 | + | for _, seed := range []string{"", "AAAA", "AAA=", "AAA", "AA==", "AA", "A", "AA=", "=", "AAAA\n", "AA\r\nAA", "-_-_"} { | |||
| 81 | + | f.Add([]byte(seed)) | |||
| 82 | + | } | |||
| 83 | + | f.Fuzz(func(t *testing.T, input []byte) { checkBase64Equivalent(t, input) }) | |||
| 84 | + | } | |||
| 85 | + | ||||
| 86 | + | func TestParseAPCBorrowedAliasesAndParseAPCCopies(t *testing.T) { | |||
| 87 | + | apc := []byte("\x1b_Ga=t,i=1;QUJD\x1b\\") | |||
| 88 | + | copied, err := ParseAPC(apc) | |||
| 89 | + | if err != nil { | |||
| 90 | + | t.Fatal(err) | |||
| 91 | + | } | |||
| 92 | + | borrowed, err := ParseAPCBorrowed(apc) | |||
| 93 | + | if err != nil { | |||
| 94 | + | t.Fatal(err) | |||
| 95 | + | } | |||
| 96 | + | apc[len("\x1b_Ga=t,i=1;")] = 'Z' | |||
| 97 | + | if string(copied.Payload) != "QUJD" { | |||
| 98 | + | t.Fatalf("ParseAPC payload aliased input: %q", copied.Payload) | |||
| 99 | + | } | |||
| 100 | + | if string(borrowed.Payload) != "ZUJD" { | |||
| 101 | + | t.Fatalf("ParseAPCBorrowed payload = %q, want aliased", borrowed.Payload) | |||
| 102 | + | } | |||
| 103 | + | } | |||
M protocol/kittygraphics/parser.go +22 -2
| @@ -159,6 +159,10 @@ func (p *Parser) commandEvent() Event { | |||
| 159 | 159 | // exported separately so adapters can validate commands without maintaining a | |
| 160 | 160 | // stream parser. | |
| 161 | 161 | func ParseCommand(body []byte, config ...Limits) (Command, error) { | |
| 162 | + | return parseCommand(body, true, config) | |
| 163 | + | } | |
| 164 | + | ||
| 165 | + | func parseCommand(body []byte, copyPayload bool, config []Limits) (Command, error) { | |
| 162 | 166 | var l Limits | |
| 163 | 167 | if len(config) != 0 { | |
| 164 | 168 | l = config[0] | |
| @@ -179,11 +183,27 @@ func ParseCommand(body []byte, config ...Limits) (Command, error) { | |||
| 179 | 183 | if uint64(len(payload)) > l.MaxPayloadBytes { | |
| 180 | 184 | return Command{}, ErrPayloadTooLarge | |
| 181 | 185 | } | |
| 182 | - | return Command{Controls: controls, Payload: append([]byte(nil), payload...)}, nil | |
| 186 | + | if copyPayload { | |
| 187 | + | payload = append([]byte(nil), payload...) | |
| 188 | + | } | |
| 189 | + | return Command{Controls: controls, Payload: payload}, nil | |
| 183 | 190 | } | |
| 184 | 191 | ||
| 185 | 192 | // ParseAPC parses a complete ESC _ G ... ST sequence. | |
| 186 | 193 | func ParseAPC(apc []byte, config ...Limits) (Command, error) { | |
| 194 | + | return parseAPC(apc, true, config) | |
| 195 | + | } | |
| 196 | + | ||
| 197 | + | // ParseAPCBorrowed is ParseAPC without copying the payload: the returned | |
| 198 | + | // Command.Payload aliases apc. The caller must not modify apc while the | |
| 199 | + | // command is in use and must not retain the command beyond the lifetime of | |
| 200 | + | // apc. Session.Process copies whatever it keeps from a command's payload, so | |
| 201 | + | // a borrowed command may be passed to it and apc reused afterwards. | |
| 202 | + | func ParseAPCBorrowed(apc []byte, config ...Limits) (Command, error) { | |
| 203 | + | return parseAPC(apc, false, config) | |
| 204 | + | } | |
| 205 | + | ||
| 206 | + | func parseAPC(apc []byte, copyPayload bool, config []Limits) (Command, error) { | |
| 187 | 207 | if len(apc) < 4 || apc[0] != 0x1b || apc[1] != '_' || apc[2] != 'G' { | |
| 188 | 208 | return Command{}, ErrInvalidAPC | |
| 189 | 209 | } | |
| @@ -195,5 +215,5 @@ func ParseAPC(apc []byte, config ...Limits) (Command, error) { | |||
| 195 | 215 | } else { | |
| 196 | 216 | return Command{}, ErrAPCTruncated | |
| 197 | 217 | } | |
| 198 | - | return ParseCommand(body, config...) | |
| 218 | + | return parseCommand(body, copyPayload, config) | |
| 199 | 219 | } | |
| @@ -159,6 +159,10 @@ func (p *Parser) commandEvent() Event { | |||||
| 159 | // exported separately so adapters can validate commands without maintaining a | 159 | // exported separately so adapters can validate commands without maintaining a | ||
| 160 | // stream parser. | 160 | // stream parser. | ||
| 161 | func ParseCommand(body []byte, config ...Limits) (Command, error) { | 161 | func ParseCommand(body []byte, config ...Limits) (Command, error) { | ||
| 162 | + | return parseCommand(body, true, config) | |||
| 163 | + | } | |||
| 164 | + | ||||
| 165 | + | func parseCommand(body []byte, copyPayload bool, config []Limits) (Command, error) { | |||
| 162 | var l Limits | 166 | var l Limits | ||
| 163 | if len(config) != 0 { | 167 | if len(config) != 0 { | ||
| 164 | l = config[0] | 168 | l = config[0] | ||
| @@ -179,11 +183,27 @@ func ParseCommand(body []byte, config ...Limits) (Command, error) { | |||||
| 179 | if uint64(len(payload)) > l.MaxPayloadBytes { | 183 | if uint64(len(payload)) > l.MaxPayloadBytes { | ||
| 180 | return Command{}, ErrPayloadTooLarge | 184 | return Command{}, ErrPayloadTooLarge | ||
| 181 | } | 185 | } | ||
| 182 | - | return Command{Controls: controls, Payload: append([]byte(nil), payload...)}, nil | 186 | + | if copyPayload { |
| 187 | + | payload = append([]byte(nil), payload...) | |||
| 188 | + | } | |||
| 189 | + | return Command{Controls: controls, Payload: payload}, nil | |||
| 183 | } | 190 | } | ||
| 184 | 191 | ||||
| 185 | // ParseAPC parses a complete ESC _ G ... ST sequence. | 192 | // ParseAPC parses a complete ESC _ G ... ST sequence. | ||
| 186 | func ParseAPC(apc []byte, config ...Limits) (Command, error) { | 193 | func ParseAPC(apc []byte, config ...Limits) (Command, error) { | ||
| 194 | + | return parseAPC(apc, true, config) | |||
| 195 | + | } | |||
| 196 | + | ||||
| 197 | + | // ParseAPCBorrowed is ParseAPC without copying the payload: the returned | |||
| 198 | + | // Command.Payload aliases apc. The caller must not modify apc while the | |||
| 199 | + | // command is in use and must not retain the command beyond the lifetime of | |||
| 200 | + | // apc. Session.Process copies whatever it keeps from a command's payload, so | |||
| 201 | + | // a borrowed command may be passed to it and apc reused afterwards. | |||
| 202 | + | func ParseAPCBorrowed(apc []byte, config ...Limits) (Command, error) { | |||
| 203 | + | return parseAPC(apc, false, config) | |||
| 204 | + | } | |||
| 205 | + | ||||
| 206 | + | func parseAPC(apc []byte, copyPayload bool, config []Limits) (Command, error) { | |||
| 187 | if len(apc) < 4 || apc[0] != 0x1b || apc[1] != '_' || apc[2] != 'G' { | 207 | if len(apc) < 4 || apc[0] != 0x1b || apc[1] != '_' || apc[2] != 'G' { | ||
| 188 | return Command{}, ErrInvalidAPC | 208 | return Command{}, ErrInvalidAPC | ||
| 189 | } | 209 | } | ||
| @@ -195,5 +215,5 @@ func ParseAPC(apc []byte, config ...Limits) (Command, error) { | |||||
| 195 | } else { | 215 | } else { | ||
| 196 | return Command{}, ErrAPCTruncated | 216 | return Command{}, ErrAPCTruncated | ||
| 197 | } | 217 | } | ||
| 198 | - | return ParseCommand(body, config...) | 218 | + | return parseCommand(body, copyPayload, config) |
| 199 | } | 219 | } | ||
M protocol/kittygraphics/session.go +15 -7
| @@ -162,6 +162,12 @@ func (s *Session) Finish() (Result, error) { | |||
| 162 | 162 | ||
| 163 | 163 | // Process applies one already parsed command. It is useful for callers that | |
| 164 | 164 | // have their own stream framing but want this package's strict adapter. | |
| 165 | + | // | |
| 166 | + | // Process never retains command.Payload (or the APC bytes it was parsed from) | |
| 167 | + | // beyond the call: whatever must outlive it, such as a pending chunked upload | |
| 168 | + | // or a committed asset, is copied or freshly decoded. Callers may therefore pass | |
| 169 | + | // a command from ParseAPCBorrowed and reuse or overwrite the underlying bytes | |
| 170 | + | // as soon as Process returns. | |
| 165 | 171 | func (s *Session) Process(command Command) (Result, error) { | |
| 166 | 172 | if s == nil { | |
| 167 | 173 | return Result{}, ErrNoScene | |
| @@ -287,14 +293,16 @@ func (s *Session) transmit(command Command, display bool) ([][]byte, *Mutation, | |||
| 287 | 293 | s.upload = &upload{ | |
| 288 | 294 | controls: c, | |
| 289 | 295 | imageID: imageID, | |
| 290 | - | payload: append([]byte(nil), command.Payload...), | |
| 291 | - | chunks: 1, | |
| 292 | - | display: display, | |
| 293 | - | origin: s.origin, | |
| 296 | + | // The session keeps the payload across Process calls, so it must | |
| 297 | + | // not alias the caller's (possibly borrowed) bytes. | |
| 298 | + | payload: append([]byte(nil), command.Payload...), | |
| 299 | + | chunks: 1, | |
| 300 | + | display: display, | |
| 301 | + | origin: s.origin, | |
| 294 | 302 | } | |
| 295 | 303 | return nil, nil, nil | |
| 296 | 304 | } | |
| 297 | - | return s.commitUpload(upload{controls: c, imageID: imageID, payload: append([]byte(nil), command.Payload...), chunks: 1, display: display, origin: s.origin}, display) | |
| 305 | + | return s.commitUpload(upload{controls: c, imageID: imageID, payload: command.Payload, chunks: 1, display: display, origin: s.origin}, display) | |
| 298 | 306 | } | |
| 299 | 307 | ||
| 300 | 308 | func (s *Session) commitUpload(value upload, display bool) ([][]byte, *Mutation, error) { | |
| @@ -336,14 +344,14 @@ func (s *Session) commitUpload(value upload, display bool) ([][]byte, *Mutation, | |||
| 336 | 344 | if replacing { | |
| 337 | 345 | // Scene replacement is copy-on-write: a rejected new asset leaves the | |
| 338 | 346 | // old asset and every placement that references it untouched. | |
| 339 | - | assetID, err = s.scene.ReplaceAsset(old, graphics.AssetBlob{ | |
| 347 | + | assetID, err = s.scene.ReplaceAssetOwned(old, graphics.AssetBlob{ | |
| 340 | 348 | Encoded: decoded, | |
| 341 | 349 | Format: assetFormat(format), | |
| 342 | 350 | Width: width, | |
| 343 | 351 | Height: height, | |
| 344 | 352 | }) | |
| 345 | 353 | } else { | |
| 346 | - | assetID, err = s.scene.AddAsset(graphics.AssetBlob{ | |
| 354 | + | assetID, err = s.scene.AddAssetOwned(graphics.AssetBlob{ | |
| 347 | 355 | Encoded: decoded, | |
| 348 | 356 | Format: assetFormat(format), | |
| 349 | 357 | Width: width, | |
| @@ -162,6 +162,12 @@ func (s *Session) Finish() (Result, error) { | |||||
| 162 | 162 | ||||
| 163 | // Process applies one already parsed command. It is useful for callers that | 163 | // Process applies one already parsed command. It is useful for callers that | ||
| 164 | // have their own stream framing but want this package's strict adapter. | 164 | // have their own stream framing but want this package's strict adapter. | ||
| 165 | + | // | |||
| 166 | + | // Process never retains command.Payload (or the APC bytes it was parsed from) | |||
| 167 | + | // beyond the call: whatever must outlive it, such as a pending chunked upload | |||
| 168 | + | // or a committed asset, is copied or freshly decoded. Callers may therefore pass | |||
| 169 | + | // a command from ParseAPCBorrowed and reuse or overwrite the underlying bytes | |||
| 170 | + | // as soon as Process returns. | |||
| 165 | func (s *Session) Process(command Command) (Result, error) { | 171 | func (s *Session) Process(command Command) (Result, error) { | ||
| 166 | if s == nil { | 172 | if s == nil { | ||
| 167 | return Result{}, ErrNoScene | 173 | return Result{}, ErrNoScene | ||
| @@ -287,14 +293,16 @@ func (s *Session) transmit(command Command, display bool) ([][]byte, *Mutation, | |||||
| 287 | s.upload = &upload{ | 293 | s.upload = &upload{ | ||
| 288 | controls: c, | 294 | controls: c, | ||
| 289 | imageID: imageID, | 295 | imageID: imageID, | ||
| 290 | - | payload: append([]byte(nil), command.Payload...), | 296 | + | // The session keeps the payload across Process calls, so it must |
| 291 | - | chunks: 1, | 297 | + | // not alias the caller's (possibly borrowed) bytes. |
| 292 | - | display: display, | 298 | + | payload: append([]byte(nil), command.Payload...), |
| 293 | - | origin: s.origin, | 299 | + | chunks: 1, |
| 300 | + | display: display, | |||
| 301 | + | origin: s.origin, | |||
| 294 | } | 302 | } | ||
| 295 | return nil, nil, nil | 303 | return nil, nil, nil | ||
| 296 | } | 304 | } | ||
| 297 | - | return s.commitUpload(upload{controls: c, imageID: imageID, payload: append([]byte(nil), command.Payload...), chunks: 1, display: display, origin: s.origin}, display) | 305 | + | return s.commitUpload(upload{controls: c, imageID: imageID, payload: command.Payload, chunks: 1, display: display, origin: s.origin}, display) |
| 298 | } | 306 | } | ||
| 299 | 307 | ||||
| 300 | func (s *Session) commitUpload(value upload, display bool) ([][]byte, *Mutation, error) { | 308 | func (s *Session) commitUpload(value upload, display bool) ([][]byte, *Mutation, error) { | ||
| @@ -336,14 +344,14 @@ func (s *Session) commitUpload(value upload, display bool) ([][]byte, *Mutation, | |||||
| 336 | if replacing { | 344 | if replacing { | ||
| 337 | // Scene replacement is copy-on-write: a rejected new asset leaves the | 345 | // Scene replacement is copy-on-write: a rejected new asset leaves the | ||
| 338 | // old asset and every placement that references it untouched. | 346 | // old asset and every placement that references it untouched. | ||
| 339 | - | assetID, err = s.scene.ReplaceAsset(old, graphics.AssetBlob{ | 347 | + | assetID, err = s.scene.ReplaceAssetOwned(old, graphics.AssetBlob{ |
| 340 | Encoded: decoded, | 348 | Encoded: decoded, | ||
| 341 | Format: assetFormat(format), | 349 | Format: assetFormat(format), | ||
| 342 | Width: width, | 350 | Width: width, | ||
| 343 | Height: height, | 351 | Height: height, | ||
| 344 | }) | 352 | }) | ||
| 345 | } else { | 353 | } else { | ||
| 346 | - | assetID, err = s.scene.AddAsset(graphics.AssetBlob{ | 354 | + | assetID, err = s.scene.AddAssetOwned(graphics.AssetBlob{ |
| 347 | Encoded: decoded, | 355 | Encoded: decoded, | ||
| 348 | Format: assetFormat(format), | 356 | Format: assetFormat(format), | ||
| 349 | Width: width, | 357 | Width: width, | ||
M screen.go +75 -18
| @@ -18,6 +18,11 @@ const maxEscapeBufferLen = 128 * 1024 | |||
| 18 | 18 | // through the text path or truncated at the OSC clipboard bound. | |
| 19 | 19 | const maxKittyEscapeBufferLen = int(kittygraphics.DefaultMaxAPCBytes) + 5 | |
| 20 | 20 | ||
| 21 | + | // maxRetainedEscapeBufferCap bounds the capacity of the escape scratch buffers | |
| 22 | + | // kept between writes so that a single huge APC does not pin its memory, while | |
| 23 | + | // ordinary fragmented uploads still reuse their buffer. | |
| 24 | + | const maxRetainedEscapeBufferCap = 1 << 20 | |
| 25 | + | ||
| 21 | 26 | const ( | |
| 22 | 27 | // ColorSchemeReportDark is the DEC 2031 dark-scheme report. | |
| 23 | 28 | ColorSchemeReportDark = "\x1b[?997;1n" | |
| @@ -72,6 +77,8 @@ type Screen struct { | |||
| 72 | 77 | damageSaturated bool | |
| 73 | 78 | damageFullRedrawSticky bool | |
| 74 | 79 | escapeBuf []byte | |
| 80 | + | combineBuf []byte | |
| 81 | + | evictScratch []renderer.Cell | |
| 75 | 82 | kittyDiscard bool | |
| 76 | 83 | kittyDiscardEscaped bool | |
| 77 | 84 | kittyPendingDisplay *kittygraphics.Controls | |
| @@ -168,7 +175,9 @@ func (s *Screen) LineBounds() []LineBound { | |||
| 168 | 175 | } | |
| 169 | 176 | ||
| 170 | 177 | func (s *Screen) Write(data []byte) { | |
| 171 | - | if len(s.escapeBuf) > 0 && isKittyEscapeContinuation(s.escapeBuf, data) { | |
| 178 | + | if len(s.escapeBuf) == 0 { | |
| 179 | + | s.trimEscapeBuffers() | |
| 180 | + | } else if isKittyEscapeContinuation(s.escapeBuf, data) { | |
| 172 | 181 | prefix := s.escapeBuf | |
| 173 | 182 | s.escapeBuf = nil | |
| 174 | 183 | data = s.continueKittyEscape(prefix, data) | |
| @@ -177,11 +186,30 @@ func (s *Screen) Write(data []byte) { | |||
| 177 | 186 | } | |
| 178 | 187 | } | |
| 179 | 188 | if len(s.escapeBuf) > 0 { | |
| 180 | - | combined := make([]byte, 0, len(s.escapeBuf)+len(data)) | |
| 189 | + | // Reuse a bounded scratch buffer: data is consumed before Write returns | |
| 190 | + | // and nothing retains slices of it (the partial-escape path copies). | |
| 191 | + | // Detach it while in use: callbacks invoked below may call Write | |
| 192 | + | // re-entrantly, and that inner Write must not overwrite bytes this | |
| 193 | + | // Write is still reading. | |
| 194 | + | need := len(s.escapeBuf) + len(data) | |
| 195 | + | combined := s.combineBuf[:0] | |
| 196 | + | s.combineBuf = nil | |
| 197 | + | if cap(combined) < need { | |
| 198 | + | combined = make([]byte, 0, need) | |
| 199 | + | } | |
| 181 | 200 | combined = append(combined, s.escapeBuf...) | |
| 182 | 201 | combined = append(combined, data...) | |
| 202 | + | defer func() { | |
| 203 | + | if s.combineBuf == nil && cap(combined) <= maxRetainedEscapeBufferCap { | |
| 204 | + | s.combineBuf = combined[:0] | |
| 205 | + | } | |
| 206 | + | }() | |
| 183 | 207 | data = combined | |
| 184 | - | s.escapeBuf = nil | |
| 208 | + | if cap(s.escapeBuf) > maxRetainedEscapeBufferCap { | |
| 209 | + | s.escapeBuf = nil | |
| 210 | + | } else { | |
| 211 | + | s.escapeBuf = s.escapeBuf[:0] | |
| 212 | + | } | |
| 185 | 213 | } | |
| 186 | 214 | if s.kittyDiscard { | |
| 187 | 215 | consumed := s.consumeKittyDiscard(data) | |
| @@ -249,42 +277,55 @@ func isKittyEscapeContinuation(prefix, data []byte) bool { | |||
| 249 | 277 | return true | |
| 250 | 278 | } | |
| 251 | 279 | ||
| 280 | + | // trimEscapeBuffers drops oversized idle escape scratch buffers. | |
| 281 | + | func (s *Screen) trimEscapeBuffers() { | |
| 282 | + | if cap(s.escapeBuf) > maxRetainedEscapeBufferCap { | |
| 283 | + | s.escapeBuf = nil | |
| 284 | + | } | |
| 285 | + | if cap(s.combineBuf) > maxRetainedEscapeBufferCap { | |
| 286 | + | s.combineBuf = nil | |
| 287 | + | } | |
| 288 | + | } | |
| 289 | + | ||
| 290 | + | // recycleEscapeBuf keeps buf's capacity for the next fragmented escape when no | |
| 291 | + | // other buffer has been installed meanwhile. Callers must be done with buf. | |
| 292 | + | func (s *Screen) recycleEscapeBuf(buf []byte) { | |
| 293 | + | if s.escapeBuf == nil && cap(buf) <= maxRetainedEscapeBufferCap { | |
| 294 | + | s.escapeBuf = buf[:0] | |
| 295 | + | } | |
| 296 | + | } | |
| 297 | + | ||
| 298 | + | // continueKittyEscape completes or extends the pending Kitty APC in prefix, | |
| 299 | + | // which is the Screen's former escapeBuf (s.escapeBuf is nil on entry). The | |
| 300 | + | // protocol adapter copies whatever it retains from a dispatched APC, so prefix | |
| 301 | + | // is recycled afterwards instead of being reallocated for every APC. | |
| 252 | 302 | func (s *Screen) continueKittyEscape(prefix, data []byte) []byte { | |
| 253 | 303 | if len(prefix) != 0 && prefix[len(prefix)-1] == 0x1b && len(data) != 0 && data[0] == '\\' { | |
| 254 | 304 | if len(prefix)+1 <= maxKittyEscapeBufferLen { | |
| 255 | 305 | apc := append(prefix, data[:1]...) | |
| 256 | 306 | s.dispatchKittyGraphics(apc) | |
| 307 | + | s.recycleEscapeBuf(apc) | |
| 257 | 308 | } else { | |
| 258 | 309 | s.abortKittyPendingDisplay() | |
| 310 | + | s.recycleEscapeBuf(prefix) | |
| 259 | 311 | } | |
| 260 | 312 | return data[1:] | |
| 261 | 313 | } | |
| 262 | 314 | for i := 0; i < len(data); i++ { | |
| 263 | 315 | if data[i] == 0x9c { | |
| 264 | 316 | end := i + 1 | |
| 265 | - | if len(prefix)+end <= maxKittyEscapeBufferLen { | |
| 266 | - | apc := append(prefix, data[:end]...) | |
| 267 | - | s.dispatchKittyGraphics(apc) | |
| 268 | - | } else { | |
| 269 | - | s.abortKittyPendingDisplay() | |
| 270 | - | } | |
| 317 | + | s.finishKittyEscape(prefix, data[:end]) | |
| 271 | 318 | return data[end:] | |
| 272 | 319 | } | |
| 273 | 320 | if data[i] == 0x1b && i+1 < len(data) && data[i+1] == '\\' { | |
| 274 | 321 | end := i + 2 | |
| 275 | - | if len(prefix)+end <= maxKittyEscapeBufferLen { | |
| 276 | - | apc := append(prefix, data[:end]...) | |
| 277 | - | s.dispatchKittyGraphics(apc) | |
| 278 | - | } else { | |
| 279 | - | s.abortKittyPendingDisplay() | |
| 280 | - | } | |
| 322 | + | s.finishKittyEscape(prefix, data[:end]) | |
| 281 | 323 | return data[end:] | |
| 282 | 324 | } | |
| 283 | 325 | } | |
| 284 | 326 | remaining := maxKittyEscapeBufferLen - len(prefix) | |
| 285 | 327 | if len(data) <= remaining { | |
| 286 | - | returnData := append(prefix, data...) | |
| 287 | - | s.escapeBuf = returnData | |
| 328 | + | s.escapeBuf = append(prefix, data...) | |
| 288 | 329 | return nil | |
| 289 | 330 | } | |
| 290 | 331 | s.abortKittyPendingDisplay() | |
| @@ -294,10 +335,24 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte { | |||
| 294 | 335 | escaped = data[remaining-1] == 0x1b | |
| 295 | 336 | } | |
| 296 | 337 | s.kittyDiscardEscaped = escaped | |
| 338 | + | s.recycleEscapeBuf(prefix) | |
| 297 | 339 | tail := data[remaining:] | |
| 298 | 340 | return tail[s.consumeKittyDiscard(tail):] | |
| 299 | 341 | } | |
| 300 | 342 | ||
| 343 | + | // finishKittyEscape dispatches prefix+terminated, or aborts when the framed | |
| 344 | + | // APC would exceed the Kitty escape bound, then recycles prefix. | |
| 345 | + | func (s *Screen) finishKittyEscape(prefix, terminated []byte) { | |
| 346 | + | if len(prefix)+len(terminated) > maxKittyEscapeBufferLen { | |
| 347 | + | s.abortKittyPendingDisplay() | |
| 348 | + | s.recycleEscapeBuf(prefix) | |
| 349 | + | return | |
| 350 | + | } | |
| 351 | + | apc := append(prefix, terminated...) | |
| 352 | + | s.dispatchKittyGraphics(apc) | |
| 353 | + | s.recycleEscapeBuf(apc) | |
| 354 | + | } | |
| 355 | + | ||
| 301 | 356 | func (s *Screen) consumeKittyDiscard(data []byte) int { | |
| 302 | 357 | for i, b := range data { | |
| 303 | 358 | if s.kittyDiscardEscaped { | |
| @@ -418,7 +473,9 @@ func (s *Screen) consumeKittyGraphics(data []byte) (consumed int, partial bool) | |||
| 418 | 473 | } | |
| 419 | 474 | ||
| 420 | 475 | func (s *Screen) dispatchKittyGraphics(apc []byte) { | |
| 421 | - | command, err := kittygraphics.ParseAPC(apc) | |
| 476 | + | // The session copies any payload it retains, so the APC bytes may be | |
| 477 | + | // borrowed for the duration of this call only. | |
| 478 | + | command, err := kittygraphics.ParseAPCBorrowed(apc) | |
| 422 | 479 | if err != nil { | |
| 423 | 480 | s.abortKittyPendingDisplay() | |
| 424 | 481 | return | |
| @@ -18,6 +18,11 @@ const maxEscapeBufferLen = 128 * 1024 | |||||
| 18 | // through the text path or truncated at the OSC clipboard bound. | 18 | // through the text path or truncated at the OSC clipboard bound. | ||
| 19 | const maxKittyEscapeBufferLen = int(kittygraphics.DefaultMaxAPCBytes) + 5 | 19 | const maxKittyEscapeBufferLen = int(kittygraphics.DefaultMaxAPCBytes) + 5 | ||
| 20 | 20 | ||||
| 21 | + | // maxRetainedEscapeBufferCap bounds the capacity of the escape scratch buffers | |||
| 22 | + | // kept between writes so that a single huge APC does not pin its memory, while | |||
| 23 | + | // ordinary fragmented uploads still reuse their buffer. | |||
| 24 | + | const maxRetainedEscapeBufferCap = 1 << 20 | |||
| 25 | + | ||||
| 21 | const ( | 26 | const ( | ||
| 22 | // ColorSchemeReportDark is the DEC 2031 dark-scheme report. | 27 | // ColorSchemeReportDark is the DEC 2031 dark-scheme report. | ||
| 23 | ColorSchemeReportDark = "\x1b[?997;1n" | 28 | ColorSchemeReportDark = "\x1b[?997;1n" | ||
| @@ -72,6 +77,8 @@ type Screen struct { | |||||
| 72 | damageSaturated bool | 77 | damageSaturated bool | ||
| 73 | damageFullRedrawSticky bool | 78 | damageFullRedrawSticky bool | ||
| 74 | escapeBuf []byte | 79 | escapeBuf []byte | ||
| 80 | + | combineBuf []byte | |||
| 81 | + | evictScratch []renderer.Cell | |||
| 75 | kittyDiscard bool | 82 | kittyDiscard bool | ||
| 76 | kittyDiscardEscaped bool | 83 | kittyDiscardEscaped bool | ||
| 77 | kittyPendingDisplay *kittygraphics.Controls | 84 | kittyPendingDisplay *kittygraphics.Controls | ||
| @@ -168,7 +175,9 @@ func (s *Screen) LineBounds() []LineBound { | |||||
| 168 | } | 175 | } | ||
| 169 | 176 | ||||
| 170 | func (s *Screen) Write(data []byte) { | 177 | func (s *Screen) Write(data []byte) { | ||
| 171 | - | if len(s.escapeBuf) > 0 && isKittyEscapeContinuation(s.escapeBuf, data) { | 178 | + | if len(s.escapeBuf) == 0 { |
| 179 | + | s.trimEscapeBuffers() | |||
| 180 | + | } else if isKittyEscapeContinuation(s.escapeBuf, data) { | |||
| 172 | prefix := s.escapeBuf | 181 | prefix := s.escapeBuf | ||
| 173 | s.escapeBuf = nil | 182 | s.escapeBuf = nil | ||
| 174 | data = s.continueKittyEscape(prefix, data) | 183 | data = s.continueKittyEscape(prefix, data) | ||
| @@ -177,11 +186,30 @@ func (s *Screen) Write(data []byte) { | |||||
| 177 | } | 186 | } | ||
| 178 | } | 187 | } | ||
| 179 | if len(s.escapeBuf) > 0 { | 188 | if len(s.escapeBuf) > 0 { | ||
| 180 | - | combined := make([]byte, 0, len(s.escapeBuf)+len(data)) | 189 | + | // Reuse a bounded scratch buffer: data is consumed before Write returns |
| 190 | + | // and nothing retains slices of it (the partial-escape path copies). | |||
| 191 | + | // Detach it while in use: callbacks invoked below may call Write | |||
| 192 | + | // re-entrantly, and that inner Write must not overwrite bytes this | |||
| 193 | + | // Write is still reading. | |||
| 194 | + | need := len(s.escapeBuf) + len(data) | |||
| 195 | + | combined := s.combineBuf[:0] | |||
| 196 | + | s.combineBuf = nil | |||
| 197 | + | if cap(combined) < need { | |||
| 198 | + | combined = make([]byte, 0, need) | |||
| 199 | + | } | |||
| 181 | combined = append(combined, s.escapeBuf...) | 200 | combined = append(combined, s.escapeBuf...) | ||
| 182 | combined = append(combined, data...) | 201 | combined = append(combined, data...) | ||
| 202 | + | defer func() { | |||
| 203 | + | if s.combineBuf == nil && cap(combined) <= maxRetainedEscapeBufferCap { | |||
| 204 | + | s.combineBuf = combined[:0] | |||
| 205 | + | } | |||
| 206 | + | }() | |||
| 183 | data = combined | 207 | data = combined | ||
| 184 | - | s.escapeBuf = nil | 208 | + | if cap(s.escapeBuf) > maxRetainedEscapeBufferCap { |
| 209 | + | s.escapeBuf = nil | |||
| 210 | + | } else { | |||
| 211 | + | s.escapeBuf = s.escapeBuf[:0] | |||
| 212 | + | } | |||
| 185 | } | 213 | } | ||
| 186 | if s.kittyDiscard { | 214 | if s.kittyDiscard { | ||
| 187 | consumed := s.consumeKittyDiscard(data) | 215 | consumed := s.consumeKittyDiscard(data) | ||
| @@ -249,42 +277,55 @@ func isKittyEscapeContinuation(prefix, data []byte) bool { | |||||
| 249 | return true | 277 | return true | ||
| 250 | } | 278 | } | ||
| 251 | 279 | ||||
| 280 | + | // trimEscapeBuffers drops oversized idle escape scratch buffers. | |||
| 281 | + | func (s *Screen) trimEscapeBuffers() { | |||
| 282 | + | if cap(s.escapeBuf) > maxRetainedEscapeBufferCap { | |||
| 283 | + | s.escapeBuf = nil | |||
| 284 | + | } | |||
| 285 | + | if cap(s.combineBuf) > maxRetainedEscapeBufferCap { | |||
| 286 | + | s.combineBuf = nil | |||
| 287 | + | } | |||
| 288 | + | } | |||
| 289 | + | ||||
| 290 | + | // recycleEscapeBuf keeps buf's capacity for the next fragmented escape when no | |||
| 291 | + | // other buffer has been installed meanwhile. Callers must be done with buf. | |||
| 292 | + | func (s *Screen) recycleEscapeBuf(buf []byte) { | |||
| 293 | + | if s.escapeBuf == nil && cap(buf) <= maxRetainedEscapeBufferCap { | |||
| 294 | + | s.escapeBuf = buf[:0] | |||
| 295 | + | } | |||
| 296 | + | } | |||
| 297 | + | ||||
| 298 | + | // continueKittyEscape completes or extends the pending Kitty APC in prefix, | |||
| 299 | + | // which is the Screen's former escapeBuf (s.escapeBuf is nil on entry). The | |||
| 300 | + | // protocol adapter copies whatever it retains from a dispatched APC, so prefix | |||
| 301 | + | // is recycled afterwards instead of being reallocated for every APC. | |||
| 252 | func (s *Screen) continueKittyEscape(prefix, data []byte) []byte { | 302 | func (s *Screen) continueKittyEscape(prefix, data []byte) []byte { | ||
| 253 | if len(prefix) != 0 && prefix[len(prefix)-1] == 0x1b && len(data) != 0 && data[0] == '\\' { | 303 | if len(prefix) != 0 && prefix[len(prefix)-1] == 0x1b && len(data) != 0 && data[0] == '\\' { | ||
| 254 | if len(prefix)+1 <= maxKittyEscapeBufferLen { | 304 | if len(prefix)+1 <= maxKittyEscapeBufferLen { | ||
| 255 | apc := append(prefix, data[:1]...) | 305 | apc := append(prefix, data[:1]...) | ||
| 256 | s.dispatchKittyGraphics(apc) | 306 | s.dispatchKittyGraphics(apc) | ||
| 307 | + | s.recycleEscapeBuf(apc) | |||
| 257 | } else { | 308 | } else { | ||
| 258 | s.abortKittyPendingDisplay() | 309 | s.abortKittyPendingDisplay() | ||
| 310 | + | s.recycleEscapeBuf(prefix) | |||
| 259 | } | 311 | } | ||
| 260 | return data[1:] | 312 | return data[1:] | ||
| 261 | } | 313 | } | ||
| 262 | for i := 0; i < len(data); i++ { | 314 | for i := 0; i < len(data); i++ { | ||
| 263 | if data[i] == 0x9c { | 315 | if data[i] == 0x9c { | ||
| 264 | end := i + 1 | 316 | end := i + 1 | ||
| 265 | - | if len(prefix)+end <= maxKittyEscapeBufferLen { | 317 | + | s.finishKittyEscape(prefix, data[:end]) |
| 266 | - | apc := append(prefix, data[:end]...) | |||
| 267 | - | s.dispatchKittyGraphics(apc) | |||
| 268 | - | } else { | |||
| 269 | - | s.abortKittyPendingDisplay() | |||
| 270 | - | } | |||
| 271 | return data[end:] | 318 | return data[end:] | ||
| 272 | } | 319 | } | ||
| 273 | if data[i] == 0x1b && i+1 < len(data) && data[i+1] == '\\' { | 320 | if data[i] == 0x1b && i+1 < len(data) && data[i+1] == '\\' { | ||
| 274 | end := i + 2 | 321 | end := i + 2 | ||
| 275 | - | if len(prefix)+end <= maxKittyEscapeBufferLen { | 322 | + | s.finishKittyEscape(prefix, data[:end]) |
| 276 | - | apc := append(prefix, data[:end]...) | |||
| 277 | - | s.dispatchKittyGraphics(apc) | |||
| 278 | - | } else { | |||
| 279 | - | s.abortKittyPendingDisplay() | |||
| 280 | - | } | |||
| 281 | return data[end:] | 323 | return data[end:] | ||
| 282 | } | 324 | } | ||
| 283 | } | 325 | } | ||
| 284 | remaining := maxKittyEscapeBufferLen - len(prefix) | 326 | remaining := maxKittyEscapeBufferLen - len(prefix) | ||
| 285 | if len(data) <= remaining { | 327 | if len(data) <= remaining { | ||
| 286 | - | returnData := append(prefix, data...) | 328 | + | s.escapeBuf = append(prefix, data...) |
| 287 | - | s.escapeBuf = returnData | |||
| 288 | return nil | 329 | return nil | ||
| 289 | } | 330 | } | ||
| 290 | s.abortKittyPendingDisplay() | 331 | s.abortKittyPendingDisplay() | ||
| @@ -294,10 +335,24 @@ func (s *Screen) continueKittyEscape(prefix, data []byte) []byte { | |||||
| 294 | escaped = data[remaining-1] == 0x1b | 335 | escaped = data[remaining-1] == 0x1b | ||
| 295 | } | 336 | } | ||
| 296 | s.kittyDiscardEscaped = escaped | 337 | s.kittyDiscardEscaped = escaped | ||
| 338 | + | s.recycleEscapeBuf(prefix) | |||
| 297 | tail := data[remaining:] | 339 | tail := data[remaining:] | ||
| 298 | return tail[s.consumeKittyDiscard(tail):] | 340 | return tail[s.consumeKittyDiscard(tail):] | ||
| 299 | } | 341 | } | ||
| 300 | 342 | ||||
| 343 | + | // finishKittyEscape dispatches prefix+terminated, or aborts when the framed | |||
| 344 | + | // APC would exceed the Kitty escape bound, then recycles prefix. | |||
| 345 | + | func (s *Screen) finishKittyEscape(prefix, terminated []byte) { | |||
| 346 | + | if len(prefix)+len(terminated) > maxKittyEscapeBufferLen { | |||
| 347 | + | s.abortKittyPendingDisplay() | |||
| 348 | + | s.recycleEscapeBuf(prefix) | |||
| 349 | + | return | |||
| 350 | + | } | |||
| 351 | + | apc := append(prefix, terminated...) | |||
| 352 | + | s.dispatchKittyGraphics(apc) | |||
| 353 | + | s.recycleEscapeBuf(apc) | |||
| 354 | + | } | |||
| 355 | + | ||||
| 301 | func (s *Screen) consumeKittyDiscard(data []byte) int { | 356 | func (s *Screen) consumeKittyDiscard(data []byte) int { | ||
| 302 | for i, b := range data { | 357 | for i, b := range data { | ||
| 303 | if s.kittyDiscardEscaped { | 358 | if s.kittyDiscardEscaped { | ||
| @@ -418,7 +473,9 @@ func (s *Screen) consumeKittyGraphics(data []byte) (consumed int, partial bool) | |||||
| 418 | } | 473 | } | ||
| 419 | 474 | ||||
| 420 | func (s *Screen) dispatchKittyGraphics(apc []byte) { | 475 | func (s *Screen) dispatchKittyGraphics(apc []byte) { | ||
| 421 | - | command, err := kittygraphics.ParseAPC(apc) | 476 | + | // The session copies any payload it retains, so the APC bytes may be |
| 477 | + | // borrowed for the duration of this call only. | |||
| 478 | + | command, err := kittygraphics.ParseAPCBorrowed(apc) | |||
| 422 | if err != nil { | 479 | if err != nil { | ||
| 423 | s.abortKittyPendingDisplay() | 480 | s.abortKittyPendingDisplay() | ||
| 424 | return | 481 | return | ||
A screen_evict_scratch_test.go +26 -0
| @@ -0,0 +1,26 @@ | |||
| 1 | + | package vt | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "testing" | |
| 5 | + | ||
| 6 | + | renderer "github.com/bnema/vev-vt/core" | |
| 7 | + | ) | |
| 8 | + | ||
| 9 | + | // OnLineEvicted callers may retain the slice: successive evictions must not | |
| 10 | + | // share the Screen's internal scratch row. | |
| 11 | + | func TestOnLineEvictedRowsAreIndependentCopies(t *testing.T) { | |
| 12 | + | s := NewScreen(4, 2) | |
| 13 | + | var rows [][]renderer.Cell | |
| 14 | + | s.OnLineEvicted = func(row []renderer.Cell) { rows = append(rows, row) } | |
| 15 | + | s.Write([]byte("aaaa\r\nbbbb\r\ncccc\r\ndddd")) | |
| 16 | + | if len(rows) != 2 { | |
| 17 | + | t.Fatalf("evicted %d rows, want 2", len(rows)) | |
| 18 | + | } | |
| 19 | + | if rows[0][0].Rune != 'a' || rows[1][0].Rune != 'b' { | |
| 20 | + | t.Fatalf("evicted rows = %q, %q", rows[0][0].Rune, rows[1][0].Rune) | |
| 21 | + | } | |
| 22 | + | rows[0][0].Rune = 'z' | |
| 23 | + | if rows[1][0].Rune != 'b' { | |
| 24 | + | t.Fatal("evicted rows alias each other") | |
| 25 | + | } | |
| 26 | + | } | |
| @@ -0,0 +1,26 @@ | |||||
| 1 | + | package vt | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "testing" | |||
| 5 | + | ||||
| 6 | + | renderer "github.com/bnema/vev-vt/core" | |||
| 7 | + | ) | |||
| 8 | + | ||||
| 9 | + | // OnLineEvicted callers may retain the slice: successive evictions must not | |||
| 10 | + | // share the Screen's internal scratch row. | |||
| 11 | + | func TestOnLineEvictedRowsAreIndependentCopies(t *testing.T) { | |||
| 12 | + | s := NewScreen(4, 2) | |||
| 13 | + | var rows [][]renderer.Cell | |||
| 14 | + | s.OnLineEvicted = func(row []renderer.Cell) { rows = append(rows, row) } | |||
| 15 | + | s.Write([]byte("aaaa\r\nbbbb\r\ncccc\r\ndddd")) | |||
| 16 | + | if len(rows) != 2 { | |||
| 17 | + | t.Fatalf("evicted %d rows, want 2", len(rows)) | |||
| 18 | + | } | |||
| 19 | + | if rows[0][0].Rune != 'a' || rows[1][0].Rune != 'b' { | |||
| 20 | + | t.Fatalf("evicted rows = %q, %q", rows[0][0].Rune, rows[1][0].Rune) | |||
| 21 | + | } | |||
| 22 | + | rows[0][0].Rune = 'z' | |||
| 23 | + | if rows[1][0].Rune != 'b' { | |||
| 24 | + | t.Fatal("evicted rows alias each other") | |||
| 25 | + | } | |||
| 26 | + | } | |||
M screen_graphics_test.go +106 -0
| @@ -4,6 +4,7 @@ import ( | |||
| 4 | 4 | "encoding/base64" | |
| 5 | 5 | "math" | |
| 6 | 6 | "strconv" | |
| 7 | + | "strings" | |
| 7 | 8 | "testing" | |
| 8 | 9 | ||
| 9 | 10 | "github.com/bnema/vev-vt/graphics" | |
| @@ -331,3 +332,108 @@ func TestScreenKittyPlacementScrollSkipsBottomEdgeOverflow(t *testing.T) { | |||
| 331 | 332 | placement = screen.GraphicsSnapshot().Placements()[0] | |
| 332 | 333 | require.Equal(t, nearLimit, placement.Destination()) | |
| 333 | 334 | } | |
| 335 | + | ||
| 336 | + | // Fragmented APCs reuse the Screen's escape buffer, so retained graphics state | |
| 337 | + | // (assets and pending chunked uploads) must not alias it. | |
| 338 | + | func TestScreenKittyFragmentedAPCsDoNotAliasEscapeBuffer(t *testing.T) { | |
| 339 | + | screen := NewScreen(16, 3) | |
| 340 | + | writeByteWise := func(data []byte) { | |
| 341 | + | for i := range data { | |
| 342 | + | screen.Write(data[i : i+1]) | |
| 343 | + | } | |
| 344 | + | } | |
| 345 | + | enc := func(b ...byte) string { return base64.StdEncoding.EncodeToString(b) } | |
| 346 | + | writeByteWise([]byte("\x1b_Ga=t,i=1,f=32,s=1,v=1;" + enc(1, 2, 3, 4) + "\x1b\\")) | |
| 347 | + | // Chunked upload interleaved with other escape sequences reusing the buffer. | |
| 348 | + | writeByteWise([]byte("\x1b_Ga=t,i=2,f=32,s=1,v=1,m=1;" + enc(9, 9, 9, 9)[:4] + "\x1b\\")) | |
| 349 | + | writeByteWise([]byte("\x1b[1mtext\x1b[0m")) | |
| 350 | + | writeByteWise([]byte("\x1b_Gm=0;" + enc(9, 9, 9, 9)[4:] + "\x1b\\")) | |
| 351 | + | ||
| 352 | + | snapshot := screen.GraphicsSnapshot() | |
| 353 | + | require.NotNil(t, snapshot) | |
| 354 | + | var got [][]byte | |
| 355 | + | for _, asset := range snapshot.Assets() { | |
| 356 | + | got = append(got, asset.Bytes()) | |
| 357 | + | } | |
| 358 | + | require.ElementsMatch(t, [][]byte{{1, 2, 3, 4}, {9, 9, 9, 9}}, got) | |
| 359 | + | } | |
| 360 | + | ||
| 361 | + | func screenAssetBytes(t *testing.T, screen *Screen) [][]byte { | |
| 362 | + | t.Helper() | |
| 363 | + | snapshot := screen.GraphicsSnapshot() | |
| 364 | + | require.NotNil(t, snapshot) | |
| 365 | + | var got [][]byte | |
| 366 | + | for _, asset := range snapshot.Assets() { | |
| 367 | + | got = append(got, asset.Bytes()) | |
| 368 | + | } | |
| 369 | + | return got | |
| 370 | + | } | |
| 371 | + | ||
| 372 | + | // A callback may call Write re-entrantly; the inner Write must not clobber the | |
| 373 | + | // reusable combine buffer the outer Write is still parsing. | |
| 374 | + | func TestScreenWriteReentrantFromCallbackKeepsOuterStreamIntact(t *testing.T) { | |
| 375 | + | t.Run("split CSI then DA reply writes text", func(t *testing.T) { | |
| 376 | + | screen := NewScreen(40, 3) | |
| 377 | + | fired := 0 | |
| 378 | + | screen.OnResponse = func([]byte) { | |
| 379 | + | fired++ | |
| 380 | + | // Leave a partial escape, then complete it: the second inner | |
| 381 | + | // Write takes the combine path while the outer Write is mid-parse. | |
| 382 | + | screen.Write([]byte("\x1b[")) | |
| 383 | + | screen.Write([]byte("1mZZZZZZZZZZ")) | |
| 384 | + | } | |
| 385 | + | screen.Write([]byte("\x1b[1")) | |
| 386 | + | screen.Write([]byte("m\x1b[cBCDEFGH")) | |
| 387 | + | require.Equal(t, 1, fired) | |
| 388 | + | require.Equal(t, "ZZZZZZZZZZBCDEFGH", strings.TrimRight(rowText(screen.Snapshot().Row(0)), " ")) | |
| 389 | + | }) | |
| 390 | + | t.Run("split APC response writes upload", func(t *testing.T) { | |
| 391 | + | screen := NewScreen(40, 3) | |
| 392 | + | inner := false | |
| 393 | + | screen.OnResponse = func([]byte) { | |
| 394 | + | if inner { | |
| 395 | + | return | |
| 396 | + | } | |
| 397 | + | inner = true | |
| 398 | + | for _, b := range []byte("\x1b_Ga=t,f=32,s=1,v=1;CQkJCQ==\x1b\\") { | |
| 399 | + | screen.Write([]byte{b}) | |
| 400 | + | } | |
| 401 | + | } | |
| 402 | + | screen.Write([]byte("\x1b_Ga=t,i=1,f=32,s=1,v=1;AQID")) | |
| 403 | + | screen.Write([]byte("BA==\x1b\\XYZ")) | |
| 404 | + | require.Equal(t, "XYZ", strings.TrimRight(rowText(screen.Snapshot().Row(0)), " ")) | |
| 405 | + | require.ElementsMatch(t, [][]byte{{1, 2, 3, 4}, {9, 9, 9, 9}}, screenAssetBytes(t, screen)) | |
| 406 | + | }) | |
| 407 | + | } | |
| 408 | + | ||
| 409 | + | // The Screen borrows the caller's Write buffer while parsing; nothing it | |
| 410 | + | // retains may alias that buffer once Write returns. | |
| 411 | + | func TestScreenKittyGraphicsDoesNotAliasCallerWriteBuffer(t *testing.T) { | |
| 412 | + | scribble := func(b []byte) { | |
| 413 | + | for i := range b { | |
| 414 | + | b[i] = 0xff | |
| 415 | + | } | |
| 416 | + | } | |
| 417 | + | t.Run("single-shot transmit and display", func(t *testing.T) { | |
| 418 | + | screen := NewScreen(16, 3) | |
| 419 | + | buf := []byte("\x1b_Ga=T,i=1,f=32,s=1,v=1;AQIDBA==\x1b\\") | |
| 420 | + | screen.Write(buf) | |
| 421 | + | scribble(buf) | |
| 422 | + | require.Equal(t, [][]byte{{1, 2, 3, 4}}, screenAssetBytes(t, screen)) | |
| 423 | + | }) | |
| 424 | + | t.Run("first and final chunk in one write", func(t *testing.T) { | |
| 425 | + | screen := NewScreen(16, 3) | |
| 426 | + | buf := []byte("\x1b_Ga=t,i=1,f=32,s=1,v=1,m=1;AQID\x1b\\\x1b_Gm=0;BA==\x1b\\") | |
| 427 | + | screen.Write(buf) | |
| 428 | + | scribble(buf) | |
| 429 | + | require.Equal(t, [][]byte{{1, 2, 3, 4}}, screenAssetBytes(t, screen)) | |
| 430 | + | }) | |
| 431 | + | t.Run("first chunk write then buffer reuse before final chunk", func(t *testing.T) { | |
| 432 | + | screen := NewScreen(16, 3) | |
| 433 | + | buf := []byte("\x1b_Ga=t,i=1,f=32,s=1,v=1,m=1;AQID\x1b\\") | |
| 434 | + | screen.Write(buf) | |
| 435 | + | scribble(buf) | |
| 436 | + | screen.Write([]byte("\x1b_Gm=0;BA==\x1b\\")) | |
| 437 | + | require.Equal(t, [][]byte{{1, 2, 3, 4}}, screenAssetBytes(t, screen)) | |
| 438 | + | }) | |
| 439 | + | } | |
| @@ -4,6 +4,7 @@ import ( | |||||
| 4 | "encoding/base64" | 4 | "encoding/base64" | ||
| 5 | "math" | 5 | "math" | ||
| 6 | "strconv" | 6 | "strconv" | ||
| 7 | + | "strings" | |||
| 7 | "testing" | 8 | "testing" | ||
| 8 | 9 | ||||
| 9 | "github.com/bnema/vev-vt/graphics" | 10 | "github.com/bnema/vev-vt/graphics" | ||
| @@ -331,3 +332,108 @@ func TestScreenKittyPlacementScrollSkipsBottomEdgeOverflow(t *testing.T) { | |||||
| 331 | placement = screen.GraphicsSnapshot().Placements()[0] | 332 | placement = screen.GraphicsSnapshot().Placements()[0] | ||
| 332 | require.Equal(t, nearLimit, placement.Destination()) | 333 | require.Equal(t, nearLimit, placement.Destination()) | ||
| 333 | } | 334 | } | ||
| 335 | + | ||||
| 336 | + | // Fragmented APCs reuse the Screen's escape buffer, so retained graphics state | |||
| 337 | + | // (assets and pending chunked uploads) must not alias it. | |||
| 338 | + | func TestScreenKittyFragmentedAPCsDoNotAliasEscapeBuffer(t *testing.T) { | |||
| 339 | + | screen := NewScreen(16, 3) | |||
| 340 | + | writeByteWise := func(data []byte) { | |||
| 341 | + | for i := range data { | |||
| 342 | + | screen.Write(data[i : i+1]) | |||
| 343 | + | } | |||
| 344 | + | } | |||
| 345 | + | enc := func(b ...byte) string { return base64.StdEncoding.EncodeToString(b) } | |||
| 346 | + | writeByteWise([]byte("\x1b_Ga=t,i=1,f=32,s=1,v=1;" + enc(1, 2, 3, 4) + "\x1b\\")) | |||
| 347 | + | // Chunked upload interleaved with other escape sequences reusing the buffer. | |||
| 348 | + | writeByteWise([]byte("\x1b_Ga=t,i=2,f=32,s=1,v=1,m=1;" + enc(9, 9, 9, 9)[:4] + "\x1b\\")) | |||
| 349 | + | writeByteWise([]byte("\x1b[1mtext\x1b[0m")) | |||
| 350 | + | writeByteWise([]byte("\x1b_Gm=0;" + enc(9, 9, 9, 9)[4:] + "\x1b\\")) | |||
| 351 | + | ||||
| 352 | + | snapshot := screen.GraphicsSnapshot() | |||
| 353 | + | require.NotNil(t, snapshot) | |||
| 354 | + | var got [][]byte | |||
| 355 | + | for _, asset := range snapshot.Assets() { | |||
| 356 | + | got = append(got, asset.Bytes()) | |||
| 357 | + | } | |||
| 358 | + | require.ElementsMatch(t, [][]byte{{1, 2, 3, 4}, {9, 9, 9, 9}}, got) | |||
| 359 | + | } | |||
| 360 | + | ||||
| 361 | + | func screenAssetBytes(t *testing.T, screen *Screen) [][]byte { | |||
| 362 | + | t.Helper() | |||
| 363 | + | snapshot := screen.GraphicsSnapshot() | |||
| 364 | + | require.NotNil(t, snapshot) | |||
| 365 | + | var got [][]byte | |||
| 366 | + | for _, asset := range snapshot.Assets() { | |||
| 367 | + | got = append(got, asset.Bytes()) | |||
| 368 | + | } | |||
| 369 | + | return got | |||
| 370 | + | } | |||
| 371 | + | ||||
| 372 | + | // A callback may call Write re-entrantly; the inner Write must not clobber the | |||
| 373 | + | // reusable combine buffer the outer Write is still parsing. | |||
| 374 | + | func TestScreenWriteReentrantFromCallbackKeepsOuterStreamIntact(t *testing.T) { | |||
| 375 | + | t.Run("split CSI then DA reply writes text", func(t *testing.T) { | |||
| 376 | + | screen := NewScreen(40, 3) | |||
| 377 | + | fired := 0 | |||
| 378 | + | screen.OnResponse = func([]byte) { | |||
| 379 | + | fired++ | |||
| 380 | + | // Leave a partial escape, then complete it: the second inner | |||
| 381 | + | // Write takes the combine path while the outer Write is mid-parse. | |||
| 382 | + | screen.Write([]byte("\x1b[")) | |||
| 383 | + | screen.Write([]byte("1mZZZZZZZZZZ")) | |||
| 384 | + | } | |||
| 385 | + | screen.Write([]byte("\x1b[1")) | |||
| 386 | + | screen.Write([]byte("m\x1b[cBCDEFGH")) | |||
| 387 | + | require.Equal(t, 1, fired) | |||
| 388 | + | require.Equal(t, "ZZZZZZZZZZBCDEFGH", strings.TrimRight(rowText(screen.Snapshot().Row(0)), " ")) | |||
| 389 | + | }) | |||
| 390 | + | t.Run("split APC response writes upload", func(t *testing.T) { | |||
| 391 | + | screen := NewScreen(40, 3) | |||
| 392 | + | inner := false | |||
| 393 | + | screen.OnResponse = func([]byte) { | |||
| 394 | + | if inner { | |||
| 395 | + | return | |||
| 396 | + | } | |||
| 397 | + | inner = true | |||
| 398 | + | for _, b := range []byte("\x1b_Ga=t,f=32,s=1,v=1;CQkJCQ==\x1b\\") { | |||
| 399 | + | screen.Write([]byte{b}) | |||
| 400 | + | } | |||
| 401 | + | } | |||
| 402 | + | screen.Write([]byte("\x1b_Ga=t,i=1,f=32,s=1,v=1;AQID")) | |||
| 403 | + | screen.Write([]byte("BA==\x1b\\XYZ")) | |||
| 404 | + | require.Equal(t, "XYZ", strings.TrimRight(rowText(screen.Snapshot().Row(0)), " ")) | |||
| 405 | + | require.ElementsMatch(t, [][]byte{{1, 2, 3, 4}, {9, 9, 9, 9}}, screenAssetBytes(t, screen)) | |||
| 406 | + | }) | |||
| 407 | + | } | |||
| 408 | + | ||||
| 409 | + | // The Screen borrows the caller's Write buffer while parsing; nothing it | |||
| 410 | + | // retains may alias that buffer once Write returns. | |||
| 411 | + | func TestScreenKittyGraphicsDoesNotAliasCallerWriteBuffer(t *testing.T) { | |||
| 412 | + | scribble := func(b []byte) { | |||
| 413 | + | for i := range b { | |||
| 414 | + | b[i] = 0xff | |||
| 415 | + | } | |||
| 416 | + | } | |||
| 417 | + | t.Run("single-shot transmit and display", func(t *testing.T) { | |||
| 418 | + | screen := NewScreen(16, 3) | |||
| 419 | + | buf := []byte("\x1b_Ga=T,i=1,f=32,s=1,v=1;AQIDBA==\x1b\\") | |||
| 420 | + | screen.Write(buf) | |||
| 421 | + | scribble(buf) | |||
| 422 | + | require.Equal(t, [][]byte{{1, 2, 3, 4}}, screenAssetBytes(t, screen)) | |||
| 423 | + | }) | |||
| 424 | + | t.Run("first and final chunk in one write", func(t *testing.T) { | |||
| 425 | + | screen := NewScreen(16, 3) | |||
| 426 | + | buf := []byte("\x1b_Ga=t,i=1,f=32,s=1,v=1,m=1;AQID\x1b\\\x1b_Gm=0;BA==\x1b\\") | |||
| 427 | + | screen.Write(buf) | |||
| 428 | + | scribble(buf) | |||
| 429 | + | require.Equal(t, [][]byte{{1, 2, 3, 4}}, screenAssetBytes(t, screen)) | |||
| 430 | + | }) | |||
| 431 | + | t.Run("first chunk write then buffer reuse before final chunk", func(t *testing.T) { | |||
| 432 | + | screen := NewScreen(16, 3) | |||
| 433 | + | buf := []byte("\x1b_Ga=t,i=1,f=32,s=1,v=1,m=1;AQID\x1b\\") | |||
| 434 | + | screen.Write(buf) | |||
| 435 | + | scribble(buf) | |||
| 436 | + | screen.Write([]byte("\x1b_Gm=0;BA==\x1b\\")) | |||
| 437 | + | require.Equal(t, [][]byte{{1, 2, 3, 4}}, screenAssetBytes(t, screen)) | |||
| 438 | + | }) | |||
| 439 | + | } | |||
M screen_scroll.go +14 -1
| @@ -103,10 +103,23 @@ func (s *Screen) emitLineEvicted(top, n int) { | |||
| 103 | 103 | if s.alternate != nil || top != 0 { | |
| 104 | 104 | return | |
| 105 | 105 | } | |
| 106 | + | // Without a history or an observer nothing consumes the evicted rows, so | |
| 107 | + | // skip materialising them. | |
| 108 | + | if s.history == nil && s.OnLineEvicted == nil { | |
| 109 | + | return | |
| 110 | + | } | |
| 106 | 111 | // Read boundaries and IDs before the caller rotates the frame: a soft link | |
| 107 | 112 | // belongs to the row it follows, and rotation reassigns row indices. | |
| 108 | 113 | for y := top; y < top+n; y++ { | |
| 109 | - | s.recordEvicted(s.frame.Row(y), s.buffer.bound(y), s.buffer.rowIDs[y]) | |
| 114 | + | // The scratch row is only valid for this call: History copies appended | |
| 115 | + | // rows and OnLineEvicted receives its own copy (see recordEvicted). | |
| 116 | + | row := s.evictScratch[:0] | |
| 117 | + | for x := range s.frame.Width { | |
| 118 | + | row = append(row, s.frame.Cell(x, y)) | |
| 119 | + | } | |
| 120 | + | s.evictScratch = row | |
| 121 | + | s.recordEvicted(row, s.buffer.bound(y), s.buffer.rowIDs[y]) | |
| 122 | + | clear(s.evictScratch) | |
| 110 | 123 | } | |
| 111 | 124 | } | |
| 112 | 125 | ||
| @@ -103,10 +103,23 @@ func (s *Screen) emitLineEvicted(top, n int) { | |||||
| 103 | if s.alternate != nil || top != 0 { | 103 | if s.alternate != nil || top != 0 { | ||
| 104 | return | 104 | return | ||
| 105 | } | 105 | } | ||
| 106 | + | // Without a history or an observer nothing consumes the evicted rows, so | |||
| 107 | + | // skip materialising them. | |||
| 108 | + | if s.history == nil && s.OnLineEvicted == nil { | |||
| 109 | + | return | |||
| 110 | + | } | |||
| 106 | // Read boundaries and IDs before the caller rotates the frame: a soft link | 111 | // Read boundaries and IDs before the caller rotates the frame: a soft link | ||
| 107 | // belongs to the row it follows, and rotation reassigns row indices. | 112 | // belongs to the row it follows, and rotation reassigns row indices. | ||
| 108 | for y := top; y < top+n; y++ { | 113 | for y := top; y < top+n; y++ { | ||
| 109 | - | s.recordEvicted(s.frame.Row(y), s.buffer.bound(y), s.buffer.rowIDs[y]) | 114 | + | // The scratch row is only valid for this call: History copies appended |
| 115 | + | // rows and OnLineEvicted receives its own copy (see recordEvicted). | |||
| 116 | + | row := s.evictScratch[:0] | |||
| 117 | + | for x := range s.frame.Width { | |||
| 118 | + | row = append(row, s.frame.Cell(x, y)) | |||
| 119 | + | } | |||
| 120 | + | s.evictScratch = row | |||
| 121 | + | s.recordEvicted(row, s.buffer.bound(y), s.buffer.rowIDs[y]) | |||
| 122 | + | clear(s.evictScratch) | |||
| 110 | } | 123 | } | ||
| 111 | } | 124 | } | ||
| 112 | 125 | ||||