Skip to content

Commit 3f1ca5e

Browse files
authored
Fix signer receiving a drained body on retries (#620)
Signed-off-by: aouji <arash.ouji@gmail.com>
1 parent 5dd21aa commit 3f1ca5e

File tree

3 files changed

+50
-4
lines changed

3 files changed

+50
-4
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
1818
- Fix ISM Transition to omitempty Conditions field ([#609](https://github.com/opensearch-project/opensearch-go/pull/609))
1919
- Fix ISM Allocation field types ([#609](https://github.com/opensearch-project/opensearch-go/pull/609))
2020
- Fix ISM Error Notification types ([#612](https://github.com/opensearch-project/opensearch-go/pull/612))
21+
- Fix signer receiving drained body on retries ([#620](https://github.com/opensearch-project/opensearch-go/pull/620))
2122

2223
### Security
2324

opensearchtransport/opensearchtransport.go

+4-4
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,6 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
292292
c.setReqURL(conn.URL, req)
293293
c.setReqAuth(conn.URL, req)
294294

295-
if err = c.signRequest(req); err != nil {
296-
return nil, fmt.Errorf("failed to sign request: %w", err)
297-
}
298-
299295
if !c.disableRetry && i > 0 && req.Body != nil && req.Body != http.NoBody {
300296
body, err := req.GetBody()
301297
if err != nil {
@@ -304,6 +300,10 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
304300
req.Body = body
305301
}
306302

303+
if err = c.signRequest(req); err != nil {
304+
return nil, fmt.Errorf("failed to sign request: %w", err)
305+
}
306+
307307
// Set up time measures and execute the request
308308
start := time.Now().UTC()
309309
res, err = c.transport.RoundTrip(req)

opensearchtransport/opensearchtransport_internal_test.go

+45
Original file line numberDiff line numberDiff line change
@@ -64,9 +64,13 @@ type mockSigner struct {
6464
SampleKey string
6565
SampleValue string
6666
ReturnError bool
67+
testHook func(*http.Request)
6768
}
6869

6970
func (m *mockSigner) SignRequest(req *http.Request) error {
71+
if m.testHook != nil {
72+
m.testHook(req)
73+
}
7074
if m.ReturnError {
7175
return fmt.Errorf("invalid data")
7276
}
@@ -732,6 +736,47 @@ func TestTransportPerformRetries(t *testing.T) {
732736
}
733737
})
734738

739+
t.Run("Signer can sign correctly during retry", func(t *testing.T) {
740+
u, _ := url.Parse("https://foo.com/bar")
741+
signer := mockSigner{}
742+
callsToSigner := 0
743+
expectedBody := "FOOBAR"
744+
745+
signer.testHook = func(req *http.Request) {
746+
callsToSigner++
747+
body, err := io.ReadAll(req.Body)
748+
if err != nil {
749+
panic(err)
750+
}
751+
if string(body) != expectedBody {
752+
t.Fatalf("request %d body: expected %q, got %q", callsToSigner, expectedBody, body)
753+
}
754+
}
755+
756+
tp, _ := New(
757+
Config{
758+
URLs: []*url.URL{u},
759+
Signer: &signer,
760+
Transport: &mockTransp{
761+
RoundTripFunc: func(req *http.Request) (*http.Response, error) {
762+
return &http.Response{Status: "MOCK", StatusCode: http.StatusBadGateway}, nil
763+
},
764+
},
765+
},
766+
)
767+
768+
req, _ := http.NewRequest(http.MethodPost, "/abc", strings.NewReader(expectedBody))
769+
//nolint:bodyclose // Mock response does not have a body to close
770+
_, err := tp.Perform(req)
771+
if err != nil {
772+
t.Fatalf("Unexpected error: %s", err)
773+
}
774+
775+
if callsToSigner != 4 {
776+
t.Fatalf("expected 4 requests, got %d", callsToSigner)
777+
}
778+
})
779+
735780
t.Run("Don't retry request on regular error", func(t *testing.T) {
736781
var i int
737782

0 commit comments

Comments
 (0)