From ba33b4b43c584608f544cc41880866db54f07c2d Mon Sep 17 00:00:00 2001 From: Zhi Yan Liu Date: Fri, 7 Jul 2017 14:03:39 +0800 Subject: [PATCH 1/5] Adds an option to support GET method with request body Implements `SetAllowGetMethodPayload()` on `go-resty` client to allow developer decides if request includes payload with GET method. fixes #81 --- client.go | 55 ++++++++++++++++++++++++++++++++++---------------- client_test.go | 20 +++++++++++++++++- default.go | 5 +++++ middleware.go | 3 +-- request.go | 2 +- resty_test.go | 6 ++++++ 6 files changed, 70 insertions(+), 21 deletions(-) diff --git a/client.go b/client.go index 187d71ba..4eac9e4e 100644 --- a/client.go +++ b/client.go @@ -71,21 +71,22 @@ var ( // Client type is used for HTTP/RESTful global values // for all request raised from the client type Client struct { - HostURL string - QueryParam url.Values - FormData url.Values - Header http.Header - UserInfo *User - Token string - Cookies []*http.Cookie - Error reflect.Type - Debug bool - DisableWarn bool - Log *log.Logger - RetryCount int - RetryWaitTime time.Duration - RetryMaxWaitTime time.Duration - RetryConditions []RetryConditionFunc + HostURL string + QueryParam url.Values + FormData url.Values + Header http.Header + UserInfo *User + Token string + Cookies []*http.Cookie + Error reflect.Type + Debug bool + DisableWarn bool + AllowGetMethodPayload bool + Log *log.Logger + RetryCount int + RetryWaitTime time.Duration + RetryMaxWaitTime time.Duration + RetryConditions []RetryConditionFunc httpClient *http.Client transport *http.Transport @@ -373,6 +374,15 @@ func (c *Client) SetDisableWarn(d bool) *Client { return c } +// SetAllowGetMethodPayload method allows the GET method with payload on `go-resty` client. +// For example: go-resty allows the user sends request with a payload on HTTP GET method. +// resty.SetAllowGetMethodPayload(true) +// +func (c *Client) SetAllowGetMethodPayload(a bool) *Client { + c.AllowGetMethodPayload = a + return c +} + // SetLogger method sets given writer for logging go-resty request and response details. // Default is os.Stderr // file, _ := os.OpenFile("/Users/jeeva/go-resty.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) @@ -851,8 +861,19 @@ func getPointer(v interface{}) interface{} { return reflect.New(vv.Type()).Interface() } -func isPayloadSupported(m string) bool { - return (m == MethodPost || m == MethodPut || m == MethodDelete || m == MethodPatch) +func isPayloadSupported(allowGetMethodPayload bool, m string) bool { + methods := []string{MethodPost, MethodPut, MethodDelete, MethodPatch} + + if allowGetMethodPayload { + methods = append(methods, MethodGet) + } + + for _, method := range methods { + if m == method { + return true + } + } + return false } func typeOf(i interface{}) reflect.Type { diff --git a/client_test.go b/client_test.go index 588a11c5..b5c33327 100644 --- a/client_test.go +++ b/client_test.go @@ -288,7 +288,7 @@ func TestClientOptions(t *testing.T) { SetRetryCount(3) assertEqual(t, 3, DefaultClient.RetryCount) - + rwt := time.Duration(1000) * time.Millisecond SetRetryWaitTime(rwt) assertEqual(t, rwt, DefaultClient.RetryWaitTime) @@ -324,6 +324,9 @@ func TestClientOptions(t *testing.T) { SetDebug(true) assertEqual(t, DefaultClient.Debug, true) + SetAllowGetMethodPayload(true) + assertEqual(t, DefaultClient.AllowGetMethodPayload, true) + SetScheme("http") assertEqual(t, DefaultClient.scheme, "http") @@ -344,3 +347,18 @@ func TestClientPreRequestHook(t *testing.T) { return nil }) } + +func TestClientAllowsGetMethodPayload(t *testing.T) { + ts := createGetServer(t) + defer ts.Close() + + c := dc() + c.SetAllowGetMethodPayload(true) + + payload := "test-payload" + resp, err := c.R().SetBody(payload).Get(ts.URL + "/get-method-payload-test") + + assertError(t, err) + assertEqual(t, http.StatusOK, resp.StatusCode()) + assertEqual(t, payload, resp.String()) +} diff --git a/default.go b/default.go index 2afd0f3d..7677dfaf 100644 --- a/default.go +++ b/default.go @@ -149,6 +149,11 @@ func SetDebug(d bool) *Client { return DefaultClient.SetDebug(d) } +// SetAllowGetMethodPayload method allows the GET method with payload. See `Client.SetAllowGetMethodPayload` for more information. +func SetAllowGetMethodPayload(a bool) *Client { + return DefaultClient.SetAllowGetMethodPayload(a) +} + // SetRetryCount method sets the retry count. See `Client.SetRetryCount` for more information. func SetRetryCount(count int) *Client { return DefaultClient.SetRetryCount(count) diff --git a/middleware.go b/middleware.go index 99fd6ec2..df108196 100644 --- a/middleware.go +++ b/middleware.go @@ -91,8 +91,7 @@ func parseRequestHeader(c *Client, r *Request) error { } func parseRequestBody(c *Client, r *Request) (err error) { - if isPayloadSupported(r.Method) { - + if isPayloadSupported(c.AllowGetMethodPayload, r.Method) { // Handling Multipart if r.isMultiPart && !(r.Method == MethodPatch) { if err = handleMultipart(c, r); err != nil { diff --git a/request.go b/request.go index 2cb62e14..674d7fa8 100644 --- a/request.go +++ b/request.go @@ -439,7 +439,7 @@ func (r *Request) Execute(method, url string) (*Response, error) { func (r *Request) fmtBodyString() (body string) { body = "***** NO CONTENT *****" - if isPayloadSupported(r.Method) { + if isPayloadSupported(r.client.AllowGetMethodPayload, r.Method) { // multipart or form-data if r.isMultiPart || r.isFormData { body = string(r.bodyBuf.Bytes()) diff --git a/resty_test.go b/resty_test.go index df3bb8f2..119062e3 100644 --- a/resty_test.go +++ b/resty_test.go @@ -1187,6 +1187,12 @@ func createGetServer(t *testing.T) *httptest.Server { w.Header().Set("Content-Type", "image/png") w.Header().Set("Content-Length", strconv.Itoa(len(fileBytes))) _, _ = w.Write(fileBytes) + } else if r.URL.Path == "/get-method-payload-test" { + body, err := ioutil.ReadAll(r.Body) + if err != nil { + t.Errorf("Error: could not read get body: %s", err.Error()) + } + _, _ = w.Write(body) } } }) From ba7eb76256277392f645c5d7c7c4adc4eae0f6a6 Mon Sep 17 00:00:00 2001 From: Zhi Yan Liu Date: Fri, 7 Jul 2017 15:02:12 +0800 Subject: [PATCH 2/5] Adds an option to support GET method with request body Implements `SetAllowGetMethodPayload()` on `go-resty` client to allow developer decides if request includes payload with GET method. fixes #81 --- README.md | 7 +++++++ client.go | 15 ++------------- middleware.go | 2 +- request.go | 2 +- 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 6865cfbd..5ac149a7 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,7 @@ Go Resty first released on Sep 15, 2015 then go-resty grew gradually as a very h * Debug mode - clean and informative logging presentation * Gzip - I'm not doing anything here. Go does it automatically * Well tested client library +* Support GET request includes payload resty tested with Go `v1.3` and above. @@ -620,6 +621,12 @@ r.R().Get("/index.html") ``` +#### Allow payload in GET request +```go +// Allow the request with GET method includes payload. This is disabled by default. +resty.SetAllowGetMethodPayload(true) +``` + ## Versioning resty releases versions according to [Semantic Versioning](http://semver.org) diff --git a/client.go b/client.go index 4eac9e4e..f03190f7 100644 --- a/client.go +++ b/client.go @@ -861,19 +861,8 @@ func getPointer(v interface{}) interface{} { return reflect.New(vv.Type()).Interface() } -func isPayloadSupported(allowGetMethodPayload bool, m string) bool { - methods := []string{MethodPost, MethodPut, MethodDelete, MethodPatch} - - if allowGetMethodPayload { - methods = append(methods, MethodGet) - } - - for _, method := range methods { - if m == method { - return true - } - } - return false +func isPayloadSupported(m string, allowMethodGet bool) bool { + return (m == MethodPost || m == MethodPut || m == MethodDelete || m == MethodPatch || (allowMethodGet && m == MethodGet)) } func typeOf(i interface{}) reflect.Type { diff --git a/middleware.go b/middleware.go index df108196..536df862 100644 --- a/middleware.go +++ b/middleware.go @@ -91,7 +91,7 @@ func parseRequestHeader(c *Client, r *Request) error { } func parseRequestBody(c *Client, r *Request) (err error) { - if isPayloadSupported(c.AllowGetMethodPayload, r.Method) { + if isPayloadSupported(r.Method, c.AllowGetMethodPayload) { // Handling Multipart if r.isMultiPart && !(r.Method == MethodPatch) { if err = handleMultipart(c, r); err != nil { diff --git a/request.go b/request.go index 674d7fa8..7335cbcc 100644 --- a/request.go +++ b/request.go @@ -439,7 +439,7 @@ func (r *Request) Execute(method, url string) (*Response, error) { func (r *Request) fmtBodyString() (body string) { body = "***** NO CONTENT *****" - if isPayloadSupported(r.client.AllowGetMethodPayload, r.Method) { + if isPayloadSupported(r.Method, r.client.AllowGetMethodPayload) { // multipart or form-data if r.isMultiPart || r.isFormData { body = string(r.bodyBuf.Bytes()) From a1cb17b8025159723d6e4979484ed953d338e81a Mon Sep 17 00:00:00 2001 From: Zhi Yan Liu Date: Fri, 7 Jul 2017 15:02:12 +0800 Subject: [PATCH 3/5] Adds an option to support GET method with request body Implements `SetAllowGetMethodPayload()` on `go-resty` client to allow developer decides if request includes payload with GET method. fixes #81 --- client_test.go | 15 --------------- resty_test.go | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/client_test.go b/client_test.go index b5c33327..e5c3e93a 100644 --- a/client_test.go +++ b/client_test.go @@ -347,18 +347,3 @@ func TestClientPreRequestHook(t *testing.T) { return nil }) } - -func TestClientAllowsGetMethodPayload(t *testing.T) { - ts := createGetServer(t) - defer ts.Close() - - c := dc() - c.SetAllowGetMethodPayload(true) - - payload := "test-payload" - resp, err := c.R().SetBody(payload).Get(ts.URL + "/get-method-payload-test") - - assertError(t, err) - assertEqual(t, http.StatusOK, resp.StatusCode()) - assertEqual(t, payload, resp.String()) -} diff --git a/resty_test.go b/resty_test.go index 119062e3..28f0006a 100644 --- a/resty_test.go +++ b/resty_test.go @@ -1135,6 +1135,21 @@ func TestSRVInvalidService(t *testing.T) { assertEqual(t, true, strings.Contains(err.Error(), "no such host")) } +func TestAllowsGetMethodPayload(t *testing.T) { + ts := createGetServer(t) + defer ts.Close() + + c := dc() + c.SetAllowGetMethodPayload(true) + + payload := "test-payload" + resp, err := c.R().SetBody(payload).Get(ts.URL + "/get-method-payload-test") + + assertError(t, err) + assertEqual(t, http.StatusOK, resp.StatusCode()) + assertEqual(t, payload, resp.String()) +} + func getTestDataPath() string { pwd, _ := os.Getwd() return pwd + "/test-data" From adf32b2daf585a16f2269a92a5b79e898c0b04b2 Mon Sep 17 00:00:00 2001 From: Zhi Yan Liu Date: Fri, 7 Jul 2017 15:02:12 +0800 Subject: [PATCH 4/5] Makes testcases of supporting GET method with request body to be order-unrelated fixes #81 --- client_test.go | 16 ++++++++++++++++ resty_test.go | 15 --------------- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/client_test.go b/client_test.go index e5c3e93a..b408e613 100644 --- a/client_test.go +++ b/client_test.go @@ -347,3 +347,19 @@ func TestClientPreRequestHook(t *testing.T) { return nil }) } + +func TestClientAllowsGetMethodPayload(t *testing.T) { + ts := createGetServer(t) + defer ts.Close() + + c := dc() + c.SetAllowGetMethodPayload(true) + c.SetPreRequestHook(func(*Client, *Request) error { return nil }) // for coverage + + payload := "test-payload" + resp, err := c.R().SetBody(payload).Get(ts.URL + "/get-method-payload-test") + + assertError(t, err) + assertEqual(t, http.StatusOK, resp.StatusCode()) + assertEqual(t, payload, resp.String()) +} diff --git a/resty_test.go b/resty_test.go index 28f0006a..119062e3 100644 --- a/resty_test.go +++ b/resty_test.go @@ -1135,21 +1135,6 @@ func TestSRVInvalidService(t *testing.T) { assertEqual(t, true, strings.Contains(err.Error(), "no such host")) } -func TestAllowsGetMethodPayload(t *testing.T) { - ts := createGetServer(t) - defer ts.Close() - - c := dc() - c.SetAllowGetMethodPayload(true) - - payload := "test-payload" - resp, err := c.R().SetBody(payload).Get(ts.URL + "/get-method-payload-test") - - assertError(t, err) - assertEqual(t, http.StatusOK, resp.StatusCode()) - assertEqual(t, payload, resp.String()) -} - func getTestDataPath() string { pwd, _ := os.Getwd() return pwd + "/test-data" From f8dc9f29052515915f6458b194b13eae795cc52d Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Fri, 7 Jul 2017 10:42:25 -0700 Subject: [PATCH 5/5] few arrangement --- README.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 5ac149a7..bc6783eb 100644 --- a/README.md +++ b/README.md @@ -38,6 +38,7 @@ Go Resty first released on Sep 15, 2015 then go-resty grew gradually as a very h * Cookies for your request and CookieJar support * SRV Record based request instead of Host URL * Client settings like `Timeout`, `RedirectPolicy`, `Proxy`, `TLSClientConfig`, `Transport`, etc. +* Optionally allows GET request with payload, see [SetAllowGetMethodPayload](https://godoc.org/github.com/go-resty/resty#Client.SetOutputDirectory#Client.SetAllowGetMethodPayload) * resty design * Have client level settings & options and also override at Request level if you want to * Request and Response middlewares @@ -47,7 +48,6 @@ Go Resty first released on Sep 15, 2015 then go-resty grew gradually as a very h * Debug mode - clean and informative logging presentation * Gzip - I'm not doing anything here. Go does it automatically * Well tested client library -* Support GET request includes payload resty tested with Go `v1.3` and above. @@ -512,6 +512,12 @@ resty.SetRESTMode() resty.SetHTTPMode() ``` +#### Allow GET request with Payload +```go +// Allow GET request with Payload. This is disabled by default. +resty.SetAllowGetMethodPayload(true) +``` + #### Wanna Multiple Clients ```go // Here you go! @@ -621,12 +627,6 @@ r.R().Get("/index.html") ``` -#### Allow payload in GET request -```go -// Allow the request with GET method includes payload. This is disabled by default. -resty.SetAllowGetMethodPayload(true) -``` - ## Versioning resty releases versions according to [Semantic Versioning](http://semver.org)