fix(protocol): harden moved boundary helpers
- commit
- 6632680c642e33d4ed45586399e796c5bf0dcbf1
- author
- brice · · 2026-08-28 08:31 UTC
- committer
- brice · · 2026-08-28 14:56 UTC
- parent
- edcc67a
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
11 files · +39 -18
M internal/adapters/sessionwire/server.go +1 -4
| @@ -2,7 +2,6 @@ | |||
| 2 | 2 | package sessionwire | |
| 3 | 3 | ||
| 4 | 4 | import ( | |
| 5 | - | "encoding/binary" | |
| 6 | 5 | "errors" | |
| 7 | 6 | ||
| 8 | 7 | "github.com/bnema/vev/internal/ports" | |
| @@ -51,9 +50,7 @@ func (c *serverConnection) ReceiveClient() (protocol.ClientMessage, error) { | |||
| 51 | 50 | case wire.MsgCommand: | |
| 52 | 51 | failure.Kind = protocol.DecodeMessageCommand | |
| 53 | 52 | failure.Version, _ = wire.PeekCommandVersion(frame.Payload) | |
| 54 | - | if len(frame.Payload) >= 10 { | |
| 55 | - | failure.RequestID = binary.BigEndian.Uint64(frame.Payload[2:10]) | |
| 56 | - | } | |
| 53 | + | failure.RequestID, _ = wire.PeekCommandRequestID(frame.Payload) | |
| 57 | 54 | case wire.MsgKill: | |
| 58 | 55 | failure.Kind = protocol.DecodeMessageKill | |
| 59 | 56 | case wire.MsgRemotePreviewRequest: | |
| @@ -2,7 +2,6 @@ | |||||
| 2 | package sessionwire | 2 | package sessionwire | ||
| 3 | 3 | ||||
| 4 | import ( | 4 | import ( | ||
| 5 | - | "encoding/binary" | |||
| 6 | "errors" | 5 | "errors" | ||
| 7 | 6 | ||||
| 8 | "github.com/bnema/vev/internal/ports" | 7 | "github.com/bnema/vev/internal/ports" | ||
| @@ -51,9 +50,7 @@ func (c *serverConnection) ReceiveClient() (protocol.ClientMessage, error) { | |||||
| 51 | case wire.MsgCommand: | 50 | case wire.MsgCommand: | ||
| 52 | failure.Kind = protocol.DecodeMessageCommand | 51 | failure.Kind = protocol.DecodeMessageCommand | ||
| 53 | failure.Version, _ = wire.PeekCommandVersion(frame.Payload) | 52 | failure.Version, _ = wire.PeekCommandVersion(frame.Payload) | ||
| 54 | - | if len(frame.Payload) >= 10 { | 53 | + | failure.RequestID, _ = wire.PeekCommandRequestID(frame.Payload) |
| 55 | - | failure.RequestID = binary.BigEndian.Uint64(frame.Payload[2:10]) | |||
| 56 | - | } | |||
| 57 | case wire.MsgKill: | 54 | case wire.MsgKill: | ||
| 58 | failure.Kind = protocol.DecodeMessageKill | 55 | failure.Kind = protocol.DecodeMessageKill | ||
| 59 | case wire.MsgRemotePreviewRequest: | 56 | case wire.MsgRemotePreviewRequest: | ||
M internal/app/spawn_test.go +4 -9
| @@ -14,16 +14,11 @@ import ( | |||
| 14 | 14 | "time" | |
| 15 | 15 | ||
| 16 | 16 | "github.com/bnema/vev/internal/protocol/wire" | |
| 17 | + | wiremocks "github.com/bnema/vev/internal/protocol/wire/mocks" | |
| 17 | 18 | ) | |
| 18 | 19 | ||
| 19 | 20 | var errDialFailed = errors.New("dial failed") | |
| 20 | 21 | ||
| 21 | - | type spawnTestTransport struct{} | |
| 22 | - | ||
| 23 | - | func (spawnTestTransport) Send(wire.Frame) error { return nil } | |
| 24 | - | func (spawnTestTransport) Recv() (wire.Frame, error) { return wire.Frame{}, nil } | |
| 25 | - | func (spawnTestTransport) Close() error { return nil } | |
| 26 | - | ||
| 27 | 22 | const ( | |
| 28 | 23 | spawnTestChildFileEnv = "VEV_SPAWN_TEST_CHILD_FILE" | |
| 29 | 24 | spawnTestLauncherFileEnv = "VEV_SPAWN_TEST_LAUNCHER_FILE" | |
| @@ -310,7 +305,7 @@ func TestRetryAttemptsClampsNonPositiveInitialDelay(t *testing.T) { | |||
| 310 | 305 | } | |
| 311 | 306 | ||
| 312 | 307 | func TestRetryDialSucceedsAfterTransientFailures(t *testing.T) { | |
| 313 | - | want := spawnTestTransport{} | |
| 308 | + | want := wiremocks.NewMockTransport(t) | |
| 314 | 309 | var calls atomic.Int32 | |
| 315 | 310 | dial := func(context.Context, string) (wire.Transport, error) { | |
| 316 | 311 | if calls.Add(1) <= 3 { | |
| @@ -398,7 +393,7 @@ func TestEnsureDaemonDoesNotSpawnWhenContextCancelsAfterFailedDial(t *testing.T) | |||
| 398 | 393 | ||
| 399 | 394 | func TestEnsureDaemonSpawnsThenDials(t *testing.T) { | |
| 400 | 395 | dir := filepath.Join(t.TempDir(), "vev") | |
| 401 | - | want := spawnTestTransport{} | |
| 396 | + | want := wiremocks.NewMockTransport(t) | |
| 402 | 397 | var dialCalls, spawnCalls atomic.Int32 | |
| 403 | 398 | dial := func(context.Context, string) (wire.Transport, error) { | |
| 404 | 399 | // First (pre-spawn) dial fails; the post-spawn retry succeeds. | |
| @@ -429,7 +424,7 @@ func TestEnsureDaemonSpawnsThenDials(t *testing.T) { | |||
| 429 | 424 | } | |
| 430 | 425 | ||
| 431 | 426 | func TestEnsureDaemonReturnsExistingDaemon(t *testing.T) { | |
| 432 | - | want := spawnTestTransport{} | |
| 427 | + | want := wiremocks.NewMockTransport(t) | |
| 433 | 428 | var spawnCalls atomic.Int32 | |
| 434 | 429 | dial := func(context.Context, string) (wire.Transport, error) { return want, nil } | |
| 435 | 430 | spawn := func() error { spawnCalls.Add(1); return nil } | |
| @@ -14,16 +14,11 @@ import ( | |||||
| 14 | "time" | 14 | "time" | ||
| 15 | 15 | ||||
| 16 | "github.com/bnema/vev/internal/protocol/wire" | 16 | "github.com/bnema/vev/internal/protocol/wire" | ||
| 17 | + | wiremocks "github.com/bnema/vev/internal/protocol/wire/mocks" | |||
| 17 | ) | 18 | ) | ||
| 18 | 19 | ||||
| 19 | var errDialFailed = errors.New("dial failed") | 20 | var errDialFailed = errors.New("dial failed") | ||
| 20 | 21 | ||||
| 21 | - | type spawnTestTransport struct{} | |||
| 22 | - | ||||
| 23 | - | func (spawnTestTransport) Send(wire.Frame) error { return nil } | |||
| 24 | - | func (spawnTestTransport) Recv() (wire.Frame, error) { return wire.Frame{}, nil } | |||
| 25 | - | func (spawnTestTransport) Close() error { return nil } | |||
| 26 | - | ||||
| 27 | const ( | 22 | const ( | ||
| 28 | spawnTestChildFileEnv = "VEV_SPAWN_TEST_CHILD_FILE" | 23 | spawnTestChildFileEnv = "VEV_SPAWN_TEST_CHILD_FILE" | ||
| 29 | spawnTestLauncherFileEnv = "VEV_SPAWN_TEST_LAUNCHER_FILE" | 24 | spawnTestLauncherFileEnv = "VEV_SPAWN_TEST_LAUNCHER_FILE" | ||
| @@ -310,7 +305,7 @@ func TestRetryAttemptsClampsNonPositiveInitialDelay(t *testing.T) { | |||||
| 310 | } | 305 | } | ||
| 311 | 306 | ||||
| 312 | func TestRetryDialSucceedsAfterTransientFailures(t *testing.T) { | 307 | func TestRetryDialSucceedsAfterTransientFailures(t *testing.T) { | ||
| 313 | - | want := spawnTestTransport{} | 308 | + | want := wiremocks.NewMockTransport(t) |
| 314 | var calls atomic.Int32 | 309 | var calls atomic.Int32 | ||
| 315 | dial := func(context.Context, string) (wire.Transport, error) { | 310 | dial := func(context.Context, string) (wire.Transport, error) { | ||
| 316 | if calls.Add(1) <= 3 { | 311 | if calls.Add(1) <= 3 { | ||
| @@ -398,7 +393,7 @@ func TestEnsureDaemonDoesNotSpawnWhenContextCancelsAfterFailedDial(t *testing.T) | |||||
| 398 | 393 | ||||
| 399 | func TestEnsureDaemonSpawnsThenDials(t *testing.T) { | 394 | func TestEnsureDaemonSpawnsThenDials(t *testing.T) { | ||
| 400 | dir := filepath.Join(t.TempDir(), "vev") | 395 | dir := filepath.Join(t.TempDir(), "vev") | ||
| 401 | - | want := spawnTestTransport{} | 396 | + | want := wiremocks.NewMockTransport(t) |
| 402 | var dialCalls, spawnCalls atomic.Int32 | 397 | var dialCalls, spawnCalls atomic.Int32 | ||
| 403 | dial := func(context.Context, string) (wire.Transport, error) { | 398 | dial := func(context.Context, string) (wire.Transport, error) { | ||
| 404 | // First (pre-spawn) dial fails; the post-spawn retry succeeds. | 399 | // First (pre-spawn) dial fails; the post-spawn retry succeeds. | ||
| @@ -429,7 +424,7 @@ func TestEnsureDaemonSpawnsThenDials(t *testing.T) { | |||||
| 429 | } | 424 | } | ||
| 430 | 425 | ||||
| 431 | func TestEnsureDaemonReturnsExistingDaemon(t *testing.T) { | 426 | func TestEnsureDaemonReturnsExistingDaemon(t *testing.T) { | ||
| 432 | - | want := spawnTestTransport{} | 427 | + | want := wiremocks.NewMockTransport(t) |
| 433 | var spawnCalls atomic.Int32 | 428 | var spawnCalls atomic.Int32 | ||
| 434 | dial := func(context.Context, string) (wire.Transport, error) { return want, nil } | 429 | dial := func(context.Context, string) (wire.Transport, error) { return want, nil } | ||
| 435 | spawn := func() error { spawnCalls.Add(1); return nil } | 430 | spawn := func() error { spawnCalls.Add(1); return nil } | ||
M internal/domain/terminalcap/capabilities_test.go +2 -0
| @@ -16,6 +16,8 @@ func TestDetect(t *testing.T) { | |||
| 16 | 16 | wantGraphics bool | |
| 17 | 17 | }{ | |
| 18 | 18 | {name: "declared truecolor", env: []string{"TERM=xterm-256color", "COLORTERM=truecolor"}, wantMode: TrueColor, wantSource: SourceDeclared}, | |
| 19 | + | {name: "declared 24bit", env: []string{"TERM=xterm-256color", "COLORTERM=24bit"}, wantMode: TrueColor, wantSource: SourceDeclared}, | |
| 20 | + | {name: "direct terminfo entry", env: []string{"TERM=foot-direct"}, wantMode: TrueColor, wantSource: SourceDeclared}, | |
| 19 | 21 | {name: "kitty signals infer truecolor", env: []string{"TERM=xterm-kitty", "KITTY_WINDOW_ID=1"}, wantMode: TrueColor, wantSource: SourceHeuristic, wantApp: ApplicationKitty}, | |
| 20 | 22 | {name: "kitty environment with declared truecolor remains color-only", env: []string{"TERM=xterm-kitty", "COLORTERM=truecolor", "KITTY_WINDOW_ID=1"}, wantMode: TrueColor, wantSource: SourceDeclared, wantApp: ApplicationKitty}, | |
| 21 | 23 | {name: "256 color terminal remains a constrained attachment", env: []string{"TERM=xterm-256color"}, wantMode: Indexed256, wantSource: SourceDeclared}, | |
| @@ -16,6 +16,8 @@ func TestDetect(t *testing.T) { | |||||
| 16 | wantGraphics bool | 16 | wantGraphics bool | ||
| 17 | }{ | 17 | }{ | ||
| 18 | {name: "declared truecolor", env: []string{"TERM=xterm-256color", "COLORTERM=truecolor"}, wantMode: TrueColor, wantSource: SourceDeclared}, | 18 | {name: "declared truecolor", env: []string{"TERM=xterm-256color", "COLORTERM=truecolor"}, wantMode: TrueColor, wantSource: SourceDeclared}, | ||
| 19 | + | {name: "declared 24bit", env: []string{"TERM=xterm-256color", "COLORTERM=24bit"}, wantMode: TrueColor, wantSource: SourceDeclared}, | |||
| 20 | + | {name: "direct terminfo entry", env: []string{"TERM=foot-direct"}, wantMode: TrueColor, wantSource: SourceDeclared}, | |||
| 19 | {name: "kitty signals infer truecolor", env: []string{"TERM=xterm-kitty", "KITTY_WINDOW_ID=1"}, wantMode: TrueColor, wantSource: SourceHeuristic, wantApp: ApplicationKitty}, | 21 | {name: "kitty signals infer truecolor", env: []string{"TERM=xterm-kitty", "KITTY_WINDOW_ID=1"}, wantMode: TrueColor, wantSource: SourceHeuristic, wantApp: ApplicationKitty}, | ||
| 20 | {name: "kitty environment with declared truecolor remains color-only", env: []string{"TERM=xterm-kitty", "COLORTERM=truecolor", "KITTY_WINDOW_ID=1"}, wantMode: TrueColor, wantSource: SourceDeclared, wantApp: ApplicationKitty}, | 22 | {name: "kitty environment with declared truecolor remains color-only", env: []string{"TERM=xterm-kitty", "COLORTERM=truecolor", "KITTY_WINDOW_ID=1"}, wantMode: TrueColor, wantSource: SourceDeclared, wantApp: ApplicationKitty}, | ||
| 21 | {name: "256 color terminal remains a constrained attachment", env: []string{"TERM=xterm-256color"}, wantMode: Indexed256, wantSource: SourceDeclared}, | 23 | {name: "256 color terminal remains a constrained attachment", env: []string{"TERM=xterm-256color"}, wantMode: Indexed256, wantSource: SourceDeclared}, | ||
M internal/protocol/catalogue/catalogue.go +1 -1
| @@ -165,7 +165,7 @@ func ValidateRemoteCatalog(c RemoteCatalog) error { | |||
| 165 | 165 | if session.Tabs == nil { | |
| 166 | 166 | return fmt.Errorf("%w: missing tab list", ErrInvalidRemoteCatalog) | |
| 167 | 167 | } | |
| 168 | - | tabs := CatalogTabs(session) | |
| 168 | + | tabs := session.Tabs | |
| 169 | 169 | if len(tabs) == 0 { | |
| 170 | 170 | if session.ActiveTabID != "" { | |
| 171 | 171 | return fmt.Errorf("%w: active tab is absent", ErrInvalidRemoteCatalog) | |
| @@ -165,7 +165,7 @@ func ValidateRemoteCatalog(c RemoteCatalog) error { | |||||
| 165 | if session.Tabs == nil { | 165 | if session.Tabs == nil { | ||
| 166 | return fmt.Errorf("%w: missing tab list", ErrInvalidRemoteCatalog) | 166 | return fmt.Errorf("%w: missing tab list", ErrInvalidRemoteCatalog) | ||
| 167 | } | 167 | } | ||
| 168 | - | tabs := CatalogTabs(session) | 168 | + | tabs := session.Tabs |
| 169 | if len(tabs) == 0 { | 169 | if len(tabs) == 0 { | ||
| 170 | if session.ActiveTabID != "" { | 170 | if session.ActiveTabID != "" { | ||
| 171 | return fmt.Errorf("%w: active tab is absent", ErrInvalidRemoteCatalog) | 171 | return fmt.Errorf("%w: active tab is absent", ErrInvalidRemoteCatalog) | ||
M internal/protocol/session.go +1 -1
| @@ -290,7 +290,7 @@ func ValidateHello(h Hello) error { | |||
| 290 | 290 | if err := validateHelloNavigation(h); err != nil { | |
| 291 | 291 | return fmt.Errorf("%w: navigation: %w", ErrInvalidHello, err) | |
| 292 | 292 | } | |
| 293 | - | if len(h.Name) > math.MaxUint16 || len(h.TermEnv) > math.MaxUint16 || len(h.Cwd) > math.MaxUint16 || len(h.Env) > math.MaxUint32 { | |
| 293 | + | if len(h.Name) > math.MaxUint16 || len(h.TermEnv) > math.MaxUint16 || len(h.Cwd) > math.MaxUint16 || uint64(len(h.Env)) > math.MaxUint32 { | |
| 294 | 294 | return ErrInvalidHello | |
| 295 | 295 | } | |
| 296 | 296 | for _, entry := range h.Env { | |
| @@ -290,7 +290,7 @@ func ValidateHello(h Hello) error { | |||||
| 290 | if err := validateHelloNavigation(h); err != nil { | 290 | if err := validateHelloNavigation(h); err != nil { | ||
| 291 | return fmt.Errorf("%w: navigation: %w", ErrInvalidHello, err) | 291 | return fmt.Errorf("%w: navigation: %w", ErrInvalidHello, err) | ||
| 292 | } | 292 | } | ||
| 293 | - | if len(h.Name) > math.MaxUint16 || len(h.TermEnv) > math.MaxUint16 || len(h.Cwd) > math.MaxUint16 || len(h.Env) > math.MaxUint32 { | 293 | + | if len(h.Name) > math.MaxUint16 || len(h.TermEnv) > math.MaxUint16 || len(h.Cwd) > math.MaxUint16 || uint64(len(h.Env)) > math.MaxUint32 { |
| 294 | return ErrInvalidHello | 294 | return ErrInvalidHello | ||
| 295 | } | 295 | } | ||
| 296 | for _, entry := range h.Env { | 296 | for _, entry := range h.Env { | ||
M internal/protocol/wire/remote_preview_test.go +2 -0
| @@ -6,6 +6,7 @@ import ( | |||
| 6 | 6 | renderer "github.com/bnema/vev-vt" | |
| 7 | 7 | "github.com/bnema/vev/internal/domain" | |
| 8 | 8 | "github.com/bnema/vev/internal/protocol" | |
| 9 | + | "github.com/stretchr/testify/require" | |
| 9 | 10 | ) | |
| 10 | 11 | ||
| 11 | 12 | func previewTargetForTest() domain.RemoteSessionTarget { | |
| @@ -41,6 +42,7 @@ func TestRemotePreviewCodecPreservesWideStyledCells(t *testing.T) { | |||
| 41 | 42 | func TestRemotePreviewCodecRejectsMalformedBoundsAndGarbage(t *testing.T) { | |
| 42 | 43 | preview := protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewOK, LifecycleID: previewTargetForTest().LifecycleID, TabID: "tab-1", Revision: 1, Width: 1, Height: 1, Cells: []renderer.Cell{{Rune: 'x'}}} | |
| 43 | 44 | payload := MarshalRemotePreview(preview) | |
| 45 | + | require.NotNil(t, payload) | |
| 44 | 46 | for i := 0; i < len(payload); i++ { | |
| 45 | 47 | if _, err := UnmarshalRemotePreview(payload[:i]); err == nil { | |
| 46 | 48 | t.Fatalf("prefix %d unexpectedly decoded", i) | |
| @@ -6,6 +6,7 @@ import ( | |||||
| 6 | renderer "github.com/bnema/vev-vt" | 6 | renderer "github.com/bnema/vev-vt" | ||
| 7 | "github.com/bnema/vev/internal/domain" | 7 | "github.com/bnema/vev/internal/domain" | ||
| 8 | "github.com/bnema/vev/internal/protocol" | 8 | "github.com/bnema/vev/internal/protocol" | ||
| 9 | + | "github.com/stretchr/testify/require" | |||
| 9 | ) | 10 | ) | ||
| 10 | 11 | ||||
| 11 | func previewTargetForTest() domain.RemoteSessionTarget { | 12 | func previewTargetForTest() domain.RemoteSessionTarget { | ||
| @@ -41,6 +42,7 @@ func TestRemotePreviewCodecPreservesWideStyledCells(t *testing.T) { | |||||
| 41 | func TestRemotePreviewCodecRejectsMalformedBoundsAndGarbage(t *testing.T) { | 42 | func TestRemotePreviewCodecRejectsMalformedBoundsAndGarbage(t *testing.T) { | ||
| 42 | preview := protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewOK, LifecycleID: previewTargetForTest().LifecycleID, TabID: "tab-1", Revision: 1, Width: 1, Height: 1, Cells: []renderer.Cell{{Rune: 'x'}}} | 43 | preview := protocol.RemotePreview{Version: protocol.RemotePreviewSchemaVersion, Status: protocol.RemotePreviewOK, LifecycleID: previewTargetForTest().LifecycleID, TabID: "tab-1", Revision: 1, Width: 1, Height: 1, Cells: []renderer.Cell{{Rune: 'x'}}} | ||
| 43 | payload := MarshalRemotePreview(preview) | 44 | payload := MarshalRemotePreview(preview) | ||
| 45 | + | require.NotNil(t, payload) | |||
| 44 | for i := 0; i < len(payload); i++ { | 46 | for i := 0; i < len(payload); i++ { | ||
| 45 | if _, err := UnmarshalRemotePreview(payload[:i]); err == nil { | 47 | if _, err := UnmarshalRemotePreview(payload[:i]); err == nil { | ||
| 46 | t.Fatalf("prefix %d unexpectedly decoded", i) | 48 | t.Fatalf("prefix %d unexpectedly decoded", i) | ||
M internal/protocol/wire/routes_test.go +3 -2
| @@ -226,14 +226,15 @@ func TestRouteLabelBoundsMatchRemoteDisplayOrigin(t *testing.T) { | |||
| 226 | 226 | func TestRouteLabelsRejectTerminalUnsafeText(t *testing.T) { | |
| 227 | 227 | values := []string{"line\nfeed", "\u2028line-separator", "\u2029paragraph-separator", "\x1b[31mred", "\u202eoverride", string([]byte{0xff, 0xfe})} | |
| 228 | 228 | for _, value := range values { | |
| 229 | - | t.Run("name/"+value, func(t *testing.T) { | |
| 229 | + | encodedName := hex.EncodeToString([]byte(value)) | |
| 230 | + | t.Run("name/"+encodedName, func(t *testing.T) { | |
| 230 | 231 | _, err := MarshalRecentRouteSnapshot(protocol.RecentRouteSnapshot{ | |
| 231 | 232 | Generation: 1, | |
| 232 | 233 | Entries: []protocol.RecentRouteEntry{{Key: 1, Generation: 1, Target: protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{1}, SessionName: value}, Name: value, Kind: protocol.RouteKindLocal}}, | |
| 233 | 234 | }) | |
| 234 | 235 | require.Error(t, err) | |
| 235 | 236 | }) | |
| 236 | - | t.Run("host/"+value, func(t *testing.T) { | |
| 237 | + | t.Run("host/"+encodedName, func(t *testing.T) { | |
| 237 | 238 | _, err := MarshalRecentRouteSnapshot(protocol.RecentRouteSnapshot{ | |
| 238 | 239 | Generation: 1, | |
| 239 | 240 | Entries: []protocol.RecentRouteEntry{{Key: 1, Generation: 1, Target: testExactTarget(), Name: "work", HostLabel: value, Kind: protocol.RouteKindRemote}}, | |
| @@ -226,14 +226,15 @@ func TestRouteLabelBoundsMatchRemoteDisplayOrigin(t *testing.T) { | |||||
| 226 | func TestRouteLabelsRejectTerminalUnsafeText(t *testing.T) { | 226 | func TestRouteLabelsRejectTerminalUnsafeText(t *testing.T) { | ||
| 227 | values := []string{"line\nfeed", "\u2028line-separator", "\u2029paragraph-separator", "\x1b[31mred", "\u202eoverride", string([]byte{0xff, 0xfe})} | 227 | values := []string{"line\nfeed", "\u2028line-separator", "\u2029paragraph-separator", "\x1b[31mred", "\u202eoverride", string([]byte{0xff, 0xfe})} | ||
| 228 | for _, value := range values { | 228 | for _, value := range values { | ||
| 229 | - | t.Run("name/"+value, func(t *testing.T) { | 229 | + | encodedName := hex.EncodeToString([]byte(value)) |
| 230 | + | t.Run("name/"+encodedName, func(t *testing.T) { | |||
| 230 | _, err := MarshalRecentRouteSnapshot(protocol.RecentRouteSnapshot{ | 231 | _, err := MarshalRecentRouteSnapshot(protocol.RecentRouteSnapshot{ | ||
| 231 | Generation: 1, | 232 | Generation: 1, | ||
| 232 | Entries: []protocol.RecentRouteEntry{{Key: 1, Generation: 1, Target: protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{1}, SessionName: value}, Name: value, Kind: protocol.RouteKindLocal}}, | 233 | Entries: []protocol.RecentRouteEntry{{Key: 1, Generation: 1, Target: protocol.ExactSessionTarget{LifecycleID: domain.SessionLifecycleID{1}, SessionName: value}, Name: value, Kind: protocol.RouteKindLocal}}, | ||
| 233 | }) | 234 | }) | ||
| 234 | require.Error(t, err) | 235 | require.Error(t, err) | ||
| 235 | }) | 236 | }) | ||
| 236 | - | t.Run("host/"+value, func(t *testing.T) { | 237 | + | t.Run("host/"+encodedName, func(t *testing.T) { |
| 237 | _, err := MarshalRecentRouteSnapshot(protocol.RecentRouteSnapshot{ | 238 | _, err := MarshalRecentRouteSnapshot(protocol.RecentRouteSnapshot{ | ||
| 238 | Generation: 1, | 239 | Generation: 1, | ||
| 239 | Entries: []protocol.RecentRouteEntry{{Key: 1, Generation: 1, Target: testExactTarget(), Name: "work", HostLabel: value, Kind: protocol.RouteKindRemote}}, | 240 | Entries: []protocol.RecentRouteEntry{{Key: 1, Generation: 1, Target: testExactTarget(), Name: "work", HostLabel: value, Kind: protocol.RouteKindRemote}}, | ||
M internal/protocol/wire/wire.go +7 -0
| @@ -1374,6 +1374,10 @@ func MarshalSessions(m protocol.Sessions) []byte { | |||
| 1374 | 1374 | return w.b | |
| 1375 | 1375 | } | |
| 1376 | 1376 | ||
| 1377 | + | // A session record needs two empty length-prefixed strings, two flags, a tab | |
| 1378 | + | // count, and a state byte before either string contributes data. | |
| 1379 | + | const sessionRecordMinLen = 9 | |
| 1380 | + | ||
| 1377 | 1381 | // UnmarshalSessions decodes a Sessions message payload. | |
| 1378 | 1382 | func UnmarshalSessions(b []byte) (protocol.Sessions, error) { | |
| 1379 | 1383 | r := payloadReader{b: b} | |
| @@ -1381,6 +1385,9 @@ func UnmarshalSessions(b []byte) (protocol.Sessions, error) { | |||
| 1381 | 1385 | if err != nil { | |
| 1382 | 1386 | return protocol.Sessions{}, err | |
| 1383 | 1387 | } | |
| 1388 | + | if int(count) > len(r.b)/sessionRecordMinLen { | |
| 1389 | + | return protocol.Sessions{}, errShortPayload | |
| 1390 | + | } | |
| 1384 | 1391 | sessions := make([]protocol.SessionInfo, 0, count) | |
| 1385 | 1392 | for range int(count) { | |
| 1386 | 1393 | var s protocol.SessionInfo | |
| @@ -1374,6 +1374,10 @@ func MarshalSessions(m protocol.Sessions) []byte { | |||||
| 1374 | return w.b | 1374 | return w.b | ||
| 1375 | } | 1375 | } | ||
| 1376 | 1376 | ||||
| 1377 | + | // A session record needs two empty length-prefixed strings, two flags, a tab | |||
| 1378 | + | // count, and a state byte before either string contributes data. | |||
| 1379 | + | const sessionRecordMinLen = 9 | |||
| 1380 | + | ||||
| 1377 | // UnmarshalSessions decodes a Sessions message payload. | 1381 | // UnmarshalSessions decodes a Sessions message payload. | ||
| 1378 | func UnmarshalSessions(b []byte) (protocol.Sessions, error) { | 1382 | func UnmarshalSessions(b []byte) (protocol.Sessions, error) { | ||
| 1379 | r := payloadReader{b: b} | 1383 | r := payloadReader{b: b} | ||
| @@ -1381,6 +1385,9 @@ func UnmarshalSessions(b []byte) (protocol.Sessions, error) { | |||||
| 1381 | if err != nil { | 1385 | if err != nil { | ||
| 1382 | return protocol.Sessions{}, err | 1386 | return protocol.Sessions{}, err | ||
| 1383 | } | 1387 | } | ||
| 1388 | + | if int(count) > len(r.b)/sessionRecordMinLen { | |||
| 1389 | + | return protocol.Sessions{}, errShortPayload | |||
| 1390 | + | } | |||
| 1384 | sessions := make([]protocol.SessionInfo, 0, count) | 1391 | sessions := make([]protocol.SessionInfo, 0, count) | ||
| 1385 | for range int(count) { | 1392 | for range int(count) { | ||
| 1386 | var s protocol.SessionInfo | 1393 | var s protocol.SessionInfo | ||
M internal/protocol/wire/wire_command.go +9 -0
| @@ -20,6 +20,15 @@ func PeekCommandVersion(b []byte) (uint16, bool) { | |||
| 20 | 20 | return binary.BigEndian.Uint16(b), true | |
| 21 | 21 | } | |
| 22 | 22 | ||
| 23 | + | // PeekCommandRequestID returns the request ID that follows the leading | |
| 24 | + | // protocol version in a CommandRequest payload. | |
| 25 | + | func PeekCommandRequestID(b []byte) (uint64, bool) { | |
| 26 | + | if len(b) < 10 { | |
| 27 | + | return 0, false | |
| 28 | + | } | |
| 29 | + | return binary.BigEndian.Uint64(b[2:10]), true | |
| 30 | + | } | |
| 31 | + | ||
| 23 | 32 | // MarshalCommandRequest encodes m into a CommandRequest payload. | |
| 24 | 33 | func MarshalCommandRequest(m protocol.CommandRequest) ([]byte, error) { | |
| 25 | 34 | if len(m.Args) > math.MaxUint16 { | |
| @@ -20,6 +20,15 @@ func PeekCommandVersion(b []byte) (uint16, bool) { | |||||
| 20 | return binary.BigEndian.Uint16(b), true | 20 | return binary.BigEndian.Uint16(b), true | ||
| 21 | } | 21 | } | ||
| 22 | 22 | ||||
| 23 | + | // PeekCommandRequestID returns the request ID that follows the leading | |||
| 24 | + | // protocol version in a CommandRequest payload. | |||
| 25 | + | func PeekCommandRequestID(b []byte) (uint64, bool) { | |||
| 26 | + | if len(b) < 10 { | |||
| 27 | + | return 0, false | |||
| 28 | + | } | |||
| 29 | + | return binary.BigEndian.Uint64(b[2:10]), true | |||
| 30 | + | } | |||
| 31 | + | ||||
| 23 | // MarshalCommandRequest encodes m into a CommandRequest payload. | 32 | // MarshalCommandRequest encodes m into a CommandRequest payload. | ||
| 24 | func MarshalCommandRequest(m protocol.CommandRequest) ([]byte, error) { | 33 | func MarshalCommandRequest(m protocol.CommandRequest) ([]byte, error) { | ||
| 25 | if len(m.Args) > math.MaxUint16 { | 34 | if len(m.Args) > math.MaxUint16 { | ||
M internal/protocol/wire/wire_test.go +8 -0
| @@ -579,6 +579,9 @@ func TestCommandRequestGoldenAndRoundTrip(t *testing.T) { | |||
| 579 | 579 | version, ok := PeekCommandVersion(got) | |
| 580 | 580 | require.True(t, ok) | |
| 581 | 581 | require.Equal(t, tt.msg.Version, version) | |
| 582 | + | requestID, ok := PeekCommandRequestID(got) | |
| 583 | + | require.True(t, ok) | |
| 584 | + | require.Equal(t, tt.msg.RequestID, requestID) | |
| 582 | 585 | assertAllPrefixesFail(t, got, UnmarshalCommandRequest) | |
| 583 | 586 | assertTrailingGarbageFails(t, got, UnmarshalCommandRequest) | |
| 584 | 587 | }) | |
| @@ -587,6 +590,9 @@ func TestCommandRequestGoldenAndRoundTrip(t *testing.T) { | |||
| 587 | 590 | if _, ok := PeekCommandVersion([]byte{0x00}); ok { | |
| 588 | 591 | t.Fatal("PeekCommandVersion accepted a one-byte payload") | |
| 589 | 592 | } | |
| 593 | + | if _, ok := PeekCommandRequestID(make([]byte, 9)); ok { | |
| 594 | + | t.Fatal("PeekCommandRequestID accepted a nine-byte payload") | |
| 595 | + | } | |
| 590 | 596 | } | |
| 591 | 597 | ||
| 592 | 598 | func TestMarshalCommandRequestRejectsTooManyArguments(t *testing.T) { | |
| @@ -897,6 +903,8 @@ func TestSessionsGoldenAndRoundTrip(t *testing.T) { | |||
| 897 | 903 | ||
| 898 | 904 | full := MarshalSessions(tests[1].msg) | |
| 899 | 905 | assertAllPrefixesFail(t, full, UnmarshalSessions) | |
| 906 | + | _, err := UnmarshalSessions([]byte{0xff, 0xff}) | |
| 907 | + | require.ErrorIs(t, err, errShortPayload) | |
| 900 | 908 | assertTrailingGarbageFails(t, full, UnmarshalSessions) | |
| 901 | 909 | } | |
| 902 | 910 | ||
| @@ -579,6 +579,9 @@ func TestCommandRequestGoldenAndRoundTrip(t *testing.T) { | |||||
| 579 | version, ok := PeekCommandVersion(got) | 579 | version, ok := PeekCommandVersion(got) | ||
| 580 | require.True(t, ok) | 580 | require.True(t, ok) | ||
| 581 | require.Equal(t, tt.msg.Version, version) | 581 | require.Equal(t, tt.msg.Version, version) | ||
| 582 | + | requestID, ok := PeekCommandRequestID(got) | |||
| 583 | + | require.True(t, ok) | |||
| 584 | + | require.Equal(t, tt.msg.RequestID, requestID) | |||
| 582 | assertAllPrefixesFail(t, got, UnmarshalCommandRequest) | 585 | assertAllPrefixesFail(t, got, UnmarshalCommandRequest) | ||
| 583 | assertTrailingGarbageFails(t, got, UnmarshalCommandRequest) | 586 | assertTrailingGarbageFails(t, got, UnmarshalCommandRequest) | ||
| 584 | }) | 587 | }) | ||
| @@ -587,6 +590,9 @@ func TestCommandRequestGoldenAndRoundTrip(t *testing.T) { | |||||
| 587 | if _, ok := PeekCommandVersion([]byte{0x00}); ok { | 590 | if _, ok := PeekCommandVersion([]byte{0x00}); ok { | ||
| 588 | t.Fatal("PeekCommandVersion accepted a one-byte payload") | 591 | t.Fatal("PeekCommandVersion accepted a one-byte payload") | ||
| 589 | } | 592 | } | ||
| 593 | + | if _, ok := PeekCommandRequestID(make([]byte, 9)); ok { | |||
| 594 | + | t.Fatal("PeekCommandRequestID accepted a nine-byte payload") | |||
| 595 | + | } | |||
| 590 | } | 596 | } | ||
| 591 | 597 | ||||
| 592 | func TestMarshalCommandRequestRejectsTooManyArguments(t *testing.T) { | 598 | func TestMarshalCommandRequestRejectsTooManyArguments(t *testing.T) { | ||
| @@ -897,6 +903,8 @@ func TestSessionsGoldenAndRoundTrip(t *testing.T) { | |||||
| 897 | 903 | ||||
| 898 | full := MarshalSessions(tests[1].msg) | 904 | full := MarshalSessions(tests[1].msg) | ||
| 899 | assertAllPrefixesFail(t, full, UnmarshalSessions) | 905 | assertAllPrefixesFail(t, full, UnmarshalSessions) | ||
| 906 | + | _, err := UnmarshalSessions([]byte{0xff, 0xff}) | |||
| 907 | + | require.ErrorIs(t, err, errShortPayload) | |||
| 900 | assertTrailingGarbageFails(t, full, UnmarshalSessions) | 908 | assertTrailingGarbageFails(t, full, UnmarshalSessions) | ||
| 901 | } | 909 | } | ||
| 902 | 910 | ||||
M internal/usecase/daemon/wire_metadata_test.go +1 -1
| @@ -6,7 +6,7 @@ import ( | |||
| 6 | 6 | "testing" | |
| 7 | 7 | ) | |
| 8 | 8 | ||
| 9 | - | func TestFrameWelcomeSnapshotsSessionMetadataUnderLock(t *testing.T) { | |
| 9 | + | func TestServerWelcomeSnapshotsSessionMetadataUnderLock(t *testing.T) { | |
| 10 | 10 | sess := &session{sessionCore: sessionCore{id: "session", name: "initial"}} | |
| 11 | 11 | ac := &attachedClient{resumeToken: 42} | |
| 12 | 12 | ||
| @@ -6,7 +6,7 @@ import ( | |||||
| 6 | "testing" | 6 | "testing" | ||
| 7 | ) | 7 | ) | ||
| 8 | 8 | ||||
| 9 | - | func TestFrameWelcomeSnapshotsSessionMetadataUnderLock(t *testing.T) { | 9 | + | func TestServerWelcomeSnapshotsSessionMetadataUnderLock(t *testing.T) { |
| 10 | sess := &session{sessionCore: sessionCore{id: "session", name: "initial"}} | 10 | sess := &session{sessionCore: sessionCore{id: "session", name: "initial"}} | ||
| 11 | ac := &attachedClient{resumeToken: 42} | 11 | ac := &attachedClient{resumeToken: 42} | ||
| 12 | 12 | ||||