From 16455dbde685f8f95d9f29471a2307e0d62fdbb8 Mon Sep 17 00:00:00 2001 From: Aris Tzoumas Date: Thu, 24 Nov 2022 00:04:04 +0200 Subject: [PATCH] reuseconn --- .DS_Store | Bin 0 -> 6148 bytes .circleci/config.yml | 48 --- .golangci.yml | 72 ++-- README.md | 63 +-- go.mod | 14 +- go.sum | 57 ++- main.go | 6 +- main_go112.go | 20 - passes/bodyclose/bodyclose.go | 363 ------------------ passes/bodyclose/bodyclose_test.go | 13 - passes/bodyclose/testdata/src/a/a.go | 115 ------ .../bodyclose/testdata/src/a/defer_crash.go | 11 - passes/bodyclose/testdata/src/a/helper.go | 7 - passes/bodyclose/testdata/src/a/issue1.go | 13 - passes/bodyclose/testdata/src/a/issue27.go | 30 -- passes/bodyclose/testdata/src/a/issue3.go | 43 --- passes/bodyclose/testdata/src/a/issue4.go | 20 - passes/bodyclose/testdata/src/a/issue5.go | 15 - passes/bodyclose/testdata/src/a/no_import.go | 5 - plugin/reuseconn.go | 18 + reuseconn/collector.go | 214 +++++++++++ reuseconn/reuseconn.go | 279 ++++++++++++++ reuseconn/reuseconn_test.go | 18 + reuseconn/testdata/src/a/a.go | 61 +++ reuseconn/testdata/src/b/b.go | 21 + reuseconn/testdata/src/util/util.go | 44 +++ 26 files changed, 788 insertions(+), 782 deletions(-) create mode 100644 .DS_Store delete mode 100644 .circleci/config.yml delete mode 100644 main_go112.go delete mode 100644 passes/bodyclose/bodyclose.go delete mode 100644 passes/bodyclose/bodyclose_test.go delete mode 100644 passes/bodyclose/testdata/src/a/a.go delete mode 100644 passes/bodyclose/testdata/src/a/defer_crash.go delete mode 100644 passes/bodyclose/testdata/src/a/helper.go delete mode 100644 passes/bodyclose/testdata/src/a/issue1.go delete mode 100644 passes/bodyclose/testdata/src/a/issue27.go delete mode 100644 passes/bodyclose/testdata/src/a/issue3.go delete mode 100644 passes/bodyclose/testdata/src/a/issue4.go delete mode 100644 passes/bodyclose/testdata/src/a/issue5.go delete mode 100644 passes/bodyclose/testdata/src/a/no_import.go create mode 100644 plugin/reuseconn.go create mode 100644 reuseconn/collector.go create mode 100644 reuseconn/reuseconn.go create mode 100644 reuseconn/reuseconn_test.go create mode 100644 reuseconn/testdata/src/a/a.go create mode 100644 reuseconn/testdata/src/b/b.go create mode 100644 reuseconn/testdata/src/util/util.go diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..adb6279cba4a53ea5896d14baf599aa6b817610a GIT binary patch literal 6148 zcmeHK%}T>S5T0$TCWw%OLXQhx3sx(NcnP(>fDt{Y)W!x4#$8ETdnkpR^@V&ApU0Wq zjZmuKNknI0=G&d0Eb}GoW&uES$HQ%a002Bx!kUlf6QMZig4CRcP?%>Va040?sCcrF z&5qy50KGdOa!6nd1=^4IS3Jg$eW zAitp5sn%IsjR)~rnoRn^`jJ+nL8_A66r{rhQ!dX_HPn-?9;;ztY9kx)e9!L(&FOS^ zztfVP&i^&1Sy0vAK14(!b9hlzz5!3jFentU0XU1qYvW_7vqR)9MzZ zRb^Gd$P6$8%)ozP!0mtD>VIjN`(y@~fp0TF`-4O!3>}sh&DMd2ejh2`ASA&t-6aU6 z!_Z-A5hEzVmLl3x;jS3MmZM*~ywG83(UyZSE8}zA%EH}HgjpT^(xih3EwW_>n1Oi) z>UJ8?`G5TN{eQlQXUqUI@TV9Mjb7C2VoUC9U1*NZT8Vm%N It is the caller's responsibility to close Body. The default HTTP client's Transport may not reuse HTTP/1.x "keep-alive" TCP connections if the Body is not read to completion and closed. diff --git a/go.mod b/go.mod index c50f389..1e49a04 100644 --- a/go.mod +++ b/go.mod @@ -1,8 +1,14 @@ -module github.com/timakin/bodyclose +module github.com/atzoum/reuseconn -go 1.12 +go 1.18 require ( - github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3 - golang.org/x/tools v0.0.0-20190322203728-c1a832b0ad89 + github.com/gostaticanalysis/analysisutil v0.7.1 + golang.org/x/tools v0.3.0 +) + +require ( + github.com/gostaticanalysis/comment v1.4.2 // indirect + golang.org/x/mod v0.7.0 // indirect + golang.org/x/sys v0.2.0 // indirect ) diff --git a/go.sum b/go.sum index 1821293..eb15b4f 100644 --- a/go.sum +++ b/go.sum @@ -1,9 +1,54 @@ -github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3 h1:JVnpOZS+qxli+rgVl98ILOXVNbW+kb5wcxeGx8ShUIw= -github.com/gostaticanalysis/analysisutil v0.0.0-20190318220348-4088753ea4d3/go.mod h1:eEOZF4jCKGi+aprrirO9e7WKB3beBRtWgqGunKl6pKE= +github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/google/go-cmp v0.5.4 h1:L8R9j+yAqZuZjsqh/z+F1NCffTKKLShY6zXTItVIZ8M= +github.com/google/go-cmp v0.5.4/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= +github.com/gostaticanalysis/analysisutil v0.7.1 h1:ZMCjoue3DtDWQ5WyU16YbjbQEQ3VuzwxALrpYd+HeKk= +github.com/gostaticanalysis/analysisutil v0.7.1/go.mod h1:v21E3hY37WKMGSnbsw2S/ojApNWb6C1//mXO48CXbVc= +github.com/gostaticanalysis/comment v1.4.2 h1:hlnx5+S2fY9Zo9ePo4AhgYsYHbM2+eAv8m/s1JiCd6Q= +github.com/gostaticanalysis/comment v1.4.2/go.mod h1:KLUTGDv6HOCotCH8h2erHKmpci2ZoR8VPu34YA2uzdM= +github.com/gostaticanalysis/testutil v0.3.1-0.20210208050101-bfb5c8eec0e4 h1:d2/eIbH9XjD1fFwD5SHv8x168fjbQ9PB8hvs8DSEC08= +github.com/gostaticanalysis/testutil v0.3.1-0.20210208050101-bfb5c8eec0e4/go.mod h1:D+FIZ+7OahH3ePw/izIEeH5I06eKs1IKI4Xr64/Am3M= +github.com/hashicorp/go-version v1.2.1 h1:zEfKbn2+PDgroKdiOzqiE8rsmLqU2uwi5PB5pBJ3TkI= +github.com/hashicorp/go-version v1.2.1/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09ZGVZPK5anwXA= +github.com/otiai10/copy v1.2.0 h1:HvG945u96iNadPoG2/Ja2+AUJeW5YuFQMixq9yirC+k= +github.com/otiai10/copy v1.2.0/go.mod h1:rrF5dJ5F0t/EWSYODDu4j9/vEeYHMkc8jt0zJChqQWw= +github.com/otiai10/curr v0.0.0-20150429015615-9b4961190c95/go.mod h1:9qAhocn7zKJG+0mI8eUu6xqkFDYS2kb2saOteoSB3cE= +github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6j4vs= +github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT91xUo= +github.com/otiai10/mint v1.3.1/go.mod h1:/yxELlJQ0ufhjUwhshSj+wFjZ78CnZ48/1wtmBH1OTc= +github.com/tenntenn/modver v1.0.1 h1:2klLppGhDgzJrScMpkj9Ujy3rXPUspSjAcev9tSEBgA= +github.com/tenntenn/modver v1.0.1/go.mod h1:bePIyQPb7UeioSRkw3Q0XeMhYZSMx9B8ePqg6SAMGH0= +github.com/tenntenn/text/transform v0.0.0-20200319021203-7eef512accb3 h1:f+jULpRQGxTSkNYKJ51yaw6ChIqO+Je8UqsTKN/cDag= +github.com/tenntenn/text/transform v0.0.0-20200319021203-7eef512accb3/go.mod h1:ON8b8w4BN/kE1EOhwT0o+d62W65a6aPw1nouo9LMgyY= +github.com/yuin/goldmark v1.2.1/go.mod h1:3hX8gzYuyVAZsxl0MRgGTJEmQBFcNTphYh9decYSb74= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= -golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= +golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto= +golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA= +golang.org/x/mod v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA= +golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= +golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= +golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= +golang.org/x/net v0.0.0-20201021035429-f5854403a974/go.mod h1:sp8m0HH+o8qH0wwXwYZr8TS3Oi6o0r6Gce1SSxlDquU= +golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= +golang.org/x/sync v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= +golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200930185726-fdedc70b468f/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20210124154548-22da62e12c0c/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A= +golang.org/x/sys v0.2.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/tools v0.0.0-20190311215038-5c2858a9cfe5/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= -golang.org/x/tools v0.0.0-20190322203728-c1a832b0ad89 h1:iWXXYN3edZ3Nd/7I6Rt1sXrWVmhF9bgVtlEJ7BbH124= -golang.org/x/tools v0.0.0-20190322203728-c1a832b0ad89/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= +golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= +golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k= +golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= +golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= +golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= +golang.org/x/tools v0.1.1-0.20210205202024-ef80cdb6ec6d/go.mod h1:9bzcO0MWcOuT0tm1iBGzDVPshzfwoVvREIui8C+MHqU= +golang.org/x/tools v0.1.1-0.20210302220138-2ac05c832e1a/go.mod h1:9bzcO0MWcOuT0tm1iBGzDVPshzfwoVvREIui8C+MHqU= +golang.org/x/tools v0.3.0 h1:SrNbZl6ECOS1qFzgTdQfWXZM9XBkiA6tkFrH9YSTPHM= +golang.org/x/tools v0.3.0/go.mod h1:/rWhSS2+zyEVwoJf8YAX6L2f0ntZ7Kn/mGgAWcipA5k= +golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= +golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/main.go b/main.go index ef32009..35c6a6a 100644 --- a/main.go +++ b/main.go @@ -1,10 +1,8 @@ -// +build !go1.12 - package main import ( - "github.com/timakin/bodyclose/passes/bodyclose" + "github.com/atzoum/reuseconn/reuseconn" "golang.org/x/tools/go/analysis/singlechecker" ) -func main() { singlechecker.Main(bodyclose.Analyzer) } +func main() { singlechecker.Main(reuseconn.Analyzer) } diff --git a/main_go112.go b/main_go112.go deleted file mode 100644 index 34efc13..0000000 --- a/main_go112.go +++ /dev/null @@ -1,20 +0,0 @@ -// +build go1.12 - -package main - -import ( - "github.com/timakin/bodyclose/passes/bodyclose" - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/unitchecker" -) - -// Analyzers returns analyzers of bodyclose. -func analyzers() []*analysis.Analyzer { - return []*analysis.Analyzer{ - bodyclose.Analyzer, - } -} - -func main() { - unitchecker.Main(analyzers()...) -} diff --git a/passes/bodyclose/bodyclose.go b/passes/bodyclose/bodyclose.go deleted file mode 100644 index a7ff30b..0000000 --- a/passes/bodyclose/bodyclose.go +++ /dev/null @@ -1,363 +0,0 @@ -package bodyclose - -import ( - "fmt" - "go/ast" - "go/types" - "strconv" - "strings" - - "github.com/gostaticanalysis/analysisutil" - "golang.org/x/tools/go/analysis" - "golang.org/x/tools/go/analysis/passes/buildssa" - "golang.org/x/tools/go/ssa" -) - -var Analyzer = &analysis.Analyzer{ - Name: "bodyclose", - Doc: Doc, - Run: new(runner).run, - Requires: []*analysis.Analyzer{ - buildssa.Analyzer, - }, -} - -const ( - Doc = "bodyclose checks whether HTTP response body is closed successfully" - - nethttpPath = "net/http" - closeMethod = "Close" -) - -type runner struct { - pass *analysis.Pass - resObj types.Object - resTyp *types.Pointer - bodyObj types.Object - closeMthd *types.Func - skipFile map[*ast.File]bool -} - -// run executes an analysis for the pass. The receiver is passed -// by value because this func is called in parallel for different passes. -func (r runner) run(pass *analysis.Pass) (interface{}, error) { - r.pass = pass - funcs := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs - - r.resObj = analysisutil.LookupFromImports(pass.Pkg.Imports(), nethttpPath, "Response") - if r.resObj == nil { - // skip checking - return nil, nil - } - - resNamed, ok := r.resObj.Type().(*types.Named) - if !ok { - return nil, fmt.Errorf("cannot find http.Response") - } - r.resTyp = types.NewPointer(resNamed) - - resStruct, ok := r.resObj.Type().Underlying().(*types.Struct) - if !ok { - return nil, fmt.Errorf("cannot find http.Response") - } - for i := 0; i < resStruct.NumFields(); i++ { - field := resStruct.Field(i) - if field.Id() == "Body" { - r.bodyObj = field - } - } - if r.bodyObj == nil { - return nil, fmt.Errorf("cannot find the object http.Response.Body") - } - bodyNamed := r.bodyObj.Type().(*types.Named) - bodyItrf := bodyNamed.Underlying().(*types.Interface) - for i := 0; i < bodyItrf.NumMethods(); i++ { - bmthd := bodyItrf.Method(i) - if bmthd.Id() == closeMethod { - r.closeMthd = bmthd - } - } - - r.skipFile = map[*ast.File]bool{} - for _, f := range funcs { - // skip if the function is just referenced - var isreffunc bool - for i := 0; i < f.Signature.Results().Len(); i++ { - if f.Signature.Results().At(i).Type().String() == r.resTyp.String() { - isreffunc = true - } - } - if isreffunc { - continue - } - - for _, b := range f.Blocks { - for i := range b.Instrs { - pos := b.Instrs[i].Pos() - if r.isopen(b, i) { - pass.Reportf(pos, "response body must be closed") - } - } - } - } - - return nil, nil -} - -func (r *runner) isopen(b *ssa.BasicBlock, i int) bool { - call, ok := r.getReqCall(b.Instrs[i]) - if !ok { - return false - } - - if len(*call.Referrers()) == 0 { - return true - } - cRefs := *call.Referrers() - for _, cRef := range cRefs { - val, ok := r.getResVal(cRef) - if !ok { - continue - } - - if len(*val.Referrers()) == 0 { - return true - } - resRefs := *val.Referrers() - for _, resRef := range resRefs { - switch resRef := resRef.(type) { - case *ssa.Store: // Call in Closure function - if len(*resRef.Addr.Referrers()) == 0 { - return true - } - - for _, aref := range *resRef.Addr.Referrers() { - if c, ok := aref.(*ssa.MakeClosure); ok { - f := c.Fn.(*ssa.Function) - if r.noImportedNetHTTP(f) { - // skip this - return false - } - called := r.isClosureCalled(c) - - return r.calledInFunc(f, called) - } - - } - case *ssa.Call: // Indirect function call - if f, ok := resRef.Call.Value.(*ssa.Function); ok { - for _, b := range f.Blocks { - for i := range b.Instrs { - return r.isopen(b, i) - } - } - } - case *ssa.FieldAddr: // Normal reference to response entity - if resRef.Referrers() == nil { - return true - } - - bRefs := *resRef.Referrers() - - for _, bRef := range bRefs { - bOp, ok := r.getBodyOp(bRef) - if !ok { - continue - } - if len(*bOp.Referrers()) == 0 { - return true - } - ccalls := *bOp.Referrers() - for _, ccall := range ccalls { - if r.isCloseCall(ccall) { - return false - } - } - } - } - } - } - - return true -} - -func (r *runner) getReqCall(instr ssa.Instruction) (*ssa.Call, bool) { - call, ok := instr.(*ssa.Call) - if !ok { - return nil, false - } - if !strings.Contains(call.Type().String(), r.resTyp.String()) { - return nil, false - } - return call, true -} - -func (r *runner) getResVal(instr ssa.Instruction) (ssa.Value, bool) { - switch instr := instr.(type) { - case *ssa.FieldAddr: - if instr.X.Type().String() == r.resTyp.String() { - return instr.X.(ssa.Value), true - } - case ssa.Value: - if instr.Type().String() == r.resTyp.String() { - return instr, true - } - } - return nil, false -} - -func (r *runner) getBodyOp(instr ssa.Instruction) (*ssa.UnOp, bool) { - op, ok := instr.(*ssa.UnOp) - if !ok { - return nil, false - } - if op.Type() != r.bodyObj.Type() { - return nil, false - } - return op, true -} - -func (r *runner) isCloseCall(ccall ssa.Instruction) bool { - switch ccall := ccall.(type) { - case *ssa.Defer: - if ccall.Call.Method != nil && ccall.Call.Method.Name() == r.closeMthd.Name() { - return true - } - case *ssa.Call: - if ccall.Call.Method != nil && ccall.Call.Method.Name() == r.closeMthd.Name() { - return true - } - case *ssa.ChangeInterface: - if ccall.Type().String() == "io.Closer" { - closeMtd := ccall.Type().Underlying().(*types.Interface).Method(0) - crs := *ccall.Referrers() - for _, cs := range crs { - if cs, ok := cs.(*ssa.Defer); ok { - if val, ok := cs.Common().Value.(*ssa.Function); ok { - for _, b := range val.Blocks { - for _, instr := range b.Instrs { - if c, ok := instr.(*ssa.Call); ok { - if c.Call.Method == closeMtd { - return true - } - } - } - } - } - } - - if returnOp, ok := cs.(*ssa.Return); ok { - for _, resultValue := range returnOp.Results { - if resultValue.Type().String() == "io.Closer" { - return true - } - } - } - } - } - case *ssa.Return: - for _, resultValue := range ccall.Results { - if resultValue.Type().String() == "io.ReadCloser" { - return true - } - } - } - return false -} - -func (r *runner) isClosureCalled(c *ssa.MakeClosure) bool { - refs := *c.Referrers() - if len(refs) == 0 { - return false - } - for _, ref := range refs { - switch ref.(type) { - case *ssa.Call, *ssa.Defer: - return true - } - } - return false -} - -func (r *runner) noImportedNetHTTP(f *ssa.Function) (ret bool) { - obj := f.Object() - if obj == nil { - return false - } - - file := analysisutil.File(r.pass, obj.Pos()) - if file == nil { - return false - } - - if skip, has := r.skipFile[file]; has { - return skip - } - defer func() { - r.skipFile[file] = ret - }() - - for _, impt := range file.Imports { - path, err := strconv.Unquote(impt.Path.Value) - if err != nil { - continue - } - path = analysisutil.RemoveVendor(path) - if path == nethttpPath { - return false - } - } - - return true -} - -func (r *runner) calledInFunc(f *ssa.Function, called bool) bool { - for _, b := range f.Blocks { - for i, instr := range b.Instrs { - switch instr := instr.(type) { - case *ssa.UnOp: - refs := *instr.Referrers() - if len(refs) == 0 { - return true - } - for _, r := range refs { - if v, ok := r.(ssa.Value); ok { - if ptr, ok := v.Type().(*types.Pointer); !ok || !isNamedType(ptr.Elem(), "io", "ReadCloser") { - continue - } - vrefs := *v.Referrers() - for _, vref := range vrefs { - if vref, ok := vref.(*ssa.UnOp); ok { - vrefs := *vref.Referrers() - if len(vrefs) == 0 { - return true - } - for _, vref := range vrefs { - if c, ok := vref.(*ssa.Call); ok { - if c.Call.Method != nil && c.Call.Method.Name() == closeMethod { - return !called - } - } - } - } - } - } - - } - default: - return r.isopen(b, i) || !called - } - } - } - return false -} - -// isNamedType reports whether t is the named type path.name. -func isNamedType(t types.Type, path, name string) bool { - n, ok := t.(*types.Named) - if !ok { - return false - } - obj := n.Obj() - return obj.Name() == name && obj.Pkg() != nil && obj.Pkg().Path() == path -} diff --git a/passes/bodyclose/bodyclose_test.go b/passes/bodyclose/bodyclose_test.go deleted file mode 100644 index 209c4d6..0000000 --- a/passes/bodyclose/bodyclose_test.go +++ /dev/null @@ -1,13 +0,0 @@ -package bodyclose_test - -import ( - "testing" - - "github.com/timakin/bodyclose/passes/bodyclose" - "golang.org/x/tools/go/analysis/analysistest" -) - -func Test(t *testing.T) { - testdata := analysistest.TestData() - analysistest.Run(t, testdata, bodyclose.Analyzer, "a") -} diff --git a/passes/bodyclose/testdata/src/a/a.go b/passes/bodyclose/testdata/src/a/a.go deleted file mode 100644 index dcf9448..0000000 --- a/passes/bodyclose/testdata/src/a/a.go +++ /dev/null @@ -1,115 +0,0 @@ -package a - -import ( - "fmt" - "net/http" -) - -func f1() { - resp, err := http.Get("http://example.com/") // OK - if err != nil { - // handle error - } - resp.Body.Close() - - resp2, err := http.Get("http://example.com/") // OK - if err != nil { - // handle error - } - resp2.Body.Close() -} - -func f2() { - resp, err := http.Get("http://example.com/") // OK - if err != nil { - // handle error - } - body := resp.Body - body.Close() - - resp2, err := http.Get("http://example.com/") // OK - body2 := resp2.Body - body2.Close() - if err != nil { - // handle error - } -} - -func f3() { - resp, err := http.Get("http://example.com/") // OK - if err != nil { - // handle error - } - defer resp.Body.Close() -} - -func f4() { - resp, err := http.Get("http://example.com/") // want "response body must be closed" - if err != nil { - // handle error - } - fmt.Print(resp) - - resp, err = http.Get("http://example.com/") // want "response body must be closed" - if err != nil { - // handle error - } - fmt.Print(resp.Status) - - resp, err = http.Get("http://example.com/") // want "response body must be closed" - if err != nil { - // handle error - } - fmt.Print(resp.Body) - return -} - -func f5() { - _, err := http.Get("http://example.com/") // want "response body must be closed" - if err != nil { - // handle error - } -} - -func f6() { - http.Get("http://example.com/") // want "response body must be closed" -} - -func f7() { - res, _ := http.Get("http://example.com/") // OK - resCloser := func() { - res.Body.Close() - } - resCloser() -} - -func f8() { - res, _ := http.Get("http://example.com/") // want "response body must be closed" - _ = func() { - res.Body.Close() - } -} - -func f9() { - _ = func() { - res, _ := http.Get("http://example.com/") // OK - res.Body.Close() - } -} - -func f10() { - res, _ := http.Get("http://example.com/") // OK - resCloser := func(res *http.Response) { - res.Body.Close() - } - resCloser(res) -} - -func handleResponse(res *http.Response) { - res.Body.Close() -} - -func f11() { - res, _ := http.Get("http://example.com/") // OK - handleResponse(res) -} diff --git a/passes/bodyclose/testdata/src/a/defer_crash.go b/passes/bodyclose/testdata/src/a/defer_crash.go deleted file mode 100644 index f512855..0000000 --- a/passes/bodyclose/testdata/src/a/defer_crash.go +++ /dev/null @@ -1,11 +0,0 @@ -package a - -import ( - "io" - "net/http" -) - -func testNoCrashOnDefer() { - resp, _ := http.Get("https://example.com") // want "response body must be closed" - defer func(body io.ReadCloser) {}(resp.Body) -} diff --git a/passes/bodyclose/testdata/src/a/helper.go b/passes/bodyclose/testdata/src/a/helper.go deleted file mode 100644 index 18b3185..0000000 --- a/passes/bodyclose/testdata/src/a/helper.go +++ /dev/null @@ -1,7 +0,0 @@ -package a - -import "net/http" - -func doRequestWithoutClose() (*http.Response, error) { - return http.Get("https://example.com") -} diff --git a/passes/bodyclose/testdata/src/a/issue1.go b/passes/bodyclose/testdata/src/a/issue1.go deleted file mode 100644 index f76836b..0000000 --- a/passes/bodyclose/testdata/src/a/issue1.go +++ /dev/null @@ -1,13 +0,0 @@ -package a - -import "net/http" - -func get() *http.Response { - resp, _ := http.Get("https://example.com") - return resp -} - -func main() { - resp := get() - resp.Body.Close() -} diff --git a/passes/bodyclose/testdata/src/a/issue27.go b/passes/bodyclose/testdata/src/a/issue27.go deleted file mode 100644 index b44b3e4..0000000 --- a/passes/bodyclose/testdata/src/a/issue27.go +++ /dev/null @@ -1,30 +0,0 @@ -package a - -import ( - "io" - "net/http" -) - -func issue27_1(url string) (io.ReadCloser, error) { // body should be closed by User - r, err := http.DefaultClient.Get(url) - if err != nil { - return nil, err - } - return r.Body, nil -} - -func issue27_2(url string) (io.Closer, error) { // body should be closed by User - r, err := http.DefaultClient.Get(url) - if err != nil { - return nil, err - } - return r.Body, nil -} - -func issue27_3(url string) (io.Closer, error) { // body should be closed by User - r, err := http.DefaultClient.Get(url) - if err != nil { - return nil, err - } - return r.Body, nil -} diff --git a/passes/bodyclose/testdata/src/a/issue3.go b/passes/bodyclose/testdata/src/a/issue3.go deleted file mode 100644 index 40ad60e..0000000 --- a/passes/bodyclose/testdata/src/a/issue3.go +++ /dev/null @@ -1,43 +0,0 @@ -package a - -import ( - "fmt" - "io" - "net/http" -) - -func closeBody(c io.Closer) { - _ = c.Close() -} - -func issue3_1() { - resp, _ := http.Get("https://example.com") - defer closeBody(resp.Body) -} - -func issue3_2() { - resp, _ := http.Get("https://example.com") - defer func() { - _ = resp.Body.Close() - }() -} - -func issue3_3() { - resp, err := http.DefaultClient.Do(nil) - if err != nil { - // handle err - } - defer func() { fmt.Println(resp.Body.Close()) }() -} - -func funcReceiver(msg string, er error) { - fmt.Println(msg) - if er != nil { - fmt.Println(er) - } -} - -func issue3_4() { - resp, _ := http.Get("https://example.com") - defer func() { funcReceiver("test", resp.Body.Close()) }() -} diff --git a/passes/bodyclose/testdata/src/a/issue4.go b/passes/bodyclose/testdata/src/a/issue4.go deleted file mode 100644 index 28d2470..0000000 --- a/passes/bodyclose/testdata/src/a/issue4.go +++ /dev/null @@ -1,20 +0,0 @@ -package a - -import ( - "io" - "net/http" -) - -func issue4_1() { - resp, _ := http.Get("https://example.com") // want "response body must be closed" - - foo(resp.Body) -} - -func foo(r io.ReadCloser) {} - -func issue4_2() { - resp, _ := http.Get("https://example.com") // want "response body must be closed" - - _ = http.MaxBytesReader(nil, resp.Body, 1024) -} diff --git a/passes/bodyclose/testdata/src/a/issue5.go b/passes/bodyclose/testdata/src/a/issue5.go deleted file mode 100644 index 3e9fd91..0000000 --- a/passes/bodyclose/testdata/src/a/issue5.go +++ /dev/null @@ -1,15 +0,0 @@ -package a - -import ( - "io" - "io/ioutil" - "net/http" -) - -func f12() { - res, _ := http.Get("http://example.com/") // OK - defer func() { - io.Copy(ioutil.Discard, res.Body) - res.Body.Close() - }() -} diff --git a/passes/bodyclose/testdata/src/a/no_import.go b/passes/bodyclose/testdata/src/a/no_import.go deleted file mode 100644 index 3bc5034..0000000 --- a/passes/bodyclose/testdata/src/a/no_import.go +++ /dev/null @@ -1,5 +0,0 @@ -package a - -func doRequestInHelperFunc() { - _, _ = doRequestWithoutClose() // want "response body must be closed" -} diff --git a/plugin/reuseconn.go b/plugin/reuseconn.go new file mode 100644 index 0000000..e04735d --- /dev/null +++ b/plugin/reuseconn.go @@ -0,0 +1,18 @@ +package main + +import ( + "github.com/atzoum/reuseconn/reuseconn" + "golang.org/x/tools/go/analysis" +) + +type analyzerPlugin struct{} + +// This must be implemented +func (*analyzerPlugin) GetAnalyzers() []*analysis.Analyzer { + return []*analysis.Analyzer{ + reuseconn.Analyzer, + } +} + +// This must be defined and named 'AnalyzerPlugin' +var AnalyzerPlugin analyzerPlugin diff --git a/reuseconn/collector.go b/reuseconn/collector.go new file mode 100644 index 0000000..164f87a --- /dev/null +++ b/reuseconn/collector.go @@ -0,0 +1,214 @@ +package reuseconn + +import ( + "go/token" + "go/types" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/buildssa" + "golang.org/x/tools/go/ssa" +) + +type Fn struct { + Pkg, Name string +} +type ResponseDisposers struct { + Fns []Fn +} + +func (*ResponseDisposers) AFact() { + // just a marker method +} + +func (r *ResponseDisposers) String() string { + var fns []string + for _, f := range r.Fns { + fns = append(fns, f.Name) + } + return "responseDisposers:" + strings.Join(fns, ",") +} + +type readCloserFuncCollector struct { + facts map[string]*ResponseDisposers +} + +// analyze collects all functions that read the response body to completion and close it. +func (c *readCloserFuncCollector) analyze(pass *analysis.Pass) { + var disposerFns []*ssa.Function + funcs := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs + for _, f := range funcs { + respOrRespBodyParam := func() *ssa.Parameter { + if f.Signature.Params().Len() == 1 { + paramTypeString := f.Signature.Params().At(0).Type().String() + if paramTypeString == readCloserType || paramTypeString == responseType { + return f.Params[0] + } + } + return nil + }() + if respOrRespBodyParam == nil { + continue + } + if len(*respOrRespBodyParam.Referrers()) == 0 { + continue + } + + var read, closed bool + + switch respOrRespBodyParam.Type().String() { + case readCloserType: // the body was passed as a function parameter + for _, ref := range *respOrRespBodyParam.Referrers() { + if !read && isReadCall(ref) { + read = true + } + if !closed && isCloseCall(ref) { + closed = true + } + } + case responseType: // the whole response was passed as a function parameter + for _, ref := range *respOrRespBodyParam.Referrers() { + if field, okField := ref.(*ssa.FieldAddr); okField { // referrer must get the body field + if p, okPointer := field.Type().(*types.Pointer); okPointer && isNamedType(p.Elem(), "io", "ReadCloser") { + for _, ref := range *field.Referrers() { // body must be read or closed + if isCloseCall(ref) { + closed = true + } + if isReadCall(ref) { + read = true + } + } + } + } + } + } + if read && closed { + disposerFns = append(disposerFns, f) + } + } + if len(disposerFns) > 0 { + var disposerFnsNames []Fn + for _, f := range disposerFns { + disposerFnsNames = append(disposerFnsNames, Fn{Pkg: f.Pkg.Pkg.Path(), Name: f.Name()}) + } + pass.ExportPackageFact(&ResponseDisposers{Fns: disposerFnsNames}) + } +} + +// isDisposeCall returns true if the given instruction is a call to a function that reads and closes the response body. +func (c *readCloserFuncCollector) isDisposeCall(pass *analysis.Pass, call ssa.Instruction) bool { + if call, ok := call.(*ssa.Call); ok { // needs to be a function call + if f, ok := call.Common().Value.(*ssa.Function); ok { + disposerFns := c.getDisposers(pass, f.Package().Pkg) + for _, disposer := range disposerFns { + if f.Name() == disposer.Name && + f.Package().Pkg.Path() == disposer.Pkg { + return true + } + } + } + } + if call, ok := call.(*ssa.Defer); ok { // needs to be a function call + if f, ok := call.Common().Value.(*ssa.Function); ok { + disposerFns := c.getDisposers(pass, f.Package().Pkg) + for _, disposer := range disposerFns { + if f.Name() == disposer.Name && + f.Package().Pkg.Path() == disposer.Pkg { + return true + } + } + } + } + return false +} + +func (c *readCloserFuncCollector) getDisposers(pass *analysis.Pass, pkg *types.Package) []Fn { + if c.facts == nil { + c.facts = make(map[string]*ResponseDisposers) + } + if f, ok := c.facts[pkg.Path()]; ok { + return f.Fns + } + var disposerFns ResponseDisposers + if pass.ImportPackageFact(pkg, &disposerFns) { + c.facts[pkg.Path()] = &disposerFns + } + return disposerFns.Fns +} + +// isCloseCall returns true if the given instruction is a call to a function that closes the response body. +func isCloseCall(ccall ssa.Instruction) bool { + return isMethodCall(ccall, closeMethod, "io.Closer") +} + +// isReadCall returns true if the given instruction is a call to a function that reads the response body. +func isReadCall(ccall ssa.Instruction) bool { + return isMethodCall(ccall, readMethod, "io.Reader") +} + +func isMethodCall(ccall ssa.Instruction, methodName, interfaceName string) bool { + switch ccall := ccall.(type) { + case *ssa.UnOp: // pointer indirection + if ccall.Op == token.MUL { + for _, ref := range *ccall.Referrers() { + if isMethodCall(ref, methodName, interfaceName) { + return true + } + } + } + case *ssa.Defer: // defer call + if ccall.Call.Method != nil && ccall.Call.Method.Name() == methodName { + return true + } + case *ssa.Call: // method call + if ccall.Call.Method != nil && ccall.Call.Method.Name() == methodName { + return true + } + case *ssa.ChangeInterface: + if ccall.Type().String() == interfaceName { + mtd := ccall.Type().Underlying().(*types.Interface).Method(0) + crs := *ccall.Referrers() + for _, cs := range crs { + if cs, ok := cs.(*ssa.Defer); ok { + if val, ok := cs.Common().Value.(*ssa.Function); ok { + for _, b := range val.Blocks { + for _, instr := range b.Instrs { + if c, ok := instr.(*ssa.Call); ok { + if c.Call.Method == mtd { + return true + } + } + } + } + } + } + + if returnOp, ok := cs.(*ssa.Return); ok { + for _, resultValue := range returnOp.Results { + if resultValue.Type().String() == interfaceName { + return true + } + } + } + + if cs, ok := cs.(*ssa.Call); ok { + if cs.Call.Method == mtd { + return true + } + for _, arg := range cs.Call.Args { + if arg.Type().String() == interfaceName { + return true + } + } + } + } + } + case *ssa.Return: + for _, resultValue := range ccall.Results { + if resultValue.Type().String() == "io.ReadCloser" || resultValue.Type().String() == interfaceName { + return true + } + } + } + return false +} diff --git a/reuseconn/reuseconn.go b/reuseconn/reuseconn.go new file mode 100644 index 0000000..6267057 --- /dev/null +++ b/reuseconn/reuseconn.go @@ -0,0 +1,279 @@ +package reuseconn + +import ( + "go/ast" + "go/types" + "strconv" + "strings" + + "github.com/gostaticanalysis/analysisutil" + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/buildssa" + "golang.org/x/tools/go/ssa" +) + +var Analyzer = &analysis.Analyzer{ + Name: "reuseconn", + Doc: Doc, + Run: new(runner).run, + Requires: []*analysis.Analyzer{ + buildssa.Analyzer, + }, + FactTypes: []analysis.Fact{&ResponseDisposers{}}, +} + +const ( + Doc = "reuseconn checks whether HTTP response body is both closed and consumed, so that the underlying TCP connection can be reused" + nethttpPath = "net/http" + closeMethod = "Close" + readMethod = "Read" + responseType = "*net/http.Response" + readCloserType = "io.ReadCloser" +) + +type runner struct { + funcCollector readCloserFuncCollector +} + +// run executes an analysis for the pass. The receiver is passed +// by value because this func is called in parallel for different passes. +func (r *runner) run(pass *analysis.Pass) (interface{}, error) { + r.funcCollector.analyze(pass) + + funcs := pass.ResultOf[buildssa.Analyzer].(*buildssa.SSA).SrcFuncs + + responseObj := analysisutil.LookupFromImports(pass.Pkg.Imports(), nethttpPath, "Response") + if responseObj == nil { + return nil, nil //nolint:nilnil // this is valid + } + + skipFile := map[*ast.File]bool{} + + for _, fu := range funcs { + var skip bool // if the function returns a response it shouldn't be expected to consume or close the body + for i := 0; i < fu.Signature.Results().Len(); i++ { + if fu.Signature.Results().At(i).Type().String() == responseType { + skip = true + break + } + } + if skip { + continue + } + + for _, block := range fu.Blocks { + for i := range block.Instrs { + pos := block.Instrs[i].Pos() + if call, ok := getReqCall(block.Instrs[i]); ok { + if isNotDisposed(call, pass, skipFile, &r.funcCollector) { + pass.Reportf(pos, "response body must be disposed properly in a single function read to completion and closed") + } + } + + } + } + } + + return nil, nil +} + +// isNotDisposed returns true if the response body is not disposed properly +func isNotDisposed(call *ssa.Call, pass *analysis.Pass, skipFile map[*ast.File]bool, funcCollector *readCloserFuncCollector) bool { + callReferrers := *call.Referrers() + if len(callReferrers) == 0 { + return true // if the call is not used at all, then its response is not disposed + } + for _, callReferrer := range callReferrers { + responseVal, ok := getResponseVal(callReferrer) + if !ok { + continue + } + + if len(*responseVal.Referrers()) == 0 { + return true // if the response is not used at all, then it is not disposed + } + + responseReferrers := *responseVal.Referrers() + for _, responseReferrer := range responseReferrers { + switch responseReferrer := responseReferrer.(type) { + case *ssa.Store: // Call in Closure function + if len(*responseReferrer.Addr.Referrers()) == 0 { + return true + } + + for _, addrReferrer := range *responseReferrer.Addr.Referrers() { + if mc, ok := addrReferrer.(*ssa.MakeClosure); ok { + f := mc.Fn.(*ssa.Function) + if noImportedNetHTTP(pass, skipFile, f) { + // skip this + return false + } + called := isClosureCalled(mc) + return calledInFunc(pass, skipFile, funcCollector, f, called) + } + + } + case *ssa.Call, *ssa.Defer: // Indirect function call + return !funcCollector.isDisposeCall(pass, responseReferrer) + case *ssa.FieldAddr: // Reference to response entity + if responseReferrer.Referrers() == nil { + return true + } + bRefs := *responseReferrer.Referrers() + for _, bRef := range bRefs { + bOp, ok := getBodyOp(bRef) + if !ok { + continue + } + if len(*bOp.Referrers()) == 0 { + return true + } + ccalls := *bOp.Referrers() + for _, ccall := range ccalls { + if funcCollector.isDisposeCall(pass, ccall) { + return false + } + } + } + return true + } + } + } + + return true +} + +// getReqCall returns true if it is a call to the http request function +func getReqCall(instr ssa.Instruction) (*ssa.Call, bool) { + if call, ok := instr.(*ssa.Call); ok { + if strings.Contains(call.Type().String(), responseType) { + return call, true + } + } + return nil, false +} + +// getResponseVal returns the response if it can be found +func getResponseVal(instr ssa.Instruction) (ssa.Value, bool) { + switch instr := instr.(type) { + case *ssa.FieldAddr: + if instr.X.Type().String() == responseType { + return instr.X, true + } + case ssa.Value: + if instr.Type().String() == responseType { + return instr, true + } + } + return nil, false +} + +func getBodyOp(instr ssa.Instruction) (*ssa.UnOp, bool) { + op, ok := instr.(*ssa.UnOp) + if !ok { + return nil, false + } + if op.Type().String() != readCloserType { + return nil, false + } + return op, true +} + +func isClosureCalled(c *ssa.MakeClosure) bool { + refs := *c.Referrers() + if len(refs) == 0 { + return false + } + for _, ref := range refs { + switch ref.(type) { + case *ssa.Call, *ssa.Defer: + return true + } + } + return false +} + +func noImportedNetHTTP(pass *analysis.Pass, skipFile map[*ast.File]bool, f *ssa.Function) (ret bool) { + obj := f.Object() + if obj == nil { + return false + } + + file := analysisutil.File(pass, obj.Pos()) + if file == nil { + return false + } + + if skip, has := skipFile[file]; has { + return skip + } + defer func() { + skipFile[file] = ret + }() + + for _, impt := range file.Imports { + path, err := strconv.Unquote(impt.Path.Value) + if err != nil { + continue + } + path = analysisutil.RemoveVendor(path) + if path == nethttpPath { + return false + } + } + + return true +} + +func calledInFunc(pass *analysis.Pass, skipFile map[*ast.File]bool, funcCollector *readCloserFuncCollector, f *ssa.Function, called bool) bool { + for _, b := range f.Blocks { + for i, instr := range b.Instrs { + switch instr := instr.(type) { + case *ssa.UnOp: + refs := *instr.Referrers() + if len(refs) == 0 { + return true + } + for _, r := range refs { + if v, ok := r.(ssa.Value); ok { + if ptr, ok := v.Type().(*types.Pointer); !ok || !isNamedType(ptr.Elem(), "io", "ReadCloser") { + continue + } + vrefs := *v.Referrers() + for _, vref := range vrefs { + if vref, ok := vref.(*ssa.UnOp); ok { + vrefs := *vref.Referrers() + if len(vrefs) == 0 { + return true + } + for _, vref := range vrefs { + if c, ok := vref.(*ssa.Call); ok { + if c.Call.Method != nil && c.Call.Method.Name() == closeMethod { + return !called + } + } + } + } + } + } + + } + default: + if call, ok := getReqCall(b.Instrs[i]); ok { + return isNotDisposed(call, pass, skipFile, funcCollector) || !called + } + } + } + } + return false +} + +// isNamedType reports whether t is the named type path.name. +func isNamedType(t types.Type, path, name string) bool { + n, ok := t.(*types.Named) + if !ok { + return false + } + obj := n.Obj() + return obj.Name() == name && obj.Pkg() != nil && obj.Pkg().Path() == path +} diff --git a/reuseconn/reuseconn_test.go b/reuseconn/reuseconn_test.go new file mode 100644 index 0000000..e890874 --- /dev/null +++ b/reuseconn/reuseconn_test.go @@ -0,0 +1,18 @@ +package reuseconn_test + +import ( + "testing" + + "github.com/atzoum/reuseconn/reuseconn" + "golang.org/x/tools/go/analysis/analysistest" +) + +func TestA(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, reuseconn.Analyzer, "a", "util") +} + +func TestB(t *testing.T) { + testdata := analysistest.TestData() + analysistest.Run(t, testdata, reuseconn.Analyzer, "b", "util") +} diff --git a/reuseconn/testdata/src/a/a.go b/reuseconn/testdata/src/a/a.go new file mode 100644 index 0000000..1ce7110 --- /dev/null +++ b/reuseconn/testdata/src/a/a.go @@ -0,0 +1,61 @@ +package a // want package:`responseDisposers:disposeBody` + +import ( + "io" + "net/http" + "util" +) + +func disposeAfterIf(url string) { + var res *http.Response + var err error + if res, err = http.DefaultClient.Get(url); err != nil { + return + } + util.DisposeResponse(res) +} + +func disposeFunctionCall(url string) { + r, err := http.DefaultClient.Get(url) + if err != nil { + return + } + util.DisposeResponse(r) +} + +func closeFunctionCall(url string) { + r, err := http.DefaultClient.Get(url) // want `response body must be disposed properly in a single function read to completion and closed` + if err != nil { + return + } + util.CloseResponse(r) +} + +func readFunctionCall(url string) { + r, err := http.DefaultClient.Get(url) // want `response body must be disposed properly in a single function read to completion and closed` + if err != nil { + return + } + util.ReadResponse(r) + +} + +func doRequestWithoutDispose() { + _, _ = doRequestWithoutClose() // want `response body must be disposed properly in a single function read to completion and closed` +} + +func doRequestWithLocalDispose() { + r, err := doRequestWithoutClose() + if err == nil { + disposeBody(r.Body) + } +} + +func disposeBody(body io.ReadCloser) { + _, _ = io.Copy(io.Discard, body) + _ = body.Close() +} + +func doRequestWithoutClose() (*http.Response, error) { + return http.Get("https://example.com") +} diff --git a/reuseconn/testdata/src/b/b.go b/reuseconn/testdata/src/b/b.go new file mode 100644 index 0000000..4e7b7f2 --- /dev/null +++ b/reuseconn/testdata/src/b/b.go @@ -0,0 +1,21 @@ +package b + +import ( + "net/http" + "util" +) + +func deferredDispose(url string) { + r, _ := http.DefaultClient.Get(url) + defer util.DisposeResponse(r) +} + +func deferredClose(url string) { + r, _ := http.DefaultClient.Get(url) // want `response body must be disposed properly in a single function read to completion and closed` + defer util.CloseResponse(r) +} + +func deferredAnonDispose(url string) { + r, _ := http.DefaultClient.Get(url) + defer func() { util.DisposeResponse(r) }() +} diff --git a/reuseconn/testdata/src/util/util.go b/reuseconn/testdata/src/util/util.go new file mode 100644 index 0000000..f568a1f --- /dev/null +++ b/reuseconn/testdata/src/util/util.go @@ -0,0 +1,44 @@ +package util // want package:`responseDisposers:DisposeBody,DisposeResponse` + +import ( + "io" + "net/http" +) + +// DisposeBody both reads and closes the body. +func DisposeBody(body io.ReadCloser) { + _, _ = io.Copy(io.Discard, body) + _ = body.Close() +} + +// CloseBody only closes the body. +func CloseBody(body io.ReadCloser) { + _ = body.Close() +} + +// ReadBody only reads the body. +func ReadBody(body io.ReadCloser) { + _, _ = io.Copy(io.Discard, body) +} + +// DisposeResponse both reads and closes the response's body. +func DisposeResponse(resp *http.Response) { + if resp != nil && resp.Body != nil { + _, _ = io.Copy(io.Discard, resp.Body) + _ = resp.Body.Close() + } +} + +// CloseResponse only closes the response's body. +func CloseResponse(resp *http.Response) { + if resp != nil && resp.Body != nil { + _ = resp.Body.Close() + } +} + +// ReadResponse only reads the response's body. +func ReadResponse(resp *http.Response) { + if resp != nil && resp.Body != nil { + _, _ = io.Copy(io.Discard, resp.Body) + } +}