Skip to content

Commit 4f0f828

Browse files
authored
Merge pull request #2 from gojek/NOJIRA_bug_clone_request
[BUGFIX]: fix nil pointer bug in request.Clone()
2 parents 101a992 + c7395fd commit 4f0f828

File tree

2 files changed

+35
-24
lines changed

2 files changed

+35
-24
lines changed

http/request.go

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -50,21 +50,19 @@ func NewHTTPRequest(req *http.Request) (*Request, error) {
5050

5151
// Copy creates a deep copy of this request
5252
func (r *Request) Clone() (fiber.Request, error) {
53-
body, err := r.GetBody()
54-
if err != nil {
55-
return nil, err
56-
}
53+
bodyReader := bytes.NewReader(r.Payload())
5754

58-
proxyRequest, err := http.NewRequest(r.Method, r.URL.String(), body)
55+
proxyRequest, err := http.NewRequest(r.Method, r.URL.String(), bodyReader)
5956
if err != nil {
6057
return nil, err
6158
}
6259

63-
for key, values := range r.Header() {
64-
for i := range values {
65-
proxyRequest.Header.Add(key, values[i])
66-
}
60+
proxyRequest.GetBody = func() (io.ReadCloser, error) {
61+
return ioutil.NopCloser(bodyReader), nil
6762
}
63+
64+
proxyRequest.Header = r.Header()
65+
6866
return &Request{CachedPayload: r.CachedPayload, Request: proxyRequest}, nil
6967
}
7068

http/request_test.go

Lines changed: 28 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ import (
99
"testing"
1010
"time"
1111

12-
"github.com/stretchr/testify/mock"
13-
14-
"github.com/stretchr/testify/require"
12+
"github.com/gojek/fiber"
1513
fiberHTTP "github.com/gojek/fiber/http"
1614
"github.com/gojek/fiber/internal/testutils"
15+
"github.com/stretchr/testify/mock"
16+
"github.com/stretchr/testify/require"
1717
)
1818

1919
var requestPayload, _ = testutils.ReadFile("../internal/testdata/request_payload.json")
@@ -100,25 +100,38 @@ func TestRequest_Clone(t *testing.T) {
100100
"http://localhost:9999/api/mock",
101101
strings.NewReader("*** request payload ***"),
102102
))
103+
req.Request.Header = http.Header{
104+
"fiber-key": []string{
105+
fmt.Sprintf("test-%d", time.Now().Unix()),
106+
},
107+
}
103108

104-
req.Request.Header.Add("fiber-key", fmt.Sprintf("test-%d", time.Now().Unix()))
105-
106-
cloned, err := req.Clone()
107-
require.NoError(t, err)
109+
compareRequests := func(t *testing.T, original *fiberHTTP.Request, clone fiber.Request) {
110+
clonedReq, ok := clone.(*fiberHTTP.Request)
111+
require.True(t, ok, "clone should have the same type as the original")
108112

109-
copyReq, ok := cloned.(*fiberHTTP.Request)
113+
require.NotEqual(t, original, clonedReq)
114+
require.Equal(t, original.Header(), clonedReq.Header())
110115

111-
require.True(t, ok, "clone should have the same type as the original")
116+
expectedBody, err := original.GetBody()
117+
require.NoError(t, err)
118+
require.Equal(t, readBytes(expectedBody), readBytes(clonedReq.Body))
119+
require.Equal(t, req.URL, clonedReq.URL)
120+
}
112121

113-
require.NotEqual(t, req, copyReq)
114-
require.Equal(t, req.Header(), copyReq.Header())
122+
clone, err := req.Clone()
123+
require.NoError(t, err)
115124

116-
expectedBody, err1 := req.GetBody()
125+
t.Run("success", func(t *testing.T) {
126+
compareRequests(t, req, clone)
127+
})
117128

118-
require.NoError(t, err1)
119-
require.Equal(t, readBytes(expectedBody), readBytes(copyReq.Body))
129+
cloneOfClone, err := clone.Clone()
130+
require.NoError(t, err)
120131

121-
require.Equal(t, req.URL, copyReq.URL)
132+
t.Run("success | clone of cloned", func(t *testing.T) {
133+
compareRequests(t, req, cloneOfClone)
134+
})
122135
}
123136

124137
func TestRequest_OperationName(t *testing.T) {

0 commit comments

Comments
 (0)