Fix linter errors, modernize code

Signed-off-by: Knut Ahlers <knut@ahlers.me>

cache_filesystem.go

@@ -3,10 +3,16 @@ package main
 import (
 	"bytes"
 	"io"
-	"io/ioutil"
 	"os"
 	"path"
 	"time"
+
+	"github.com/pkg/errors"
+)
+
+const (
+	cacheDirMode  = 0o700
+	cacheFileMode = 0o600
 )
 
 func filesystemCache(opts generateMapConfig) (io.ReadCloser, error) {
@@ -14,7 +20,8 @@ func filesystemCache(opts generateMapConfig) (io.ReadCloser, error) {
 	cacheFileName := path.Join(cfg.CacheDir, cacheKey[0:2], cacheKey+".png")
 
 	if info, err := os.Stat(cacheFileName); err == nil && info.ModTime().Add(cfg.ForceCache).After(time.Now()) {
-		return os.Open(cacheFileName)
+		f, err := os.Open(cacheFileName) //#nosec:G304 // Intended to open a variable file
+		return f, errors.Wrap(err, "opening file")
 	}
 
 	// No cache hit, generate a new map
@@ -25,16 +32,16 @@ func filesystemCache(opts generateMapConfig) (io.ReadCloser, error) {
 
 	buf := new(bytes.Buffer)
 	if _, err = io.Copy(buf, mapReader); err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "writing file")
 	}
 
-	if err = os.MkdirAll(path.Dir(cacheFileName), 0700); err != nil {
+	if err = os.MkdirAll(path.Dir(cacheFileName), cacheDirMode); err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "creating cache dir")
 	}
 
-	if err = ioutil.WriteFile(cacheFileName, buf.Bytes(), 0644); err != nil {
+	if err = os.WriteFile(cacheFileName, buf.Bytes(), cacheFileMode); err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "writing cache file")
 	}
 
-	return ioutil.NopCloser(buf), err
+	return io.NopCloser(buf), err
 }

go.mod

@@ -11,6 +11,7 @@ require (
 	github.com/golang/geo v0.0.0-20230421003525-6adc56603217
 	github.com/gorilla/mux v1.8.0
 	github.com/lucasb-eyer/go-colorful v1.2.0
+	github.com/pkg/errors v0.9.1
 	github.com/sirupsen/logrus v1.9.3
 )
 
