Skip to content

Commit a19277a

Browse files
committed
fix(https): close client connection when target connection errors
Signed-off-by: wucm667 <stevenwucongmin@gmail.com>
1 parent e493e1c commit a19277a

2 files changed

Lines changed: 96 additions & 5 deletions

File tree

https.go

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -473,12 +473,20 @@ func copyOrWarn(ctx *ProxyCtx, dst io.Writer, src io.Reader) error {
473473

474474
func copyAndClose(ctx *ProxyCtx, dst, src halfClosable, wg *sync.WaitGroup) {
475475
_, err := io.Copy(dst, src)
476-
if err != nil && !errors.Is(err, net.ErrClosed) {
477-
ctx.Warnf("Error copying to client: %s", err.Error())
476+
if err != nil {
477+
if !errors.Is(err, net.ErrClosed) {
478+
ctx.Warnf("Error copying to client: %s", err.Error())
479+
}
480+
// Fully close dst to unblock any goroutine blocked on
481+
// io.Copy reading from it. Half-close (CloseWrite/CloseRead)
482+
// would not interrupt a pending read, leaving the other
483+
// goroutine stuck and the client connection never closed.
484+
_ = dst.Close()
485+
_ = src.Close()
486+
} else {
487+
_ = dst.CloseWrite()
488+
_ = src.CloseRead()
478489
}
479-
480-
_ = dst.CloseWrite()
481-
_ = src.CloseRead()
482490
wg.Done()
483491
}
484492

proxy_test.go

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1566,3 +1566,86 @@ func TestNoTrailersUnchanged(t *testing.T) {
15661566
require.Equal(t, "ok", strings.TrimSpace(string(body)))
15671567
require.Empty(t, resp.Trailer)
15681568
}
1569+
1570+
// TestTransparentTunnelClosesClientConnOnTargetError verifies that when the
1571+
// target connection closes unexpectedly during a transparent TCP tunnel
1572+
// (ConnectAccept path), the client connection is also closed so the client
1573+
// doesn't hang indefinitely.
1574+
//
1575+
// Regression test for https://github.com/elazarl/goproxy/issues/657
1576+
func TestTransparentTunnelClosesClientConnOnTargetError(t *testing.T) {
1577+
// Target server: closes connection immediately after reading from client.
1578+
// This simulates a target IO error (timeout, connection reset).
1579+
targetListener, err := net.Listen("tcp", "127.0.0.1:0")
1580+
require.NoError(t, err)
1581+
defer targetListener.Close()
1582+
1583+
targetConnCh := make(chan net.Conn, 1)
1584+
go func() {
1585+
conn, err := targetListener.Accept()
1586+
if err != nil {
1587+
return
1588+
}
1589+
targetConnCh <- conn
1590+
}()
1591+
1592+
// Proxy server: transparent tunnel (ConnectAccept, the default).
1593+
// No HandleConnect handler means OkConnect (transparent tunnel) is used.
1594+
proxy := goproxy.NewProxyHttpServer()
1595+
1596+
proxyServer := httptest.NewServer(proxy)
1597+
defer proxyServer.Close()
1598+
1599+
proxyURL, err := url.Parse(proxyServer.URL)
1600+
require.NoError(t, err)
1601+
1602+
// Connect to proxy.
1603+
clientConn, err := net.Dial("tcp", proxyURL.Host)
1604+
require.NoError(t, err)
1605+
defer clientConn.Close()
1606+
1607+
// Send CONNECT request.
1608+
connectReq := &http.Request{
1609+
Method: http.MethodConnect,
1610+
URL: &url.URL{Opaque: targetListener.Addr().String()},
1611+
Host: targetListener.Addr().String(),
1612+
Header: make(http.Header),
1613+
}
1614+
require.NoError(t, connectReq.Write(clientConn))
1615+
1616+
// Read CONNECT response.
1617+
br := bufio.NewReader(clientConn)
1618+
connectResp, err := http.ReadResponse(br, connectReq)
1619+
require.NoError(t, err)
1620+
require.Equal(t, http.StatusOK, connectResp.StatusCode)
1621+
1622+
// Wait for target to accept connection.
1623+
var targetConn net.Conn
1624+
select {
1625+
case targetConn = <-targetConnCh:
1626+
case <-time.After(5 * time.Second):
1627+
t.Fatal("timed out waiting for target connection")
1628+
}
1629+
1630+
// Send data through the tunnel so the target starts reading.
1631+
_, err = clientConn.Write([]byte("hello"))
1632+
require.NoError(t, err)
1633+
1634+
// Target closes connection (simulates IO error).
1635+
require.NoError(t, targetConn.Close())
1636+
1637+
// Verify client connection is closed within timeout.
1638+
done := make(chan struct{})
1639+
go func() {
1640+
buf := make([]byte, 1024)
1641+
_, _ = clientConn.Read(buf)
1642+
close(done)
1643+
}()
1644+
1645+
select {
1646+
case <-done:
1647+
// Connection was closed — this is the expected behavior.
1648+
case <-time.After(3 * time.Second):
1649+
t.Fatal("client connection was not closed after target closed; client would hang indefinitely")
1650+
}
1651+
}

0 commit comments

Comments
 (0)