Skip to content

Commit

Permalink
fix: The client write operation did not immediately return upon encou…
Browse files Browse the repository at this point in the history
…ntering an RST packet. (#1849)

The current client implementation does not immediately return when encountering an RST packet while sending a request, but instead ignores it. This behavior is inconsistent with the net/http package and does not make logical sense.
  • Loading branch information
newacorn authored Aug 31, 2024
1 parent d31f4ef commit d5c7d89
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 165 deletions.
5 changes: 2 additions & 3 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -2989,8 +2989,7 @@ func (t *transport) RoundTrip(hc *HostClient, req *Request, resp *Response) (ret
err = ErrTimeout
}

isConnRST := isConnectionReset(err)
if err != nil && !isConnRST {
if err != nil {
hc.closeConn(cc)
return true, err
}
Expand Down Expand Up @@ -3025,7 +3024,7 @@ func (t *transport) RoundTrip(hc *HostClient, req *Request, resp *Response) (ret
return needRetry, err
}

closeConn := resetConnection || req.ConnectionClose() || resp.ConnectionClose() || isConnRST
closeConn := resetConnection || req.ConnectionClose() || resp.ConnectionClose()
if customStreamBody && resp.bodyStream != nil {
rbs := resp.bodyStream
resp.bodyStream = newCloseReaderWithError(rbs, func(wErr error) error {
Expand Down
69 changes: 69 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import (
"fmt"
"io"
"net"
"net/http"
"net/url"
"os"
"regexp"
"runtime"
"strings"
"sync"
"sync/atomic"
"syscall"
"testing"
"time"

Expand Down Expand Up @@ -3531,3 +3533,70 @@ func TestClientHeadWithBody(t *testing.T) {
t.Error(err)
}
}

func TestRevertPull1233(t *testing.T) {
if runtime.GOOS == "windows" {
t.SkipNow()
}
t.Parallel()
const expectedStatus = http.StatusTeapot
ln, err := net.Listen("tcp", "127.0.0.1:8089")
defer func() { ln.Close() }()
if err != nil {
t.Fatal(err.Error())
}

go func() {
for {
conn, err := ln.Accept()
if err != nil {
if !strings.Contains(err.Error(), "closed") {
t.Errorf(err.Error())
}
return
}
_, err = conn.Write([]byte("HTTP/1.1 418 Teapot\r\n\r\n"))
if err != nil {
t.Error(err)
}
err = conn.(*net.TCPConn).SetLinger(0)
if err != nil {
t.Errorf(err.Error())
}
conn.Close()
}
}()

reqURL := "http://" + ln.Addr().String()
reqStrBody := "hello 2323 23323 2323 2323 232323 323 2323 2333333 hello 2323 23323 2323 2323 232323 323 2323 2333333 hello 2323 23323 2323 2323 232323 323 2323 2333333 hello 2323 23323 2323 2323 232323 323 2323 2333333 hello 2323 23323 2323 2323 232323 323 2323 2333333"
req2 := AcquireRequest()
resp2 := AcquireResponse()
defer func() {
ReleaseRequest(req2)
ReleaseResponse(resp2)
}()
req2.SetRequestURI(reqURL)
req2.SetBodyStream(F{strings.NewReader(reqStrBody)}, -1)
cli2 := Client{}
err = cli2.Do(req2, resp2)
if !errors.Is(err, syscall.EPIPE) && !errors.Is(err, syscall.ECONNRESET) {
t.Errorf("expected error %v or %v, but got nil", syscall.EPIPE, syscall.ECONNRESET)
}
if expectedStatus == resp2.StatusCode() {
t.Errorf("Not Expected status code %d", resp2.StatusCode())
}
}

type F struct {
*strings.Reader
}

func (f F) Read(p []byte) (n int, err error) {
if len(p) > 10 {
p = p[:10]
}
// Ensure that subsequent segments can see the RST packet caused by sending previous
// segments to a closed connection.
time.Sleep(500 * time.Microsecond)
return f.Reader.Read(p)
}
134 changes: 0 additions & 134 deletions client_unix_test.go

This file was deleted.

6 changes: 0 additions & 6 deletions http.go
Original file line number Diff line number Diff line change
Expand Up @@ -1439,9 +1439,6 @@ func (resp *Response) ReadLimitBody(r *bufio.Reader, maxBodySize int) error {
if !resp.mustSkipBody() {
err = resp.ReadBody(r, maxBodySize)
if err != nil {
if isConnectionReset(err) {
return nil
}
return err
}
}
Expand All @@ -1450,9 +1447,6 @@ func (resp *Response) ReadLimitBody(r *bufio.Reader, maxBodySize int) error {
if resp.Header.ContentLength() == -1 && !resp.StreamBody && !resp.mustSkipBody() {
err = resp.Header.ReadTrailer(r)
if err != nil && err != io.EOF {
if isConnectionReset(err) {
return nil
}
return err
}
}
Expand Down
12 changes: 0 additions & 12 deletions tcp.go

This file was deleted.

10 changes: 0 additions & 10 deletions tcp_windows.go

This file was deleted.

0 comments on commit d5c7d89

Please sign in to comment.