diff --git a/BUILD.bazel b/BUILD.bazel index fa57a8afb8..75a2795fa8 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -193,7 +193,7 @@ nogo( "//tools/analyzers/featureconfig:go_default_library", "//tools/analyzers/gocognit:go_default_library", "//tools/analyzers/ineffassign:go_default_library", - "//tools/analyzers/httperror:go_default_library", + "//tools/analyzers/httpwriter:go_default_library", "//tools/analyzers/interfacechecker:go_default_library", "//tools/analyzers/logcapitalization:go_default_library", "//tools/analyzers/logruswitherror:go_default_library", diff --git a/changelog/radek_extend-http-analyzer.md b/changelog/radek_extend-http-analyzer.md new file mode 100644 index 0000000000..adc823d4ac --- /dev/null +++ b/changelog/radek_extend-http-analyzer.md @@ -0,0 +1,3 @@ +### Changed + +- Extend `httperror` analyzer to more functions. \ No newline at end of file diff --git a/network/httputil/writer.go b/network/httputil/writer.go index 745b9409e2..de78efffc9 100644 --- a/network/httputil/writer.go +++ b/network/httputil/writer.go @@ -43,6 +43,7 @@ func WriteJson(w http.ResponseWriter, v any) { func WriteSsz(w http.ResponseWriter, respSsz []byte) { w.Header().Set("Content-Length", strconv.Itoa(len(respSsz))) w.Header().Set("Content-Type", api.OctetStreamMediaType) + w.WriteHeader(http.StatusOK) if _, err := io.Copy(w, io.NopCloser(bytes.NewReader(respSsz))); err != nil { log.WithError(err).Error("Could not write response message") } diff --git a/tools/analyzers/httperror/BUILD.bazel b/tools/analyzers/httpwriter/BUILD.bazel similarity index 96% rename from tools/analyzers/httperror/BUILD.bazel rename to tools/analyzers/httpwriter/BUILD.bazel index 5df2bb292e..2b363d99cb 100644 --- a/tools/analyzers/httperror/BUILD.bazel +++ b/tools/analyzers/httpwriter/BUILD.bazel @@ -3,7 +3,7 @@ load("@prysm//tools/go:def.bzl", "go_library") go_library( name = "go_default_library", srcs = ["analyzer.go"], - importpath = "github.com/OffchainLabs/prysm/v7/tools/analyzers/httperror", + importpath = "github.com/OffchainLabs/prysm/v7/tools/analyzers/httpwriter", visibility = ["//visibility:public"], deps = [ "@org_golang_x_tools//go/analysis:go_default_library", diff --git a/tools/analyzers/httperror/analyzer.go b/tools/analyzers/httpwriter/analyzer.go similarity index 85% rename from tools/analyzers/httperror/analyzer.go rename to tools/analyzers/httpwriter/analyzer.go index ed8dfa5046..c292367515 100644 --- a/tools/analyzers/httperror/analyzer.go +++ b/tools/analyzers/httpwriter/analyzer.go @@ -1,4 +1,4 @@ -package httperror +package httpwriter import ( "go/ast" @@ -9,8 +9,8 @@ import ( ) var Analyzer = &analysis.Analyzer{ - Name: "httperror", - Doc: "Ensures calls to httputil.HandleError are immediately followed by a return statement.", + Name: "httpwriter", + Doc: "Ensures that httputil functions which make use of the writer are immediately followed by a return statement.", Requires: []*analysis.Analyzer{ inspect.Analyzer, }, @@ -99,7 +99,7 @@ func checkBlock(pass *analysis.Pass, fn *ast.FuncDecl, block *ast.BlockStmt, nex // Now check the current statement itself: is it (or does it contain) a direct call to httputil.HandleError? // We only consider ExprStmt that are direct CallExpr to httputil.HandleError. - call := findHandleErrorCall(stmt) + call, name := findHandleErrorCall(stmt) if call == nil { continue } @@ -121,7 +121,7 @@ func checkBlock(pass *analysis.Pass, fn *ast.FuncDecl, block *ast.BlockStmt, nex continue } // otherwise it's not a return (even if it's an if/for etc) -> violation - pass.Reportf(stmt.Pos(), "call to httputil.HandleError must be immediately followed by a return statement") + pass.Reportf(stmt.Pos(), "call to httputil.%s must be immediately followed by a return statement", name) continue } @@ -133,31 +133,33 @@ func checkBlock(pass *analysis.Pass, fn *ast.FuncDecl, block *ast.BlockStmt, nex } // Non-void function and it's the last statement → violation - pass.Reportf(stmt.Pos(), "call to httputil.HandleError must be followed by return because function has return values") + pass.Reportf(stmt.Pos(), "call to httputil.%s must be immediately followed by a return statement", name) } } // findHandleErrorCall returns the call expression if stmt is a direct call to httputil.HandleError(...), // otherwise nil. We only match direct ExprStmt -> CallExpr -> SelectorExpr where selector is httputil.HandleError. -func findHandleErrorCall(stmt ast.Stmt) *ast.CallExpr { +func findHandleErrorCall(stmt ast.Stmt) (*ast.CallExpr, string) { es, ok := stmt.(*ast.ExprStmt) if !ok { - return nil + return nil, "" } call, ok := es.X.(*ast.CallExpr) if !ok { - return nil + return nil, "" } sel, ok := call.Fun.(*ast.SelectorExpr) if !ok { - return nil + return nil, "" } pkgIdent, ok := sel.X.(*ast.Ident) if !ok { - return nil + return nil, "" } - if pkgIdent.Name == "httputil" && sel.Sel.Name == "HandleError" { - return call + selectorName := sel.Sel.Name + if pkgIdent.Name == "httputil" && + (selectorName == "HandleError" || selectorName == "WriteError" || selectorName == "WriteJson" || selectorName == "WriteSSZ") { + return call, selectorName } - return nil + return nil, "" }