Skip to content

Commit

Permalink
fix: BaseService.Request invoked without result does not close http.R…
Browse files Browse the repository at this point in the history
…esponse (#176)

Fixes: #175

This commit modifies BaseService.Request() so that even if a response is not expected (i.e. the `result` parameter is nil), we'll close the response body to guard against leaking streams.   When the response body is not closed, the connection is not returned to "idle" state, which then results in excessive memory use and a quick MaxConnsPerHost limit exhaustion.

Signed-off-by: Rafał Bigaj <rafal.bigaj@pl.ibm.com>

* fix: additional tweaks

Signed-off-by: Phil Adams <phil_adams@us.ibm.com>
  • Loading branch information
rafalbigaj committed Feb 23, 2023
1 parent 7d782ce commit a8c0324
Showing 1 changed file with 7 additions and 3 deletions.
10 changes: 7 additions & 3 deletions v5/core/base_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta

// If debug is enabled, then dump the request.
if GetLogger().IsLogLevelEnabled(LevelDebug) {
buf, dumpErr := httputil.DumpRequestOut(req, req.Body != nil)
buf, dumpErr := httputil.DumpRequestOut(req, !IsNil(req.Body))
if dumpErr == nil {
GetLogger().Debug("Request:\n%s\n", RedactSecrets(string(buf)))
} else {
Expand All @@ -407,7 +407,7 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta

// If debug is enabled, then dump the response.
if GetLogger().IsLogLevelEnabled(LevelDebug) {
buf, dumpErr := httputil.DumpResponse(httpResponse, httpResponse.Body != nil)
buf, dumpErr := httputil.DumpResponse(httpResponse, !IsNil(httpResponse.Body))
if err == nil {
GetLogger().Debug("Response:\n%s\n", RedactSecrets(string(buf)))
} else {
Expand All @@ -430,7 +430,7 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
var responseBody []byte

// First, read the response body into a byte array.
if httpResponse.Body != nil {
if !IsNil(httpResponse.Body) {
var readErr error

defer httpResponse.Body.Close() // #nosec G307
Expand Down Expand Up @@ -533,6 +533,10 @@ func (service *BaseService) Request(req *http.Request, result interface{}) (deta
return
}
}
} else if !IsNil(httpResponse.Body) {
// We weren't expecting a response, but we have a reponse body,
// so we need to close it now since we're not going to consume it.
_ = httpResponse.Body.Close()
}

return
Expand Down

0 comments on commit a8c0324

Please sign in to comment.