From 57004b9b50cd9556a44311fced1c3a2d82cbe72d Mon Sep 17 00:00:00 2001 From: merlinz01 <158784988+merlinz01@users.noreply.github.com> Date: Wed, 8 May 2024 19:54:37 -0400 Subject: [PATCH 1/4] fix empty request body on retries with compression enabled Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com> --- opensearchtransport/opensearchtransport.go | 9 ++-- .../opensearchtransport_internal_test.go | 41 +++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/opensearchtransport/opensearchtransport.go b/opensearchtransport/opensearchtransport.go index ac8ad057e..c93b2cd5c 100644 --- a/opensearchtransport/opensearchtransport.go +++ b/opensearchtransport/opensearchtransport.go @@ -245,7 +245,9 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) { } req.GetBody = func() (io.ReadCloser, error) { - return io.NopCloser(buf), nil + // We have to return a new reader each time so that retries don't read from an already-consumed body. + reader := bytes.NewReader(buf.Bytes()) + return io.NopCloser(reader), nil } //nolint:errcheck // error is always nil req.Body, _ = req.GetBody() @@ -258,8 +260,9 @@ func (c *Client) Perform(req *http.Request) (*http.Response, error) { //nolint:errcheck // ignored as this is only for logging buf.ReadFrom(req.Body) req.GetBody = func() (io.ReadCloser, error) { - r := buf - return io.NopCloser(&r), nil + // Return a new reader each time + reader := bytes.NewReader(buf.Bytes()) + return io.NopCloser(reader), nil } //nolint:errcheck // error is always nil req.Body, _ = req.GetBody() diff --git a/opensearchtransport/opensearchtransport_internal_test.go b/opensearchtransport/opensearchtransport_internal_test.go index fbe1f26a3..f6baffb00 100644 --- a/opensearchtransport/opensearchtransport_internal_test.go +++ b/opensearchtransport/opensearchtransport_internal_test.go @@ -689,6 +689,47 @@ func TestTransportPerformRetries(t *testing.T) { } }) + t.Run("Reset request body during retry with request body compression", func(t *testing.T) { + var bodies []string + u, _ := url.Parse("https://foo.com/bar") + tp, _ := New( + Config{ + URLs: []*url.URL{u}, + CompressRequestBody: true, + Transport: &mockTransp{ + RoundTripFunc: func(req *http.Request) (*http.Response, error) { + body, err := io.ReadAll(req.Body) + if err != nil { + panic(err) + } + bodies = append(bodies, string(body)) + return &http.Response{Status: "MOCK", StatusCode: http.StatusBadGateway}, nil + }, + }, + }, + ) + + foobar := "FOOBAR" + foobar_gzipped := "\x1f\x8b\b\x00\x00\x00\x00\x00\x00\xffr\xf3\xf7wr\f\x02\x04\x00\x00\xff\xff\x13\xd8\x0en\x06\x00\x00\x00" + + req, _ := http.NewRequest(http.MethodPost, "/abc", strings.NewReader(foobar)) + //nolint:bodyclose // Mock response does not have a body to close + res, err := tp.Perform(req) + if err != nil { + t.Fatalf("Unexpected error: %s", err) + } + _ = res + + if n := len(bodies); n != 4 { + t.Fatalf("expected 4 requests, got %d", n) + } + for i, body := range bodies { + if body != foobar_gzipped { + t.Fatalf("request %d body: expected %q, got %q", i, foobar_gzipped, body) + } + } + }) + t.Run("Don't retry request on regular error", func(t *testing.T) { var i int From cc5052d0eb55c4799aa33bb21c175a4fa053a51f Mon Sep 17 00:00:00 2001 From: merlinz01 <158784988+merlinz01@users.noreply.github.com> Date: Wed, 8 May 2024 20:05:00 -0400 Subject: [PATCH 2/4] update changelog Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com> --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2176c81bf..3eda6c6ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Fixed +- Fixes empty request body on retry with compression enabled. ([#542](https://github.com/opensearch-project/opensearch-go/pull/542)) + ### Security ### Dependencies From cc835d4c43306d6d231658504dc38e332d459bf4 Mon Sep 17 00:00:00 2001 From: merlinz01 <158784988+merlinz01@users.noreply.github.com> Date: Tue, 14 May 2024 11:00:26 -0400 Subject: [PATCH 3/4] fix lint error Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com> --- opensearchtransport/opensearchtransport_internal_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/opensearchtransport/opensearchtransport_internal_test.go b/opensearchtransport/opensearchtransport_internal_test.go index f6baffb00..6daaeba06 100644 --- a/opensearchtransport/opensearchtransport_internal_test.go +++ b/opensearchtransport/opensearchtransport_internal_test.go @@ -710,7 +710,7 @@ func TestTransportPerformRetries(t *testing.T) { ) foobar := "FOOBAR" - foobar_gzipped := "\x1f\x8b\b\x00\x00\x00\x00\x00\x00\xffr\xf3\xf7wr\f\x02\x04\x00\x00\xff\xff\x13\xd8\x0en\x06\x00\x00\x00" + foobarGzipped := "\x1f\x8b\b\x00\x00\x00\x00\x00\x00\xffr\xf3\xf7wr\f\x02\x04\x00\x00\xff\xff\x13\xd8\x0en\x06\x00\x00\x00" req, _ := http.NewRequest(http.MethodPost, "/abc", strings.NewReader(foobar)) //nolint:bodyclose // Mock response does not have a body to close @@ -724,8 +724,8 @@ func TestTransportPerformRetries(t *testing.T) { t.Fatalf("expected 4 requests, got %d", n) } for i, body := range bodies { - if body != foobar_gzipped { - t.Fatalf("request %d body: expected %q, got %q", i, foobar_gzipped, body) + if body != foobarGzipped { + t.Fatalf("request %d body: expected %q, got %q", i, foobarGzipped, body) } } }) From e7ede90ec9bbec190eed6c09eee4f413049b37f7 Mon Sep 17 00:00:00 2001 From: merlinz01 <158784988+merlinz01@users.noreply.github.com> Date: Tue, 14 May 2024 11:00:35 -0400 Subject: [PATCH 4/4] update changelog Signed-off-by: merlinz01 <158784988+merlinz01@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3eda6c6ca..dd3612e30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) ### Fixed -- Fixes empty request body on retry with compression enabled. ([#542](https://github.com/opensearch-project/opensearch-go/pull/542)) +- Fixes empty request body on retry with compression enabled ([#543](https://github.com/opensearch-project/opensearch-go/pull/543)) ### Security