From 82a8f12aee1db15c36972233fc6f2de366c09353 Mon Sep 17 00:00:00 2001 From: Knut Ahlers Date: Sun, 15 Oct 2023 13:19:01 +0200 Subject: [PATCH] Fix linter errors, modernize code Signed-off-by: Knut Ahlers --- cache_filesystem.go | 23 +++++--- go.mod | 2 +- main.go | 126 +++++++++++++++++++++++++++----------------- map.go | 7 ++- postmap.go | 8 +-- 5 files changed, 104 insertions(+), 62 deletions(-) diff --git a/cache_filesystem.go b/cache_filesystem.go index e6e3d4d..c691b45 100644 --- a/cache_filesystem.go +++ b/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 { - return nil, err + if err = os.MkdirAll(path.Dir(cacheFileName), cacheDirMode); err != nil { + return nil, errors.Wrap(err, "creating cache dir") } - if err = ioutil.WriteFile(cacheFileName, buf.Bytes(), 0644); err != nil { - return nil, err + if err = os.WriteFile(cacheFileName, buf.Bytes(), cacheFileMode); err != nil { + return nil, errors.Wrap(err, "writing cache file") } - return ioutil.NopCloser(buf), err + return io.NopCloser(buf), err } diff --git a/go.mod b/go.mod index 0535187..3c7f830 100644 --- a/go.mod +++ b/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 diff --git a/main.go b/main.go index c2bc300..e3ce655 100644 --- a/main.go +++ b/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"` - ForceCache time.Duration `flag:"force-cache" default:"24h" env:"FORCE_CACHE" description:"Force map to be cached for this duration"` + CacheDir string `flag:"cache-dir" default:"cache" description:"Directory to save the cached images to"` + 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"` - RateLimit float64 `flag:"rate-limit" default:"1" env:"RATE_LIMIT" 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"` + MaxSize string `flag:"max-size" default:"1024x1024" description:"Maximum map size requestable"` + RateLimit float64 `flag:"rate-limit" default:"1" description:"How many requests to allow per time"` + 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() { - var err error +func initApp() (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 { - log.Fatalf("Unable to parse max-size: %s", err) + if mapMaxX, mapMaxY, err = parseSize(cfg.MaxSize); err != nil { + return errors.Wrap(err, "parsing max-size") } - if cfg.VersionAndExit { - fmt.Printf("staticmap %s\n", version) - } + return nil } 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 { - err = fmt.Errorf("Map size exceeds allowed bounds of %dx%d", mapMaxX, mapMaxY) - return - } + if (x > mapMaxX || y > mapMaxY) && mapMaxX > 0 && mapMaxY > 0 { + return 0, 0, errors.Errorf("map size exceeds allowed bounds of %dx%d", mapMaxX, mapMaxY) } - 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 { - size = s - } else { - return nil, fmt.Errorf("Bad marker size %q", strings.TrimPrefix(p, "size:")) + s, ok := markerSizes[strings.TrimPrefix(p, "size:")] + if !ok { + return nil, errors.Errorf("bad marker size %q", strings.TrimPrefix(p, "size:")) } + size = s + case strings.HasPrefix(p, "color:0x"): - if c, err := colorful.Hex("#" + strings.TrimPrefix(p, "color:0x")); err == nil { - col = c - } else { - return nil, fmt.Errorf("Unable to parse color %q: %s", strings.TrimPrefix(p, "color:"), err) + c, err := colorful.Hex("#" + strings.TrimPrefix(p, "color:0x")) + if err != nil { + return nil, errors.Wrapf(err, "parsing color %q", strings.TrimPrefix(p, "color:")) } + col = c + case strings.HasPrefix(p, "color:"): - if c, ok := markerColors[strings.TrimPrefix(p, "color:")]; ok { - col = c - } else { - return nil, fmt.Errorf("Bad color name %q", strings.TrimPrefix(p, "color:")) + c, ok := markerColors[strings.TrimPrefix(p, "color:")] + if !ok { + return nil, errors.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, diff --git a/map.go b/map.go index f41d8a1..9c44bc5 100644 --- a/map.go +++ b/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") } diff --git a/postmap.go b/postmap.go index cee9a81..e13249d 100644 --- a/postmap.go +++ b/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, }) }