Repository: go-resty/resty
Pull request #380 (2020-09-27 15:41:04)

fix: get body size 0 (#300)
when the http status code is 307 or 308, the request.Body will be closed, after that the func GetBody will be called but the bodyBuf was closed, then we will get an error http: ContentLength=xxx with Body length 0

shiguangyin: see https://github.com/golang/go/blob/72a9dec156408a87548deb920a67b8bf787062db/src/net/http/client.go#L737
codecov[bot]: # [Codecov](https://codecov.io/gh/go-resty/resty/pull/380?src=pr&el=h1) Report
> Merging [#380](https://codecov.io/gh/go-resty/resty/pull/380?src=pr&el=desc) into [master](https://codecov.io/gh/go-resty/resty/commit/608c8d777d0edf2d69b0510a5c5ddf716177f187?el=desc) will **decrease** coverage by `1.08%`.
> The diff coverage is `73.33%`.

[![Impacted file tree graph](https://codecov.io/gh/go-resty/resty/pull/380/graphs/tree.svg?width=650&height=150&src=pr&token=9t4RCu3fVO)](https://codecov.io/gh/go-resty/resty/pull/380?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #380      +/-   ##
==========================================
- Coverage   96.20%   95.11%   -1.09%     
==========================================
  Files          10       10              
  Lines        1238     1003     -235     
==========================================
- Hits         1191      954     -237     
- Misses         26       27       +1     
- Partials       21       22       +1     
```


| [Impacted Files](https://codecov.io/gh/go-resty/resty/pull/380?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [client.go](https://codecov.io/gh/go-resty/resty/pull/380/diff?src=pr&el=tree#diff-Y2xpZW50Lmdv) | `95.78% <ø> (-0.95%)` | :arrow_down: |
| [middleware.go](https://codecov.io/gh/go-resty/resty/pull/380/diff?src=pr&el=tree#diff-bWlkZGxld2FyZS5nbw==) | `89.79% <73.33%> (-1.75%)` | :arrow_down: |
| [redirect.go](https://codecov.io/gh/go-resty/resty/pull/380/diff?src=pr&el=tree#diff-cmVkaXJlY3QuZ28=) | `92.00% <0.00%> (-2.12%)` | :arrow_down: |
| [util.go](https://codecov.io/gh/go-resty/resty/pull/380/diff?src=pr&el=tree#diff-dXRpbC5nbw==) | `91.93% <0.00%> (-2.01%)` | :arrow_down: |
| [retry.go](https://codecov.io/gh/go-resty/resty/pull/380/diff?src=pr&el=tree#diff-cmV0cnkuZ28=) | `97.01% <0.00%> (-0.36%)` | :arrow_down: |
| [resty.go](https://codecov.io/gh/go-resty/resty/pull/380/diff?src=pr&el=tree#diff-cmVzdHkuZ28=) | `100.00% <0.00%> (ø)` | |
| [trace.go](https://codecov.io/gh/go-resty/resty/pull/380/diff?src=pr&el=tree#diff-dHJhY2UuZ28=) | `100.00% <0.00%> (ø)` | |
| [request.go](https://codecov.io/gh/go-resty/resty/pull/380/diff?src=pr&el=tree#diff-cmVxdWVzdC5nbw==) | `100.00% <0.00%> (ø)` | |
| ... and [2 more](https://codecov.io/gh/go-resty/resty/pull/380/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/go-resty/resty/pull/380?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/go-resty/resty/pull/380?src=pr&el=footer). Last update [608c8d7...955022c](https://codecov.io/gh/go-resty/resty/pull/380?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/README.md b/README.md
index df073cd..123c759 100644
--- a/README.md
+++ b/README.md
@@ -653,7 +653,7 @@ client.
     SetRetryMaxWaitTime(20 * time.Second).
     // SetRetryAfter sets callback to calculate wait time between retries.
     // Default (nil) implies exponential backoff with jitter
-    SetRetryAfter(func(client *Client, resp *Response) (time.Duration, error) {
+    SetRetryAfter(func(client *resty.Client, resp *resty.Response) (time.Duration, error) {
         return 0, errors.New("quota exceeded")
     })
 ```
diff --git a/client.go b/client.go
index b5730c1..3bef2d6 100644
--- a/client.go
+++ b/client.go
@@ -54,6 +54,7 @@ var (
 	hdrContentLengthKey   = http.CanonicalHeaderKey("Content-Length")
 	hdrContentEncodingKey = http.CanonicalHeaderKey("Content-Encoding")
 	hdrAuthorizationKey   = http.CanonicalHeaderKey("Authorization")
+	hdrLocationKey        = http.CanonicalHeaderKey("Location")
 
 	plainTextType   = "text/plain; charset=utf-8"
 	jsonContentType = "application/json"
diff --git a/middleware.go b/middleware.go
index 8556206..50ff448 100644
--- a/middleware.go
+++ b/middleware.go
@@ -206,29 +206,16 @@ func createHTTPRequest(c *Client, r *Request) (err error) {
 		r.RawRequest = r.RawRequest.WithContext(r.ctx)
 	}
 
+	bodyCopy, err := getBodyCopy(r)
+	if err != nil {
+		return err
+	}
+
 	// assign get body func for the underlying raw request instance
 	r.RawRequest.GetBody = func() (io.ReadCloser, error) {
-		// If r.bodyBuf present, return the copy
-		if r.bodyBuf != nil {
-			return ioutil.NopCloser(bytes.NewReader(r.bodyBuf.Bytes())), nil
-		}
-
-		// Maybe body is `io.Reader`.
-		// Note: Resty user have to watchout for large body size of `io.Reader`
-		if r.RawRequest.Body != nil {
-			b, err := ioutil.ReadAll(r.RawRequest.Body)
-			if err != nil {
-				return nil, err
-			}
-
-			// Restore the Body
-			closeq(r.RawRequest.Body)
-			r.RawRequest.Body = ioutil.NopCloser(bytes.NewBuffer(b))
-
-			// Return the Body bytes
-			return ioutil.NopCloser(bytes.NewBuffer(b)), nil
+		if bodyCopy != nil {
+			return ioutil.NopCloser(bytes.NewReader(bodyCopy.Bytes())), nil
 		}
-
 		return nil, nil
 	}
 
@@ -530,3 +517,27 @@ func saveResponseIntoFile(c *Client, res *Response) error {
 
 	return nil
 }
+
+func getBodyCopy(r *Request) (*bytes.Buffer, error) {
+	// If r.bodyBuf present, return the copy
+	if r.bodyBuf != nil {
+		return bytes.NewBuffer(r.bodyBuf.Bytes()), nil
+	}
+
+	// Maybe body is `io.Reader`.
+	// Note: Resty user have to watchout for large body size of `io.Reader`
+	if r.RawRequest.Body != nil {
+		b, err := ioutil.ReadAll(r.RawRequest.Body)
+		if err != nil {
+			return nil, err
+		}
+
+		// Restore the Body
+		closeq(r.RawRequest.Body)
+		r.RawRequest.Body = ioutil.NopCloser(bytes.NewBuffer(b))
+
+		// Return the Body bytes
+		return bytes.NewBuffer(b), nil
+	}
+	return nil, nil
+}
\ No newline at end of file


Test patch:
diff --git a/request_test.go b/request_test.go
index fc8b87d..ce864e0 100644
--- a/request_test.go
+++ b/request_test.go
@@ -1807,3 +1807,37 @@ func TestDebugLoggerRequestBodyTooLarge(t *testing.T) {
 	assertNotNil(t, resp)
 	assertEqual(t, true, strings.Contains(output.String(), "REQUEST TOO LARGE"))
 }
+
+func TestPostMapTemporaryRedirect(t *testing.T)  {
+	ts := createPostServer(t)
+	defer ts.Close()
+
+	c := dc()
+	resp, err := c.R().SetBody(map[string]string{"username":"testuser", "password": "testpass"}).
+		Post(ts.URL + "/redirect")
+
+	assertNil(t, err)
+	assertNotNil(t, resp)
+	assertEqual(t, http.StatusOK, resp.StatusCode())
+}
+
+type brokenReadCloser struct{}
+
+func (b brokenReadCloser) Read(p []byte) (n int, err error) {
+	return 0, errors.New("read error")
+}
+
+func (b brokenReadCloser) Close() error {
+	return nil
+}
+
+func TestPostBodyError(t *testing.T)  {
+	ts := createPostServer(t)
+	defer ts.Close()
+
+	c := dc()
+	resp, err := c.R().SetBody(brokenReadCloser{}).Post(ts.URL + "/redirect")
+	assertNotNil(t, err)
+	assertEqual(t, "read error", err.Error())
+	assertNil(t, resp)
+}
\ No newline at end of file
diff --git a/resty_test.go b/resty_test.go
index 392eac5..a9f42a8 100644
--- a/resty_test.go
+++ b/resty_test.go
@@ -287,6 +287,9 @@ func createPostServer(t *testing.T) *httptest.Server {
 
 					return
 				}
+			} else if r.URL.Path == "/redirect" {
+				w.Header().Set(hdrLocationKey, "/login")
+				w.WriteHeader(http.StatusTemporaryRedirect)
 			}
 		}
 	})