Skip to content

Commit

Permalink
Fix signer receiving a drained body on retries
Browse files Browse the repository at this point in the history
Signed-off-by: Arash Ouji <[email protected]>
  • Loading branch information
aouji committed Sep 21, 2024
1 parent bd5d233 commit f6a14dd
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 4 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/)
- Fix ISM Transition to omitempty Conditions field ([#609](https://github.com/opensearch-project/opensearch-go/pull/609))
- Fix ISM Allocation field types ([#609](https://github.com/opensearch-project/opensearch-go/pull/609))
- Fix ISM Error Notification types ([#612](https://github.com/opensearch-project/opensearch-go/pull/612))
- Fix signer receiving drained body on retries ([#620](https://github.com/opensearch-project/opensearch-go/pull/620))

### Security

Expand Down
8 changes: 4 additions & 4 deletions opensearchtransport/opensearchtransport.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,10 +292,6 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
c.setReqURL(conn.URL, req)
c.setReqAuth(conn.URL, req)

if err = c.signRequest(req); err != nil {
return nil, fmt.Errorf("failed to sign request: %w", err)
}

if !c.disableRetry && i > 0 && req.Body != nil && req.Body != http.NoBody {
body, err := req.GetBody()
if err != nil {
Expand All @@ -304,6 +300,10 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) {
req.Body = body
}

if err = c.signRequest(req); err != nil {
return nil, fmt.Errorf("failed to sign request: %w", err)
}

// Set up time measures and execute the request
start := time.Now().UTC()
res, err = c.transport.RoundTrip(req)
Expand Down
45 changes: 45 additions & 0 deletions opensearchtransport/opensearchtransport_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,13 @@ type mockSigner struct {
SampleKey string
SampleValue string
ReturnError bool
testHook func(*http.Request)
}

func (m *mockSigner) SignRequest(req *http.Request) error {
if m.testHook != nil {
m.testHook(req)
}
if m.ReturnError {
return fmt.Errorf("invalid data")
}
Expand Down Expand Up @@ -732,6 +736,47 @@ func TestTransportPerformRetries(t *testing.T) {
}
})

t.Run("Signer can sign correctly during retry", func(t *testing.T) {
u, _ := url.Parse("https://foo.com/bar")
signer := mockSigner{}
callsToSigner := 0
expectedBody := "FOOBAR"

signer.testHook = func(req *http.Request) {
callsToSigner++
body, err := io.ReadAll(req.Body)
if err != nil {
panic(err)
}
if string(body) != expectedBody {
t.Fatalf("request %d body: expected %q, got %q", callsToSigner, expectedBody, body)
}
}

tp, _ := New(
Config{
URLs: []*url.URL{u},
Signer: &signer,
Transport: &mockTransp{
RoundTripFunc: func(req *http.Request) (*http.Response, error) {
return &http.Response{Status: "MOCK", StatusCode: http.StatusBadGateway}, nil
},
},
},
)

req, _ := http.NewRequest(http.MethodPost, "/abc", strings.NewReader(expectedBody))
//nolint:bodyclose // Mock response does not have a body to close
_, err := tp.Perform(req)
if err != nil {
t.Fatalf("Unexpected error: %s", err)
}

if callsToSigner != 4 {
t.Fatalf("expected 4 requests, got %d", callsToSigner)
}
})

t.Run("Don't retry request on regular error", func(t *testing.T) {
var i int

Expand Down

0 comments on commit f6a14dd

Please sign in to comment.