Skip to content

Commit 60cb674

Browse files
kylecarbskylecarbs
committed
fix: Use buffered reader in peer to fix ShortBuffer
This prevents a io.ErrShortBuffer from occurring when the byte slice being read is smaller than the chunks sent from the opposite pipe. This makes sense for unordered connections, where transmission is not guaranteed, but does not make sense for TCP-like connections. We use a bufio.Reader when ordered to ensure data isn't lost.
1 parent 6009c90 commit 60cb674

File tree

5 files changed

+43
-7
lines changed

5 files changed

+43
-7
lines changed

‎.vscode/settings.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
"tfexec",
5858
"tfstate",
5959
"unconvert",
60+
"webrtc",
6061
"xerrors",
6162
"yamux"
6263
]

‎go.mod

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ replace github.com/chzyer/readline => github.com/kylecarbs/readline v0.0.0-20220
1616

1717
require (
1818
cdr.dev/slog v1.4.1
19-
github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2
2019
github.com/briandowns/spinner v1.18.1
2120
github.com/coder/retry v1.3.0
21+
github.com/creack/pty v1.1.17
2222
github.com/fatih/color v1.13.0
2323
github.com/go-chi/chi/v5 v5.0.7
2424
github.com/go-chi/render v1.0.1
@@ -50,6 +50,7 @@ require (
5050
go.uber.org/goleak v1.1.12
5151
golang.org/x/crypto v0.0.0-20220131195533-30dcbda58838
5252
golang.org/x/oauth2 v0.0.0-20211104180415-d3ed0bb246c8
53+
golang.org/x/sys v0.0.0-20220114195835-da31bd327af9
5354
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1
5455
google.golang.org/protobuf v1.27.1
5556
nhooyr.io/websocket v1.8.7
@@ -67,7 +68,6 @@ require (
6768
github.com/cenkalti/backoff/v4 v4.1.2 // indirect
6869
github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e // indirect
6970
github.com/containerd/continuity v0.2.2 // indirect
70-
github.com/creack/pty v1.1.17 // indirect
7171
github.com/davecgh/go-spew v1.1.1 // indirect
7272
github.com/dhui/dktest v0.3.9 // indirect
7373
github.com/dlclark/regexp2 v1.4.0 // indirect
@@ -124,7 +124,6 @@ require (
124124
github.com/zeebo/errs v1.2.2 // indirect
125125
go.opencensus.io v0.23.0 // indirect
126126
golang.org/x/net v0.0.0-20220127200216-cd36cc0744dd // indirect
127-
golang.org/x/sys v0.0.0-20220114195835-da31bd327af9 // indirect
128127
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211 // indirect
129128
golang.org/x/text v0.3.7 // indirect
130129
google.golang.org/appengine v1.6.7 // indirect

‎go.sum

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,6 @@ github.com/Microsoft/hcsshim v0.8.23/go.mod h1:4zegtUJth7lAvFyc6cH2gGQ5B3OFQim01
103103
github.com/Microsoft/hcsshim/test v0.0.0-20201218223536-d3e5debf77da/go.mod h1:5hlzMzRKMLyo42nCZ9oml8AdTlq/0cvIaBv6tK1RehU=
104104
github.com/Microsoft/hcsshim/test v0.0.0-20210227013316-43a75bb4edd3/go.mod h1:mw7qgWloBUl75W/gVH3cQszUg1+gUITj7D6NY7ywVnY=
105105
github.com/NYTimes/gziphandler v0.0.0-20170623195520-56545f4a5d46/go.mod h1:3wb06e3pkSAbeQ52E9H9iFoQsEEwGN64994WTCIhntQ=
106-
github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2 h1:+vx7roKuyA63nhn5WAunQHLTznkw5W8b1Xc0dNjp83s=
107-
github.com/Netflix/go-expect v0.0.0-20220104043353-73e0943537d2/go.mod h1:HBCaDeC1lPdgDeDbhX8XFpy1jqjK0IBG8W5K+xYqA0w=
108106
github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5 h1:TngWCqHvy9oXAN6lEVMRuU21PR1EtLVZJmdB18Gu3Rw=
109107
github.com/Nvveen/Gotty v0.0.0-20120604004816-cd527374f1e5/go.mod h1:lmUJ/7eu/Q8D7ML55dXQrVaamCz2vxCfdQBasLZfHKk=
110108
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=

‎peer/channel.go

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package peer
22

33
import (
4+
"bufio"
45
"context"
56
"io"
67
"net"
@@ -78,7 +79,8 @@ type Channel struct {
7879
dc *webrtc.DataChannel
7980
// This field can be nil. It becomes set after the DataChannel
8081
// has been opened and is detached.
81-
rwc datachannel.ReadWriteCloser
82+
rwc datachannel.ReadWriteCloser
83+
reader io.Reader
8284

8385
closed chan struct{}
8486
closeMutex sync.Mutex
@@ -130,6 +132,21 @@ func (c *Channel) init() {
130132
_ = c.closeWithError(xerrors.Errorf("detach: %w", err))
131133
return
132134
}
135+
// pion/webrtc will return an io.ErrShortBuffer when a read
136+
// is triggerred with a buffer size less than the chunks written.
137+
//
138+
// This makes sense when considering UDP connections, because
139+
// bufferring of data that has no transmit guarantees is likely
140+
// to cause unexpected behavior.
141+
//
142+
// When ordered, this adds a bufio.Reader. This ensures additional
143+
// data on TCP-like connections can be read in parts, while still
144+
// being bufferred.
145+
if c.opts.Unordered {
146+
c.reader = c.rwc
147+
} else {
148+
c.reader = bufio.NewReader(c.rwc)
149+
}
133150
close(c.opened)
134151
})
135152

@@ -181,7 +198,7 @@ func (c *Channel) Read(bytes []byte) (int, error) {
181198
}
182199
}
183200

184-
bytesRead, err := c.rwc.Read(bytes)
201+
bytesRead, err := c.reader.Read(bytes)
185202
if err != nil {
186203
if c.isClosed() {
187204
return 0, c.closeError

‎peer/conn_test.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,27 @@ func TestConn(t *testing.T) {
267267
_, err := client.Ping()
268268
require.NoError(t, err)
269269
})
270+
271+
t.Run("ShortBuffer", func(t *testing.T) {
272+
t.Parallel()
273+
client, server, _ := createPair(t)
274+
exchange(client, server)
275+
go func() {
276+
channel, err := client.Dial(context.Background(), "test", nil)
277+
require.NoError(t, err)
278+
_, err = channel.Write([]byte{1, 2})
279+
require.NoError(t, err)
280+
}()
281+
channel, err := server.Accept(context.Background())
282+
require.NoError(t, err)
283+
data := make([]byte, 1)
284+
_, err = channel.Read(data)
285+
require.NoError(t, err)
286+
require.Equal(t, uint8(0x1), data[0])
287+
_, err = channel.Read(data)
288+
require.NoError(t, err)
289+
require.Equal(t, uint8(0x2), data[0])
290+
})
270291
}
271292

272293
func createPair(t *testing.T) (client *peer.Conn, server *peer.Conn, wan *vnet.Router) {

0 commit comments

Comments
 (0)