diff --git a/.github/workflows/go-retryablehttp.yml b/.github/workflows/go-retryablehttp.yml index 9751243..dad25d8 100644 --- a/.github/workflows/go-retryablehttp.yml +++ b/.github/workflows/go-retryablehttp.yml @@ -10,7 +10,7 @@ jobs: - name: Setup go uses: actions/setup-go@4d34df0c2316fe8122ab82dc22947d607c0c91f9 # v4.0.0 with: - go-version: 1.14.2 + go-version: 1.18 - uses: actions/checkout@8f4b7f84864484a7bf31766abe9204da3cbe65b3 # v3.5.0 - run: mkdir -p "$TEST_RESULTS"/go-retryablyhttp - name: restore_cache @@ -20,6 +20,7 @@ jobs: restore-keys: go-mod-v1-{{ checksum "go.sum" }} path: "/go/pkg/mod" - run: go mod download + - run: go mod tidy - name: Run go format run: |- files=$(go fmt ./...) @@ -29,7 +30,7 @@ jobs: exit 1 fi - name: Install gotestsum - run: go get gotest.tools/gotestsum + run: go install gotest.tools/gotestsum@latest - name: Run unit tests run: |- PACKAGE_NAMES=$(go list ./...) diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..7a17b9f --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,15 @@ +## 0.7.5 (Nov 8, 2023) + +BUG FIXES + +- client: fixes an issue where the request body is not preserved on temporary redirects or re-established HTTP/2 connections [GH-207] + +## 0.7.4 (Jun 6, 2023) + +BUG FIXES + +- client: fixing an issue where the Content-Type header wouldn't be sent with an empty payload when using HTTP/2 [GH-194] + +## 0.7.3 (May 15, 2023) + +Initial release diff --git a/CODEOWNERS b/CODEOWNERS index f8389c9..d6dd78a 100644 --- a/CODEOWNERS +++ b/CODEOWNERS @@ -1 +1 @@ -* @hashicorp/release-engineering \ No newline at end of file +* @hashicorp/go-retryablehttp-maintainers diff --git a/client.go b/client.go index c345af6..76ba81c 100644 --- a/client.go +++ b/client.go @@ -160,6 +160,20 @@ func (r *Request) SetBody(rawBody interface{}) error { } r.body = bodyReader r.ContentLength = contentLength + if bodyReader != nil { + r.GetBody = func() (io.ReadCloser, error) { + body, err := bodyReader() + if err != nil { + return nil, err + } + if rc, ok := body.(io.ReadCloser); ok { + return rc, nil + } + return io.NopCloser(body), nil + } + } else { + r.GetBody = func() (io.ReadCloser, error) { return http.NoBody, nil } + } return nil } @@ -258,10 +272,17 @@ func getBodyReaderAndContentLength(rawBody interface{}) (ReaderFunc, int64, erro if err != nil { return nil, 0, err } - bodyReader = func() (io.Reader, error) { - return bytes.NewReader(buf), nil + if len(buf) == 0 { + bodyReader = func() (io.Reader, error) { + return http.NoBody, nil + } + contentLength = 0 + } else { + bodyReader = func() (io.Reader, error) { + return bytes.NewReader(buf), nil + } + contentLength = int64(len(buf)) } - contentLength = int64(len(buf)) // No body provided, nothing to do case nil: @@ -293,18 +314,19 @@ func NewRequest(method, url string, rawBody interface{}) (*Request, error) { // The context controls the entire lifetime of a request and its response: // obtaining a connection, sending the request, and reading the response headers and body. func NewRequestWithContext(ctx context.Context, method, url string, rawBody interface{}) (*Request, error) { - bodyReader, contentLength, err := getBodyReaderAndContentLength(rawBody) + httpReq, err := http.NewRequestWithContext(ctx, method, url, nil) if err != nil { return nil, err } - httpReq, err := http.NewRequestWithContext(ctx, method, url, nil) - if err != nil { + req := &Request{ + Request: httpReq, + } + if err := req.SetBody(rawBody); err != nil { return nil, err } - httpReq.ContentLength = contentLength - return &Request{body: bodyReader, Request: httpReq}, nil + return req, nil } // Logger interface allows to use other loggers than diff --git a/client_test.go b/client_test.go index 5aaed05..c5e98a5 100644 --- a/client_test.go +++ b/client_test.go @@ -978,3 +978,62 @@ func TestClient_StandardClient(t *testing.T) { t.Fatalf("expected %v, got %v", client, v) } } + +func TestClient_RedirectWithBody(t *testing.T) { + var redirects int32 + // Mock server which always responds 200. + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + switch r.RequestURI { + case "/redirect": + w.Header().Set("Location", "/target") + w.WriteHeader(http.StatusTemporaryRedirect) + case "/target": + atomic.AddInt32(&redirects, 1) + w.WriteHeader(http.StatusCreated) + default: + t.Fatalf("bad uri: %s", r.RequestURI) + } + })) + defer ts.Close() + + client := NewClient() + client.RequestLogHook = func(logger Logger, req *http.Request, retryNumber int) { + if _, err := req.GetBody(); err != nil { + t.Fatalf("unexpected error with GetBody: %v", err) + } + } + // create a request with a body + req, err := NewRequest(http.MethodPost, ts.URL+"/redirect", strings.NewReader(`{"foo":"bar"}`)) + if err != nil { + t.Fatalf("err: %v", err) + } + + resp, err := client.Do(req) + if err != nil { + t.Fatalf("err: %v", err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + t.Fatalf("expected status code 201, got: %d", resp.StatusCode) + } + + // now one without a body + if err := req.SetBody(nil); err != nil { + t.Fatalf("err: %v", err) + } + + resp, err = client.Do(req) + if err != nil { + t.Fatalf("err: %v", err) + } + resp.Body.Close() + + if resp.StatusCode != http.StatusCreated { + t.Fatalf("expected status code 201, got: %d", resp.StatusCode) + } + + if atomic.LoadInt32(&redirects) != 2 { + t.Fatalf("Expected the client to be redirected 2 times, got: %d", atomic.LoadInt32(&redirects)) + } +}