Skip to content

Commit c06b2d4

Browse files
macmarcelinojcsf
authored andcommitted
TUN-10621: Test websocket path
Adding a test that tries to exercise the race condition for the websocket path. The sequence is as follows: 1. The test starts by writing the payload to the mock server 2. Then, it closes the write side of the connection. 3. It waits for 3 seconds to make the race window explicit: the bug in cloudflared would have closed the connection and we would not be able to read. 4. Read the message. With the fix applied, we should be able to read the full message.
1 parent a9d023b commit c06b2d4

2 files changed

Lines changed: 140 additions & 1 deletion

File tree

carrier/websocket_test.go

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,12 @@ import (
55
"crypto/rand"
66
"crypto/tls"
77
"crypto/x509"
8+
"errors"
89
"fmt"
10+
"io"
911
"math/big"
12+
"net/http"
13+
"net/http/httptest"
1014
"testing"
1115
"time"
1216

@@ -17,6 +21,7 @@ import (
1721
"golang.org/x/net/websocket"
1822

1923
"github.com/cloudflare/cloudflared/hello"
24+
"github.com/cloudflare/cloudflared/stream"
2025
"github.com/cloudflare/cloudflared/tlsconfig"
2126
cfwebsocket "github.com/cloudflare/cloudflared/websocket"
2227
)
@@ -122,3 +127,137 @@ func TestWebsocketWrapper(t *testing.T) {
122127
require.Equal(t, 2, n)
123128
require.Equal(t, "bc", string(buf[:n]))
124129
}
130+
131+
// halfClosePipeEnd is a bidirectional pipe built from two independent io.Pipe
132+
// pairs. Unlike net.Pipe, closing one direction does not affect the other, which
133+
// lets us simulate a half-close: the local client can signal EOF on its write
134+
// side while still reading a delayed response from the remote side.
135+
type halfClosePipeEnd struct {
136+
r *io.PipeReader // data flowing into this end
137+
w *io.PipeWriter // data flowing out of this end
138+
}
139+
140+
func newHalfClosePipe() (local, remote *halfClosePipeEnd) {
141+
// Pipe A carries data from local → remote.
142+
ar, aw := io.Pipe()
143+
// Pipe B carries data from remote → local.
144+
br, bw := io.Pipe()
145+
146+
local = &halfClosePipeEnd{r: br, w: aw}
147+
remote = &halfClosePipeEnd{r: ar, w: bw}
148+
return local, remote
149+
}
150+
151+
func (p *halfClosePipeEnd) Read(b []byte) (int, error) { return p.r.Read(b) }
152+
func (p *halfClosePipeEnd) Write(b []byte) (int, error) { return p.w.Write(b) }
153+
154+
// CloseWrite signals EOF to the remote reader without closing the read side.
155+
func (p *halfClosePipeEnd) CloseWrite() error { return p.w.Close() }
156+
157+
// Close shuts down both directions.
158+
func (p *halfClosePipeEnd) Close() error {
159+
_ = p.r.Close()
160+
return p.w.Close()
161+
}
162+
163+
// TestServeStreamWaitsForResponseAfterLocalClose exercises the websocket path
164+
// It verifies that the pipe does not tear down immediately after the client closes
165+
// its write side.
166+
//
167+
// The setup mirrors the cloudflared access tcp path:
168+
//
169+
// local app -> halfClosePipe -> ServeStream -> mock WS echo server
170+
//
171+
// Sequence:
172+
// 1. Write payload to the mock server through ServeStream.
173+
// 2. Half-close the write side (CloseWrite) — signals EOF upstream.
174+
// 3. Sleep for halfCloseWait to make the race window explicit: a buggy
175+
// (timeout=0) pipe would already have torn down the connection here.
176+
// 4. Read the echo — must succeed because the patch keeps the pipe alive.
177+
func TestServeStreamWaitsForResponseAfterLocalClose(t *testing.T) {
178+
t.Parallel()
179+
180+
const (
181+
payload = "half-close-test"
182+
// halfCloseWait makes the race window visible: if ServeStream tears
183+
// down the connection on CloseWrite the read that follows will fail
184+
// immediately, mirroring the 3-second sleep in the cftunnel reference
185+
// test. It must be shorter than DefaultTimeoutAfterFirstClose (10s).
186+
halfCloseWait = 3 * time.Second
187+
// testTimeout is an upper bound for the whole test.
188+
testTimeout = stream.DefaultTimeoutAfterFirstClose + 5*time.Second
189+
)
190+
191+
server := websocketServer()
192+
defer server.Close()
193+
194+
localEnd, remoteEnd := newHalfClosePipe()
195+
196+
log := zerolog.Nop()
197+
wsConn := NewWSConnection(&log)
198+
options := &StartOptions{
199+
OriginURL: "ws://" + server.Listener.Addr().String(),
200+
}
201+
202+
serveErrCh := make(chan error, 1)
203+
go func() {
204+
serveErrCh <- wsConn.ServeStream(options, remoteEnd)
205+
}()
206+
207+
ctx, cancel := context.WithTimeout(t.Context(), testTimeout)
208+
defer cancel()
209+
210+
// 1. Write the payload. ServeStream forwards it as a WS binary frame.
211+
_, err := localEnd.Write([]byte(payload))
212+
require.NoError(t, err)
213+
214+
// 2. Half-close the write side
215+
require.NoError(t, localEnd.CloseWrite())
216+
217+
// 3. Wait to make the race window explicit.
218+
time.Sleep(halfCloseWait)
219+
220+
// 4. Read the echo.
221+
got := make([]byte, len(payload))
222+
_, err = io.ReadFull(localEnd, got)
223+
require.NoError(t, err, "read after half-close failed: pipe was torn down too early")
224+
require.Equal(t, payload, string(got))
225+
226+
// Drain ServeStream.
227+
_ = localEnd.Close()
228+
_ = remoteEnd.Close()
229+
230+
select {
231+
case err := <-serveErrCh:
232+
if err != nil && err != io.EOF && !errors.Is(err, io.ErrClosedPipe) {
233+
require.NoError(t, err)
234+
}
235+
case <-ctx.Done():
236+
t.Fatal("ServeStream did not return in time")
237+
}
238+
}
239+
240+
func websocketServer() *httptest.Server {
241+
upgrader := gws.Upgrader{
242+
ReadBufferSize: 1024,
243+
WriteBufferSize: 1024,
244+
CheckOrigin: func(*http.Request) bool { return true },
245+
}
246+
247+
return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
248+
conn, err := upgrader.Upgrade(w, r, nil)
249+
if err != nil {
250+
return
251+
}
252+
defer func() { _ = conn.Close() }()
253+
254+
_, msg, err := conn.ReadMessage()
255+
if err != nil {
256+
return
257+
}
258+
259+
if err := conn.WriteMessage(gws.BinaryMessage, msg); err != nil {
260+
return
261+
}
262+
}))
263+
}

cmd/cloudflared/updater/update.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,5 +299,5 @@ func wasInstalledFromPackageManager() bool {
299299
}
300300

301301
func isRunningFromTerminal() bool {
302-
return term.IsTerminal(int(os.Stdout.Fd()))
302+
return term.IsTerminal(int(os.Stdout.Fd())) //nolint:gosec
303303
}

0 commit comments

Comments
 (0)