Skip to content

Commit

Permalink
fix: do not leak the response body on Client.Do() (#348)
Browse files Browse the repository at this point in the history
  • Loading branch information
pmalek committed Jun 21, 2023
1 parent 4dbca76 commit d355292
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 19 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,13 @@
- Fix: handle empty array as nil when filling record defaults
[#345](https://github.com/Kong/go-kong/pull/345)
- Fix leaking response body from `Client.Do()`. From now on `kong.Response` does
not contain the `http.Response` field which was in fact leaking the implementation
detail. It now contains the response headers, status and status code.
If users want to get the response body they can still provide the `v` parameter
to `Do()` to get the body unmarshalled into it (or copied into it if an `io.Writer`
was to be provided).
[#348](https://github.com/Kong/go-kong/pull/348)

## [v0.43.0]

Expand Down
40 changes: 25 additions & 15 deletions kong/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -240,44 +240,54 @@ func (c *Client) DoRAW(ctx context.Context, req *http.Request) (*http.Response,
return resp, err
}

// Do executes an HTTP request and returns a kong.Response
func (c *Client) Do(ctx context.Context, req *http.Request,
// Do executes an HTTP request and returns a Response.
//
// The caller can optionally provide v parameter, which when provided will contain
// the response body. Do supports wither an io.Writer (which will contain the
// response body verbatim) or anything else which the body should be unmarshalled
// into.
func (c *Client) Do(
ctx context.Context,
req *http.Request,
v interface{},
) (*Response, error) {
// TODO https://github.com/Kong/go-kong/issues/273 clear this lint ignore
resp, err := c.DoRAW(ctx, req) //nolint:bodyclose
resp, err := c.DoRAW(ctx, req)
if err != nil {
return nil, err
}
defer resp.Body.Close()

// log the response
err = c.logResponse(resp)
if err != nil {
if err = c.logResponse(resp); err != nil {
return nil, err
}

response := newResponse(resp)

// check for API errors
// Check for API errors.
// If an error status code was returned, then parse the body and create
// an API Error out of it.
if err = hasError(resp); err != nil {
return response, err
}

// response
if v != nil {
if writer, ok := v.(io.Writer); ok {
_, err = io.Copy(writer, resp.Body)
switch v := v.(type) {
case io.Writer:
_, err = io.Copy(v, resp.Body)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed copying response body: %w", err)
}
} else {
return response, nil
default:
err = json.NewDecoder(resp.Body).Decode(v)
if err != nil {
return nil, err
return nil, fmt.Errorf("failed decoding response body: %w", err)
}
return response, nil
}
}
return response, err

return response, nil
}

// ErrorOrResponseError helps to handle the case where
Expand Down
14 changes: 10 additions & 4 deletions kong/response.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,20 @@ import (
"time"
)

// Response is a Kong Admin API response. It wraps http.Response.
// Response is a Kong Admin API response.
// It contains the response headers, status and status code.
type Response struct {
*http.Response
// other Kong specific fields
Header http.Header
Status string
StatusCode int
}

func newResponse(res *http.Response) *Response {
return &Response{Response: res}
return &Response{
Header: res.Header,
Status: res.Status,
StatusCode: res.StatusCode,
}
}

func messageFromBody(b []byte) string {
Expand Down

0 comments on commit d355292

Please sign in to comment.