detached 4b7cd9e

fix(protocol): apply follow-up review fixes

commit
4b7cd9ef7c405c838ba6397d255a153f2a075cc9
author
brice · · 2026-08-02 11:39 UTC
parent
eed2010
signature
GPG VerifiedCD88F9E564E0102646FB6077A1A07C7C7C3E349D · arch-gpg Verified for the committer email
4 files · +50 -1
Unified
UnifiedSplit
M internal/adapters/sshstdio/transport.go +1 -1
@@ -21,7 +21,7 @@ import (
2121 )
2222
2323 const (
24- maxFrameLen = 16 << 20
24+ maxFrameLen = ports.MaxFrameLen
2525 frameHeaderLen = 4
2626 sshCloseTimeout = 3 * time.Second
2727 )
@@ -21,7 +21,7 @@ import (
21 )21 )
22 22
23 const (23 const (
24- maxFrameLen = 16 << 2024+ maxFrameLen = ports.MaxFrameLen
25 frameHeaderLen = 425 frameHeaderLen = 4
26 sshCloseTimeout = 3 * time.Second26 sshCloseTimeout = 3 * time.Second
27 )27 )
M internal/adapters/sshstdio/transport_test.go +35 -0
@@ -3,6 +3,7 @@ package sshstdio
33 import (
44 "bytes"
55 "context"
6+ "encoding/binary"
67 "errors"
78 "io"
89 "log/slog"
@@ -164,6 +165,40 @@ func TestTransportRejectsZeroLengthFrame(t *testing.T) {
164165 }
165166 }
166167
168+func TestTransportUsesCanonicalFrameMaximum(t *testing.T) {
169+ boundaryPayload := make([]byte, ports.MaxFrameLen-1)
170+ boundaryWire := &bytes.Buffer{}
171+ boundarySend := NewTransport(nil, boundaryWire, nil)
172+ if err := boundarySend.Send(ports.Frame{Type: ports.MsgOutput, Payload: boundaryPayload}); err != nil {
173+ t.Fatalf("boundary Send error = %v", err)
174+ }
175+ if got := binary.BigEndian.Uint32(boundaryWire.Bytes()[:frameHeaderLen]); got != ports.MaxFrameLen {
176+ t.Fatalf("boundary frame length = %d, want %d", got, ports.MaxFrameLen)
177+ }
178+ boundaryRecv := NewTransport(bytes.NewReader(boundaryWire.Bytes()), io.Discard, nil)
179+ boundaryFrame, err := boundaryRecv.Recv()
180+ if err != nil {
181+ t.Fatalf("boundary Recv error = %v", err)
182+ }
183+ if boundaryFrame.Type != ports.MsgOutput || len(boundaryFrame.Payload) != len(boundaryPayload) {
184+ t.Fatalf("boundary frame = type %d, payload %d bytes; want type %d, payload %d bytes", boundaryFrame.Type, len(boundaryFrame.Payload), ports.MsgOutput, len(boundaryPayload))
185+ }
186+
187+ send := NewTransport(nil, io.Discard, nil)
188+ err = send.Send(ports.Frame{Type: ports.MsgOutput, Payload: make([]byte, ports.MaxFrameLen)})
189+ if !errors.Is(err, ErrFrameTooLarge) {
190+ t.Fatalf("Send error = %v, want ErrFrameTooLarge", err)
191+ }
192+
193+ var header [frameHeaderLen]byte
194+ binary.BigEndian.PutUint32(header[:], ports.MaxFrameLen+1)
195+ recv := NewTransport(bytes.NewReader(header[:]), io.Discard, nil)
196+ _, err = recv.Recv()
197+ if !errors.Is(err, ErrFrameTooLarge) {
198+ t.Fatalf("Recv error = %v, want ErrFrameTooLarge", err)
199+ }
200+}
201+
167202 func TestDialContextCanceledBeforeStart(t *testing.T) {
168203 ctx, cancel := context.WithCancel(context.Background())
169204 cancel()
@@ -3,6 +3,7 @@ package sshstdio
3 import (3 import (
4 "bytes"4 "bytes"
5 "context"5 "context"
6+ "encoding/binary"
6 "errors"7 "errors"
7 "io"8 "io"
8 "log/slog"9 "log/slog"
@@ -164,6 +165,40 @@ func TestTransportRejectsZeroLengthFrame(t *testing.T) {
164 }165 }
165 }166 }
166 167
168+func TestTransportUsesCanonicalFrameMaximum(t *testing.T) {
169+ boundaryPayload := make([]byte, ports.MaxFrameLen-1)
170+ boundaryWire := &bytes.Buffer{}
171+ boundarySend := NewTransport(nil, boundaryWire, nil)
172+ if err := boundarySend.Send(ports.Frame{Type: ports.MsgOutput, Payload: boundaryPayload}); err != nil {
173+ t.Fatalf("boundary Send error = %v", err)
174+ }
175+ if got := binary.BigEndian.Uint32(boundaryWire.Bytes()[:frameHeaderLen]); got != ports.MaxFrameLen {
176+ t.Fatalf("boundary frame length = %d, want %d", got, ports.MaxFrameLen)
177+ }
178+ boundaryRecv := NewTransport(bytes.NewReader(boundaryWire.Bytes()), io.Discard, nil)
179+ boundaryFrame, err := boundaryRecv.Recv()
180+ if err != nil {
181+ t.Fatalf("boundary Recv error = %v", err)
182+ }
183+ if boundaryFrame.Type != ports.MsgOutput || len(boundaryFrame.Payload) != len(boundaryPayload) {
184+ t.Fatalf("boundary frame = type %d, payload %d bytes; want type %d, payload %d bytes", boundaryFrame.Type, len(boundaryFrame.Payload), ports.MsgOutput, len(boundaryPayload))
185+ }
186+
187+ send := NewTransport(nil, io.Discard, nil)
188+ err = send.Send(ports.Frame{Type: ports.MsgOutput, Payload: make([]byte, ports.MaxFrameLen)})
189+ if !errors.Is(err, ErrFrameTooLarge) {
190+ t.Fatalf("Send error = %v, want ErrFrameTooLarge", err)
191+ }
192+
193+ var header [frameHeaderLen]byte
194+ binary.BigEndian.PutUint32(header[:], ports.MaxFrameLen+1)
195+ recv := NewTransport(bytes.NewReader(header[:]), io.Discard, nil)
196+ _, err = recv.Recv()
197+ if !errors.Is(err, ErrFrameTooLarge) {
198+ t.Fatalf("Recv error = %v, want ErrFrameTooLarge", err)
199+ }
200+}
201+
167 func TestDialContextCanceledBeforeStart(t *testing.T) {202 func TestDialContextCanceledBeforeStart(t *testing.T) {
168 ctx, cancel := context.WithCancel(context.Background())203 ctx, cancel := context.WithCancel(context.Background())
169 cancel()204 cancel()
M internal/ports/wire_screen_test.go +3 -0
@@ -386,6 +386,9 @@ func FuzzScreenUpdate(f *testing.F) {
386386 if err != nil {
387387 t.Fatal(err)
388388 }
389+ if !bytes.Equal(encoded, data) {
390+ t.Fatalf("accepted data is not canonical: got %x, want %x", encoded, data)
391+ }
389392 roundtrip, err := UnmarshalScreenUpdate(encoded)
390393 if err != nil {
391394 t.Fatal(err)
@@ -386,6 +386,9 @@ func FuzzScreenUpdate(f *testing.F) {
386 if err != nil {386 if err != nil {
387 t.Fatal(err)387 t.Fatal(err)
388 }388 }
389+ if !bytes.Equal(encoded, data) {
390+ t.Fatalf("accepted data is not canonical: got %x, want %x", encoded, data)
391+ }
389 roundtrip, err := UnmarshalScreenUpdate(encoded)392 roundtrip, err := UnmarshalScreenUpdate(encoded)
390 if err != nil {393 if err != nil {
391 t.Fatal(err)394 t.Fatal(err)
M internal/ports/wire_test.go +11 -0
@@ -625,6 +625,17 @@ func TestOutputGoldenAndRoundTrip(t *testing.T) {
625625 }
626626
627627 assertAllPrefixesFail(t, got[:24], UnmarshalOutput)
628+
629+ extra := []byte{0x00, 0xff, 0x7f}
630+ withTrailing := append(append([]byte(nil), got...), extra...)
631+ back, err = UnmarshalOutput(withTrailing)
632+ if err != nil {
633+ t.Fatalf("UnmarshalOutput() with trailing payload error = %v", err)
634+ }
635+ wantData := append(append([]byte(nil), msg.Data...), extra...)
636+ if !bytes.Equal(back.Data, wantData) {
637+ t.Fatalf("trailing payload data = %#v, want %#v", back.Data, wantData)
638+ }
628639 }
629640
630641 func TestAckGoldenAndRoundTrip(t *testing.T) {
@@ -625,6 +625,17 @@ func TestOutputGoldenAndRoundTrip(t *testing.T) {
625 }625 }
626 626
627 assertAllPrefixesFail(t, got[:24], UnmarshalOutput)627 assertAllPrefixesFail(t, got[:24], UnmarshalOutput)
628+
629+ extra := []byte{0x00, 0xff, 0x7f}
630+ withTrailing := append(append([]byte(nil), got...), extra...)
631+ back, err = UnmarshalOutput(withTrailing)
632+ if err != nil {
633+ t.Fatalf("UnmarshalOutput() with trailing payload error = %v", err)
634+ }
635+ wantData := append(append([]byte(nil), msg.Data...), extra...)
636+ if !bytes.Equal(back.Data, wantData) {
637+ t.Fatalf("trailing payload data = %#v, want %#v", back.Data, wantData)
638+ }
628 }639 }
629 640
630 func TestAckGoldenAndRoundTrip(t *testing.T) {641 func TestAckGoldenAndRoundTrip(t *testing.T) {

Keyboard shortcuts

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

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

Branches 12

Tags 11

↑↓ moveenter openesc close