From 15ed9a8f20efa0823261904871b4e35a99bc9597 Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Wed, 20 May 2020 08:41:37 +0530 Subject: [PATCH] MM-25006: Fix occurences of unclosed http bodies (#14521) * MM-25006: Fix occurences of unclosed http bodies And while here, we also use ioutil.Discard to read the remaining body instead of reading with ioutil.ReadAll which allocates a separate byte buffer. * Fix mistake * Address review comments Co-authored-by: mattermod --- api4/system.go | 6 ++++++ app/admin.go | 5 +++++ app/command_loadtest.go | 35 +++++++++++++++++++++------------- app/post_metadata.go | 6 +++++- app/security_update_check.go | 2 +- model/client4.go | 2 +- services/marketplace/client.go | 3 ++- 7 files changed, 42 insertions(+), 17 deletions(-) diff --git a/api4/system.go b/api4/system.go index 456371e426..889ce6f094 100644 --- a/api4/system.go +++ b/api4/system.go @@ -6,6 +6,8 @@ package api4 import ( "encoding/json" "fmt" + "io" + "io/ioutil" "net/http" "runtime" "strconv" @@ -432,6 +434,10 @@ func getRedirectLocation(c *Context, w http.ResponseWriter, r *http.Request) { w.Write([]byte(model.MapToJson(m))) return } + defer func() { + io.Copy(ioutil.Discard, res.Body) + res.Body.Close() + }() location := res.Header.Get("Location") redirectLocationDataCache.AddWithExpiresInSecs(url, location, 3600) // Expires after 1 hour diff --git a/app/admin.go b/app/admin.go index 3242a689dd..e0fa4e6b5e 100644 --- a/app/admin.go +++ b/app/admin.go @@ -6,6 +6,7 @@ package app import ( "fmt" "io" + "io/ioutil" "os" "time" @@ -177,6 +178,10 @@ func (a *App) TestSiteURL(siteURL string) *model.AppError { if err != nil || res.StatusCode != 200 { return model.NewAppError("testSiteURL", "app.admin.test_site_url.failure", nil, "", http.StatusBadRequest) } + defer func() { + _, _ = io.Copy(ioutil.Discard, res.Body) + _ = res.Body.Close() + }() return nil } diff --git a/app/command_loadtest.go b/app/command_loadtest.go index 8bce16ee85..36b19417bd 100644 --- a/app/command_loadtest.go +++ b/app/command_loadtest.go @@ -5,6 +5,7 @@ package app import ( "io" + "io/ioutil" "net/http" "path" "regexp" @@ -472,20 +473,24 @@ func (me *LoadTestProvider) UrlCommand(a *App, args *model.CommandArgs, message } } - var contents io.ReadCloser - if r, err := http.Get(url); err != nil { + r, err := http.Get(url) + if err != nil { return &model.CommandResponse{Text: "Unable to get file", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}, err - } else if r.StatusCode > 400 { + } + defer func() { + io.Copy(ioutil.Discard, r.Body) + r.Body.Close() + }() + + if r.StatusCode > 400 { return &model.CommandResponse{Text: "Unable to get file", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}, errors.Errorf("unexpected status code %d", r.StatusCode) - } else { - contents = r.Body } bytes := make([]byte, 4000) // break contents into 4000 byte posts for { - length, err := contents.Read(bytes) + length, err := r.Body.Read(bytes) if err != nil && err != io.EOF { return &model.CommandResponse{Text: "Encountered error reading file", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}, err } @@ -522,16 +527,20 @@ func (me *LoadTestProvider) JsonCommand(a *App, args *model.CommandArgs, message } } - var contents io.ReadCloser - if r, err := http.Get(url); err != nil { + r, err := http.Get(url) + if err != nil { return &model.CommandResponse{Text: "Unable to get file", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}, err - } else if r.StatusCode > 400 { - return &model.CommandResponse{Text: "Unable to get file", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}, errors.Errorf("unexpected status code %d", r.StatusCode) - } else { - contents = r.Body } - post := model.PostFromJson(contents) + if r.StatusCode > 400 { + return &model.CommandResponse{Text: "Unable to get file", ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}, errors.Errorf("unexpected status code %d", r.StatusCode) + } + defer func() { + io.Copy(ioutil.Discard, r.Body) + r.Body.Close() + }() + + post := model.PostFromJson(r.Body) post.ChannelId = args.ChannelId post.UserId = args.UserId if post.Message == "" { diff --git a/app/post_metadata.go b/app/post_metadata.go index 1cfeea0e97..c2efe89542 100644 --- a/app/post_metadata.go +++ b/app/post_metadata.go @@ -7,6 +7,7 @@ import ( "bytes" "image" "io" + "io/ioutil" "net/http" "net/url" "strconv" @@ -402,7 +403,10 @@ func (a *App) getLinkMetadata(requestURL string, timestamp int64, isNewPost bool } if body != nil { - defer body.Close() + defer func() { + io.Copy(ioutil.Discard, body) + body.Close() + }() } if err == nil { diff --git a/app/security_update_check.go b/app/security_update_check.go index 91c393860a..071d00e7ff 100644 --- a/app/security_update_check.go +++ b/app/security_update_check.go @@ -106,7 +106,7 @@ func (s *Server) DoSecurityUpdateCheck() { } body, err := ioutil.ReadAll(resBody.Body) - res.Body.Close() + resBody.Body.Close() if err != nil || resBody.StatusCode != 200 { mlog.Error("Failed to read security bulletin details") return diff --git a/model/client4.go b/model/client4.go index 1e62abfed1..231397251d 100644 --- a/model/client4.go +++ b/model/client4.go @@ -65,7 +65,7 @@ type Client4 struct { func closeBody(r *http.Response) { if r.Body != nil { - _, _ = ioutil.ReadAll(r.Body) + _, _ = io.Copy(ioutil.Discard, r.Body) _ = r.Body.Close() } } diff --git a/services/marketplace/client.go b/services/marketplace/client.go index bf4595b7e6..3e68484a40 100644 --- a/services/marketplace/client.go +++ b/services/marketplace/client.go @@ -5,6 +5,7 @@ package marketplace import ( "fmt" + "io" "io/ioutil" "net/http" "net/url" @@ -79,7 +80,7 @@ func (c *Client) GetPlugin(filter *model.MarketplacePluginFilter, pluginVersion // closeBody ensures the Body of an http.Response is properly closed. func closeBody(r *http.Response) { if r.Body != nil { - _, _ = ioutil.ReadAll(r.Body) + _, _ = io.Copy(ioutil.Discard, r.Body) _ = r.Body.Close() } }