From 712ad2d517f277218f96e1f2131720b45c7e26bc Mon Sep 17 00:00:00 2001 From: Rohan Verma Date: Fri, 25 Oct 2019 11:11:47 +0530 Subject: [PATCH] chore: minor refactors based on static checks - unchecked returns fixed (most) - remove unused constants - remove unsed structs - function parameters unused or incorrectly used - removed if else chains for error checks - use regex MustCompile instead of compile - spell checks - preallocate slice cap when size known - scope issues inside range --- admin.go | 2 +- campaigns.go | 11 +++++++---- handlers.go | 3 --- install.go | 3 +-- lists.go | 10 +++++++--- main.go | 34 ++++++++++++++++++++++++---------- manager/manager.go | 6 +++--- media.go | 7 +++---- messenger/emailer.go | 3 ++- messenger/messenger.go | 2 +- public.go | 2 +- subimporter/importer.go | 4 ++-- subscribers.go | 7 +++++-- templates.go | 12 +++++------- utils.go | 10 +++++----- 15 files changed, 67 insertions(+), 49 deletions(-) diff --git a/admin.go b/admin.go index 5d19290..5200fea 100644 --- a/admin.go +++ b/admin.go @@ -38,7 +38,7 @@ func handleGetConfigScript(c echo.Context) error { ) b.Write([]byte(`var CONFIG = `)) - j.Encode(out) + _ = j.Encode(out) return c.Blob(http.StatusOK, "application/javascript", b.Bytes()) } diff --git a/campaigns.go b/campaigns.go index 5746a52..e28760b 100644 --- a/campaigns.go +++ b/campaigns.go @@ -85,9 +85,11 @@ func handleGetCampaigns(c echo.Context) error { if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, fmt.Sprintf("Error fetching campaigns: %s", pqErrMsg(err))) - } else if single && len(out.Results) == 0 { + } + if single && len(out.Results) == 0 { return echo.NewHTTPError(http.StatusBadRequest, "Campaign not found.") - } else if len(out.Results) == 0 { + } + if len(out.Results) == 0 { out.Results = make([]models.Campaign, 0) return c.JSON(http.StatusOK, out) } @@ -492,7 +494,8 @@ func handleTestCampaign(c echo.Context) error { // Send the test messages. for _, s := range subs { - if err := sendTestMessage(&s, &camp, app); err != nil { + sub := s + if err := sendTestMessage(&sub, &camp, app); err != nil { return echo.NewHTTPError(http.StatusBadRequest, fmt.Sprintf("Error sending test: %v", err)) } } @@ -500,7 +503,7 @@ func handleTestCampaign(c echo.Context) error { return c.JSON(http.StatusOK, okResp{true}) } -// sendTestMessage takes a campaign and a subsriber and sends out a sample campain message. +// sendTestMessage takes a campaign and a subsriber and sends out a sample campaign message. func sendTestMessage(sub *models.Subscriber, camp *models.Campaign, app *App) error { if err := camp.CompileTemplate(app.Manager.TemplateFuncs(camp)); err != nil { return fmt.Errorf("Error compiling template: %v", err) diff --git a/handlers.go b/handlers.go index 2d91a59..c9ec6d2 100644 --- a/handlers.go +++ b/handlers.go @@ -13,9 +13,6 @@ const ( // stdInputMaxLen is the maximum allowed length for a standard input field. stdInputMaxLen = 200 - // bodyMaxLen is the maximum allowed length for e-mail bodies. - bodyMaxLen = 1000000 - // defaultPerPage is the default number of results returned in an GET call. defaultPerPage = 20 diff --git a/install.go b/install.go index b70ed58..13e4726 100644 --- a/install.go +++ b/install.go @@ -141,6 +141,5 @@ func newConfigFile() error { return fmt.Errorf("error reading sample config (is binary stuffed?): %v", err) } - ioutil.WriteFile("config.toml", b, 0644) - return nil + return ioutil.WriteFile("config.toml", b, 0644) } diff --git a/lists.go b/lists.go index 8e877af..9a0e9fe 100644 --- a/lists.go +++ b/lists.go @@ -41,9 +41,11 @@ func handleGetLists(c echo.Context) error { if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, fmt.Sprintf("Error fetching lists: %s", pqErrMsg(err))) - } else if single && len(out.Results) == 0 { + } + if single && len(out.Results) == 0 { return echo.NewHTTPError(http.StatusBadRequest, "List not found.") - } else if len(out.Results) == 0 { + } + if len(out.Results) == 0 { return c.JSON(http.StatusOK, okResp{[]struct{}{}}) } @@ -141,7 +143,9 @@ func handleDeleteLists(c echo.Context) error { ) // Read the list IDs if they were sent in the body. - c.Bind(&ids) + if err := c.Bind(&ids); err != nil { + return err + } if id < 1 && len(ids) == 0 { return echo.NewHTTPError(http.StatusBadRequest, "Invalid ID.") diff --git a/main.go b/main.go index 4f31570..db507ec 100644 --- a/main.go +++ b/main.go @@ -84,7 +84,9 @@ func init() { f.Bool("new-config", false, "Generate sample config file") // Process flags. - f.Parse(os.Args[1:]) + if err := f.Parse(os.Args[1:]); err != nil { + logger.Fatalf("error loading flags: %v", err) + } // Display version. if v, _ := f.GetBool("version"); v { @@ -120,13 +122,15 @@ func init() { }), nil); err != nil { logger.Fatalf("error loading config from env: %v", err) } - ko.Load(posflag.Provider(f, ".", ko), nil) + if err := ko.Load(posflag.Provider(f, ".", ko), nil); err != nil { + logger.Fatalf("error loading config: %v", err) + } } // initFileSystem initializes the stuffbin FileSystem to provide // access to bunded static assets to the app. func initFileSystem(binPath string) (stuffbin.FileSystem, error) { - fs, err := stuffbin.UnStuff(os.Args[0]) + fs, err := stuffbin.UnStuff(binPath) if err == nil { return fs, nil } @@ -158,17 +162,23 @@ func initFileSystem(binPath string) (stuffbin.FileSystem, error) { // initMessengers initializes various messaging backends. func initMessengers(r *manager.Manager) messenger.Messenger { // Load SMTP configurations for the default e-mail Messenger. - var srv []messenger.Server - for _, name := range ko.MapKeys("smtp") { + var ( + mapKeys = ko.MapKeys("smtp") + srv = make([]messenger.Server, 0, len(mapKeys)) + ) + + for _, name := range mapKeys { if !ko.Bool(fmt.Sprintf("smtp.%s.enabled", name)) { logger.Printf("skipped SMTP: %s", name) continue } var s messenger.Server - ko.Unmarshal("smtp."+name, &s) + if err := ko.Unmarshal("smtp."+name, &s); err != nil { + logger.Fatalf("error loading SMTP: %v", err) + } s.Name = name - s.SendTimeout = s.SendTimeout * time.Millisecond + s.SendTimeout *= time.Millisecond srv = append(srv, s) logger.Printf("loaded SMTP: %s (%s@%s)", s.Name, s.Username, s.Host) @@ -199,8 +209,12 @@ func main() { defer db.Close() var c constants - ko.Unmarshal("app", &c) - ko.Unmarshal("privacy", &c.Privacy) + if err := ko.Unmarshal("app", &c); err != nil { + log.Fatalf("error loading app config: %v", err) + } + if err := ko.Unmarshal("privacy", &c.Privacy); err != nil { + log.Fatalf("error loading app config: %v", err) + } c.RootURL = strings.TrimRight(c.RootURL, "/") c.UploadURI = filepath.Clean(c.UploadURI) c.UploadPath = filepath.Clean(c.UploadPath) @@ -286,7 +300,7 @@ func main() { app.Messenger = initMessengers(app.Manager) // Initialize the workers that push out messages. - go m.Run(time.Duration(time.Second * 5)) + go m.Run(time.Second * 5) m.SpawnWorkers() // Initialize the HTTP server. diff --git a/manager/manager.go b/manager/manager.go index 215dda3..4e21ca7 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -91,8 +91,8 @@ func New(cfg Config, src DataSource, notifCB models.AdminNotifCallback, l *log.L notifCB: notifCB, logger: l, messengers: make(map[string]messenger.Messenger), - camps: make(map[int]*models.Campaign, 0), - links: make(map[string]string, 0), + camps: make(map[int]*models.Campaign), + links: make(map[string]string), subFetchQueue: make(chan *models.Campaign, cfg.Concurrency), msgQueue: make(chan *Message, cfg.Concurrency), msgErrorQueue: make(chan msgError, cfg.MaxSendErrors), @@ -125,7 +125,7 @@ func (m *Manager) AddMessenger(msg messenger.Messenger) error { // GetMessengerNames returns the list of registered messengers. func (m *Manager) GetMessengerNames() []string { - var names []string + names := make([]string, 0, len(m.messengers)) for n := range m.messengers { names = append(names, n) } diff --git a/media.go b/media.go index 414d429..2444b93 100644 --- a/media.go +++ b/media.go @@ -16,9 +16,8 @@ import ( var imageMimes = []string{"image/jpg", "image/jpeg", "image/png", "image/svg", "image/gif"} const ( - thumbPrefix = "thumb_" - thumbWidth = 90 - thumbHeight = 90 + thumbPrefix = "thumb_" + thumbnailSize = 90 ) // handleUploadMedia handles media file uploads. @@ -52,7 +51,7 @@ func handleUploadMedia(c echo.Context) error { fmt.Sprintf("Error opening image for resizing: %s", err)) } - t := imaging.Resize(src, thumbWidth, 0, imaging.Lanczos) + t := imaging.Resize(src, thumbnailSize, 0, imaging.Lanczos) if err := imaging.Save(t, fmt.Sprintf("%s/%s%s", app.Constants.UploadPath, thumbPrefix, fName)); err != nil { cleanUp = true return echo.NewHTTPError(http.StatusInternalServerError, diff --git a/messenger/emailer.go b/messenger/emailer.go index 4137d2b..a591c88 100644 --- a/messenger/emailer.go +++ b/messenger/emailer.go @@ -38,7 +38,8 @@ func NewEmailer(srv ...Server) (Messenger, error) { servers: make(map[string]*Server), } - for _, s := range srv { + for _, server := range srv { + s := server var auth smtp.Auth if s.AuthProtocol == "cram" { auth = smtp.CRAMMD5Auth(s.Username, s.Password) diff --git a/messenger/messenger.go b/messenger/messenger.go index 65772fa..4f035e6 100644 --- a/messenger/messenger.go +++ b/messenger/messenger.go @@ -29,6 +29,6 @@ func MakeAttachmentHeader(filename, encoding string) textproto.MIMEHeader { h := textproto.MIMEHeader{} h.Set("Content-Disposition", "attachment; filename="+filename) h.Set("Content-Type", "application/json; name=\""+filename+"\"") - h.Set("Content-Transfer-Encoding", "base64") + h.Set("Content-Transfer-Encoding", encoding) return h } diff --git a/public.go b/public.go index 2db83d7..d210357 100644 --- a/public.go +++ b/public.go @@ -232,6 +232,6 @@ func drawTransparentImage(h, w int) []byte { img = image.NewRGBA(image.Rect(0, 0, w, h)) out = &bytes.Buffer{} ) - png.Encode(out, img) + _ = png.Encode(out, img) return out.Bytes() } diff --git a/subimporter/importer.go b/subimporter/importer.go index a0a5116..4ce96e2 100644 --- a/subimporter/importer.go +++ b/subimporter/importer.go @@ -335,7 +335,7 @@ func (s *Session) ExtractZIP(srcPath string, maxCSVs int) (string, []string, err return "", nil, err } - var files []string + files := make([]string, 0, len(z.File)) for _, f := range z.File { fName := f.FileInfo().Name() @@ -421,7 +421,7 @@ func (s *Session) LoadCSV(srcPath string, delim rune) error { s.im.Unlock() // Rewind, now that we've done a linecount on the same handler. - f.Seek(0, 0) + _, _ = f.Seek(0, 0) rd := csv.NewReader(f) rd.Comma = delim diff --git a/subscribers.go b/subscribers.go index 22b4ed4..033f032 100644 --- a/subscribers.go +++ b/subscribers.go @@ -69,10 +69,13 @@ func handleGetSubscriber(c echo.Context) error { if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, fmt.Sprintf("Error fetching subscriber: %s", pqErrMsg(err))) - } else if len(out) == 0 { + } + if len(out) == 0 { return echo.NewHTTPError(http.StatusBadRequest, "Subscriber not found.") } - out.LoadLists(app.Queries.GetSubscriberLists) + if err := out.LoadLists(app.Queries.GetSubscriberLists); err != nil { + return echo.NewHTTPError(http.StatusInternalServerError, "Error loading lists for subscriber.") + } return c.JSON(http.StatusOK, okResp{out[0]}) } diff --git a/templates.go b/templates.go index 48de924..79eda7f 100644 --- a/templates.go +++ b/templates.go @@ -28,10 +28,6 @@ const (

Here is a link to listmonk.

` ) -type dummyMessage struct { - UnsubscribeURL string -} - var ( regexpTplTag = regexp.MustCompile(`{{(\s+)?template\s+?"content"(\s+)?\.(\s+)?}}`) ) @@ -56,12 +52,14 @@ func handleGetTemplates(c echo.Context) error { if err != nil { return echo.NewHTTPError(http.StatusInternalServerError, fmt.Sprintf("Error fetching templates: %s", pqErrMsg(err))) - } else if single && len(out) == 0 { + } + if single && len(out) == 0 { return echo.NewHTTPError(http.StatusBadRequest, "Template not found.") - } else if len(out) == 0 { - return c.JSON(http.StatusOK, okResp{[]struct{}{}}) } + if len(out) == 0 { + return c.JSON(http.StatusOK, okResp{[]struct{}{}}) + } if single { return c.JSON(http.StatusOK, okResp{out[0]}) } diff --git a/utils.go b/utils.go index 4be23fd..bb1cc52 100644 --- a/utils.go +++ b/utils.go @@ -27,11 +27,11 @@ var ( // filename_(number). The number is incremented in case // new file uploads conflict with existing filenames // on the filesystem. - fnameRegexp, _ = regexp.Compile(`(.+?)_([0-9]+)$`) + fnameRegexp = regexp.MustCompile(`(.+?)_([0-9]+)$`) // This replaces all special characters - tagRegexp, _ = regexp.Compile(`[^a-z0-9\-\s]`) - tagRegexpSpaces, _ = regexp.Compile(`[\s]+`) + tagRegexp = regexp.MustCompile(`[^a-z0-9\-\s]`) + tagRegexpSpaces = regexp.MustCompile(`[\s]+`) ) // ScanToStruct prepares a given set of Queries and assigns the resulting @@ -248,7 +248,7 @@ func normalizeTags(tags []string) []string { // makeMsgTpl takes a page title, heading, and message and returns // a msgTpl that can be rendered as a HTML view. This is used for -// rendering aribtrary HTML views with error and success messages. +// rendering arbitrary HTML views with error and success messages. func makeMsgTpl(pageTitle, heading, msg string) msgTpl { if heading == "" { heading = pageTitle @@ -265,7 +265,7 @@ func makeMsgTpl(pageTitle, heading, msg string) msgTpl { // parses each number into an int64 and returns a slice of the // resultant values. func parseStringIDs(s []string) ([]int64, error) { - var vals []int64 + vals := make([]int64, 0, len(s)) for _, v := range s { i, err := strconv.ParseInt(v, 10, 64)