@@ -19,7 +20,6 @@ require (
 	github.com/flopp/go-coordsparser v0.0.0-20201115094714-8baaeb7062d5 // indirect
 	github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0 // indirect
 	github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
-	github.com/pkg/errors v0.9.1 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/tkrajina/gpxgo v1.3.0 // indirect
 	golang.org/x/image v0.13.0 // indirect

main.go

@@ -2,7 +2,6 @@ package main
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -17,17 +16,18 @@ import (
 	"github.com/golang/geo/s2"
 	"github.com/gorilla/mux"
 	colorful "github.com/lucasb-eyer/go-colorful"
-	log "github.com/sirupsen/logrus"
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 )
 
 var (
 	cfg struct {
-		CacheDir       string        `flag:"cache-dir" default:"cache" env:"CACHE_DIR" description:"Directory to save the cached images to"`
+		CacheDir       string        `flag:"cache-dir" default:"cache" description:"Directory to save the cached images to"`
-		ForceCache     time.Duration `flag:"force-cache" default:"24h" env:"FORCE_CACHE" description:"Force map to be cached for this duration"`
+		ForceCache     time.Duration `flag:"force-cache" default:"24h" description:"Force map to be cached for this duration"`
 		Listen         string        `flag:"listen" default:":3000" description:"IP/Port to listen on"`
-		MaxSize        string        `flag:"max-size" default:"1024x1024" env:"MAX_SIZE" description:"Maximum map size requestable"`
+		MaxSize        string        `flag:"max-size" default:"1024x1024" description:"Maximum map size requestable"`
-		RateLimit      float64       `flag:"rate-limit" default:"1" env:"RATE_LIMIT" description:"How many requests to allow per time"`
+		RateLimit      float64       `flag:"rate-limit" default:"1" description:"How many requests to allow per time"`
-		RateLimitTime  time.Duration `flag:"rate-limit-time" default:"1s" env:"RATE_LIMIT_TIME" description:"Time interval to allow N requests in"`
+		RateLimitTime  time.Duration `flag:"rate-limit-time" default:"1s" description:"Time interval to allow N requests in"`
 		VersionAndExit bool          `flag:"version" default:"false" description:"Print version information and exit"`
 	}
 
@@ -37,22 +37,30 @@ var (
 	version = "dev"
 )
 
-func init() {
+func initApp() (err error) {
-	var err error
+	rconfig.AutoEnv(true)
 	if err = rconfig.ParseAndValidate(&cfg); err != nil {
-		log.Fatalf("Unable to parse CLI parameters")
+		return errors.Wrap(err, "parsing CLI parameters")
 	}
 
-	if mapMaxX, mapMaxY, err = parseSize(cfg.MaxSize, false); err != nil {
+	if mapMaxX, mapMaxY, err = parseSize(cfg.MaxSize); err != nil {
-		log.Fatalf("Unable to parse max-size: %s", err)
+		return errors.Wrap(err, "parsing max-size")
 	}
 
-	if cfg.VersionAndExit {
+	return nil
-		fmt.Printf("staticmap %s\n", version)
-	}
 }
 
 func main() {
+	var err error
+	if err = initApp(); err != nil {
+		logrus.WithError(err).Fatal("initializing app")
+	}
+
+	if cfg.VersionAndExit {
+		fmt.Printf("staticmap %s\n", version) //nolint:forbidigo
+		return
+	}
+
 	rateLimit := tollbooth.NewLimiter(cfg.RateLimit, &limiter.ExpirableOptions{
 		DefaultExpirationTTL: cfg.RateLimitTime,
 	})
@@ -62,7 +70,17 @@ func main() {
 	r.HandleFunc("/status", func(res http.ResponseWriter, r *http.Request) { http.Error(res, "I'm fine", http.StatusOK) })
 	r.Handle("/map.png", tollbooth.LimitFuncHandler(rateLimit, handleMapRequest)).Methods("GET")
 	r.Handle("/map.png", tollbooth.LimitFuncHandler(rateLimit, handlePostMapRequest)).Methods("POST")
-	log.Fatalf("HTTP Server exitted: %s", http.ListenAndServe(cfg.Listen, httpHelper.NewHTTPLogHandler(r)))
+
+	server := &http.Server{
+		Addr:              cfg.Listen,
+		Handler:           httpHelper.NewHTTPLogHandler(r),
+		ReadHeaderTimeout: time.Second,
+	}
+
+	logrus.WithField("version", version).Info("staticmap started")
+	if err = server.ListenAndServe(); err != nil {
+		logrus.WithError(err).Fatal("running HTTP server")
+	}
 }
 
 func handleMapRequest(res http.ResponseWriter, r *http.Request) {
@@ -84,7 +102,7 @@ func handleMapRequest(res http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if opts.Width, opts.Height, err = parseSize(r.URL.Query().Get("size"), true); err != nil {
+	if opts.Width, opts.Height, err = parseSize(r.URL.Query().Get("size")); err != nil {
 		http.Error(res, fmt.Sprintf("Unable to parse 'size' parameter: %s", err), http.StatusBadRequest)
 		return
 	}
@@ -95,15 +113,22 @@ func handleMapRequest(res http.ResponseWriter, r *http.Request) {
 	}
 
 	if mapReader, err = cacheFunc(opts); err != nil {
-		log.Errorf("Map render failed: %s (Request: %s)", err, r.URL.String())
+		logrus.Errorf("map render failed: %s (Request: %s)", err, r.URL.String())
 		http.Error(res, fmt.Sprintf("I experienced difficulties rendering your map: %s", err), http.StatusInternalServerError)
 		return
 	}
-	defer mapReader.Close()
+	defer func() {
+		if err := mapReader.Close(); err != nil {
+			logrus.WithError(err).Error("closing map cache reader (leaked fd)")
+		}
+	}()
 
 	res.Header().Set("Content-Type", "image/png")
 	res.Header().Set("Cache-Control", "public")
-	io.Copy(res, mapReader)
+
+	if _, err = io.Copy(res, mapReader); err != nil {
+		logrus.WithError(err).Debug("writing image to HTTP client")
+	}
 }
 
 func handlePostMapRequest(res http.ResponseWriter, r *http.Request) {
@@ -124,15 +149,22 @@ func handlePostMapRequest(res http.ResponseWriter, r *http.Request) {
 	}
 
 	if mapReader, err = cacheFunc(opts); err != nil {
-		log.Errorf("Map render failed: %s (Request: %s)", err, r.URL.String())
+		logrus.Errorf("map render failed: %s (Request: %s)", err, r.URL.String())
 		http.Error(res, fmt.Sprintf("I experienced difficulties rendering your map: %s", err), http.StatusInternalServerError)
 		return
 	}
-	defer mapReader.Close()
+	defer func() {
+		if err := mapReader.Close(); err != nil {
+			logrus.WithError(err).Error("closing map cache reader (leaked fd)")
+		}
+	}()
 
 	res.Header().Set("Content-Type", "image/png")
 	res.Header().Set("Cache-Control", "public")
-	io.Copy(res, mapReader)
+
+	if _, err = io.Copy(res, mapReader); err != nil {
+		logrus.WithError(err).Debug("writing image to HTTP client")
+	}
 }
 
 func parseCoordinate(coord string) (s2.LatLng, error) {
@@ -141,7 +173,7 @@ func parseCoordinate(coord string) (s2.LatLng, error) {
 	}
 
 	parts := strings.Split(coord, ",")
-	if len(parts) != 2 {
+	if len(parts) != 2 { //nolint:gomnd
 		return s2.LatLng{}, errors.New("Coordinate not in format lat,lon")
 	}
 
@@ -162,32 +194,29 @@ func parseCoordinate(coord string) (s2.LatLng, error) {
 	return pt, nil
 }
 
-func parseSize(size string, validate bool) (x, y int, err error) {
+func parseSize(size string) (x, y int, err error) {
 	if size == "" {
 		return 0, 0, errors.New("No size given")
 	}
 
 	parts := strings.Split(size, "x")
-	if len(parts) != 2 {
+	if len(parts) != 2 { //nolint:gomnd
 		return 0, 0, errors.New("Size not in format 600x300")
 	}
 
 	if x, err = strconv.Atoi(parts[0]); err != nil {
-		return
+		return 0, 0, errors.Wrap(err, "parsing width")
 	}
 
 	if y, err = strconv.Atoi(parts[1]); err != nil {
-		return
+		return 0, 0, errors.Wrap(err, "parsing height")
 	}
 
-	if validate {
+	if (x > mapMaxX || y > mapMaxY) && mapMaxX > 0 && mapMaxY > 0 {
-		if x > mapMaxX || y > mapMaxY {
+		return 0, 0, errors.Errorf("map size exceeds allowed bounds of %dx%d", mapMaxX, mapMaxY)
-			err = fmt.Errorf("Map size exceeds allowed bounds of %dx%d", mapMaxX, mapMaxY)
-			return
-		}
 	}
 
-	return
+	return x, y, nil
 }
 
 func parseMarkerLocations(markers []string) ([]marker, error) {
@@ -209,27 +238,30 @@ func parseMarkerLocations(markers []string) ([]marker, error) {
 		for _, p := range parts {
 			switch {
 			case strings.HasPrefix(p, "size:"):
-				if s, ok := markerSizes[strings.TrimPrefix(p, "size:")]; ok {
+				s, ok := markerSizes[strings.TrimPrefix(p, "size:")]
+				if !ok {
+					return nil, errors.Errorf("bad marker size %q", strings.TrimPrefix(p, "size:"))
+				}
 				size = s
-				} else {
+
-					return nil, fmt.Errorf("Bad marker size %q", strings.TrimPrefix(p, "size:"))
-				}
 			case strings.HasPrefix(p, "color:0x"):
-				if c, err := colorful.Hex("#" + strings.TrimPrefix(p, "color:0x")); err == nil {
+				c, err := colorful.Hex("#" + strings.TrimPrefix(p, "color:0x"))
-					col = c
+				if err != nil {
-				} else {
+					return nil, errors.Wrapf(err, "parsing color %q", strings.TrimPrefix(p, "color:"))
-					return nil, fmt.Errorf("Unable to parse color %q: %s", strings.TrimPrefix(p, "color:"), err)
 				}
+				col = c
+
 			case strings.HasPrefix(p, "color:"):
-				if c, ok := markerColors[strings.TrimPrefix(p, "color:")]; ok {
+				c, ok := markerColors[strings.TrimPrefix(p, "color:")]
-					col = c
+				if !ok {
-				} else {
+					return nil, errors.Errorf("bad color name %q", strings.TrimPrefix(p, "color:"))
-					return nil, fmt.Errorf("Bad color name %q", strings.TrimPrefix(p, "color:"))
 				}
+				col = c
+
 			default:
 				pos, err := parseCoordinate(p)
 				if err != nil {
-					return nil, fmt.Errorf("Unparsable chunk found in marker: %q", p)
+					return nil, errors.Errorf("unparsable chunk found in marker: %q", p)
 				}
 				result = append(result, marker{
 					pos:   pos,

map.go

@@ -11,8 +11,10 @@ import (
 	staticMap "github.com/Luzifer/go-staticmaps"
 	"github.com/fogleman/gg"
 	"github.com/golang/geo/s2"
+	"github.com/pkg/errors"
 )
 
+//nolint:gomnd // these are the "constant" definitions
 var markerColors = map[string]color.Color{
 	"black":  color.RGBA{R: 145, G: 145, B: 145, A: 0xff},
 	"brown":  color.RGBA{R: 178, G: 154, B: 123, A: 0xff},
@@ -28,6 +30,7 @@ var markerColors = map[string]color.Color{
 
 type markerSize float64
 
+//nolint:gomnd // these are the "constant" definitions
 var markerSizes = map[string]markerSize{
 	"tiny":  10,
 	"mid":   15,
@@ -106,10 +109,10 @@ func generateMap(opts generateMapConfig) (io.Reader, error) {
 
 	img, err := ctx.Render()
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "rendering context")
 	}
 
 	pngCtx := gg.NewContextForImage(img)
 	pngBuf := new(bytes.Buffer)
-	return pngBuf, pngCtx.EncodePNG(pngBuf)
+	return pngBuf, errors.Wrap(pngCtx.EncodePNG(pngBuf), "encoding to PNG")
 }

postmap.go

@@ -7,6 +7,7 @@ import (
 
 	staticMap "github.com/Luzifer/go-staticmaps"
 	"github.com/golang/geo/s2"
+	"github.com/pkg/errors"
 )
 
 type postMapEnvelope struct {
@@ -29,7 +30,7 @@ func (p postMapEnvelope) toGenerateMapConfig() (generateMapConfig, error) {
 	}
 
 	if p.Width > mapMaxX || p.Height > mapMaxY {
-		return generateMapConfig{}, fmt.Errorf("Map size exceeds allowed bounds of %dx%d", mapMaxX, mapMaxY)
+		return generateMapConfig{}, errors.Errorf("map size exceeds allowed bounds of %dx%d", mapMaxX, mapMaxY)
 	}
 
 	var err error
@@ -94,10 +95,9 @@ type postMapOverlay []string
 func (p postMapOverlay) toOverlays() ([]*staticMap.TileProvider, error) {
 	result := []*staticMap.TileProvider{}
 	for _, pat := range p {
-
 		for _, v := range []string{`{0}`, `{1}`, `{2}`} {
 			if !strings.Contains(pat, v) {
-				return nil, fmt.Errorf("Placeholder %q not found in pattern %q", v, pat)
+				return nil, errors.Errorf("placeholder %q not found in pattern %q", v, pat)
 			}
 		}
 
@@ -105,7 +105,7 @@ func (p postMapOverlay) toOverlays() ([]*staticMap.TileProvider, error) {
 
 		result = append(result, &staticMap.TileProvider{
 			Name:       fmt.Sprintf("%x", sha256.Sum256([]byte(pat))),
-			TileSize:   256,
+			TileSize:   256, //nolint:gomnd
 			URLPattern: pat,
 		})
 	}