Bug 1934515 - Add lint checks to the CI (and fix errors) (#406)

.circleci/config.yml

@@ -73,6 +73,13 @@ jobs:
           name: Go test
           command: |
             go test -v -mod vendor -covermode=atomic -coverprofile=coverage.txt ./...
+      - run:
+          name: Go lint
+          command: |
+            go vet ./...
+
+            go install honnef.co/go/tools/cmd/staticcheck@latest
+            $(go env GOPATH)/bin/staticcheck ./...
       - codecov/upload
 
   deploy:

bouncer/db.go → db.go

@@ -1,4 +1,4 @@
-package bouncer
+package main
 
 import (
 	"database/sql"
@@ -11,6 +11,7 @@ type DB struct {
 	*sql.DB
 }
 
+// NewDB returns a new database instance.
 func NewDB(dsn string) (*DB, error) {
 	db, err := sql.Open("mysql", dsn)
 
@@ -30,8 +31,7 @@ func NewDB(dsn string) (*DB, error) {
 
 // AliasFor returns the alias for a product
 //
-// For example firefox-latest will resolve to the latest
-// version of firefox.
+// For example firefox-latest will resolve to the latest version of firefox.
 func (d *DB) AliasFor(product string) (related string, err error) {
 	err = d.QueryRow(
 		"SELECT related_product FROM mirror_aliases WHERE alias = ?",
@@ -55,6 +55,7 @@ func (d *DB) OSID(name string) (id string, err error) {
 	return
 }
 
+// ProductForLanguage returns the product ID given a product name and language.
 func (d *DB) ProductForLanguage(product, lang string) (productID string, sslOnly bool, err error) {
 	sslInt := 0
 	err = d.QueryRow(

bouncer/db_test.go → db_test.go

@@ -1,4 +1,4 @@
-package bouncer
+package main
 
 import (
 	"database/sql"

handlers.go

@@ -11,13 +11,11 @@ import (
 	"regexp"
 	"strings"
 	"time"
-
-	"github.com/mozilla-services/go-bouncer/bouncer"
 )
 
 const (
-	DefaultLang          = "en-US"
-	DefaultOS            = "win"
+	defaultLang          = "en-US"
+	defaultOS            = "win"
 	fxPre2024LastNightly = "firefox-nightly-pre2024"
 	fxPre2024LastBeta    = "127.0b9"
 	fxPre2024LastRelease = "127.0"
@@ -50,7 +48,7 @@ func isWin64UserAgent(userAgent string) bool {
 // isPre2024StubUserAgent is used to detect stub installers that pin the
 // "DigiCert SHA2 Assured ID Code Signing CA" intermediate.
 func isPre2024StubUserAgent(userAgent string) bool {
-	return "NSIS InetBgDL (Mozilla)" == userAgent
+	return userAgent == "NSIS InetBgDL (Mozilla)"
 }
 
 func pre2024Product(product string) string {
@@ -93,9 +91,8 @@ func pre2024Product(product string) string {
 
 // HealthResult represents service health
 type HealthResult struct {
-	DB      bool   `json:"db"`
-	Healthy bool   `json:"healthy"`
-	Version string `json:"version"`
+	DB      bool `json:"db"`
+	Healthy bool `json:"healthy"`
 }
 
 // JSON returns json string
@@ -110,7 +107,7 @@ func (h *HealthResult) JSON() []byte {
 
 // HealthHandler returns 200 if the app looks okay
 type HealthHandler struct {
-	db *bouncer.DB
+	db *DB
 
 	CacheTime time.Duration
 }
@@ -119,7 +116,6 @@ func (h *HealthHandler) check() *HealthResult {
 	result := &HealthResult{
 		DB:      true,
 		Healthy: true,
-		Version: bouncer.Version,
 	}
 
 	err := h.db.Ping()
@@ -131,7 +127,7 @@ func (h *HealthHandler) check() *HealthResult {
 	return result
 }
 
-func (h *HealthHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
+func (h *HealthHandler) ServeHTTP(w http.ResponseWriter, _ *http.Request) {
 	if h.CacheTime > 0 {
 		w.Header().Set("Cache-Control", fmt.Sprintf("max-age=%d", h.CacheTime/time.Second))
 	}
@@ -147,18 +143,18 @@ func (h *HealthHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 
 // BouncerHandler is the primary handler for this application
 type BouncerHandler struct {
-	db *bouncer.DB
+	db *DB
 
 	CacheTime          time.Duration
-	PinHttpsHeaderName string
+	PinHTTPSHeaderName string
 	PinnedBaseURLHttp  string
 	PinnedBaseURLHttps string
 	StubRootURL        string
 }
 
 // URL returns the final redirect URL given a lang, os and product
 // if the string is == "", no mirror or location was found
-func (b *BouncerHandler) URL(pinHttps bool, lang, os, product string) (string, error) {
+func (b *BouncerHandler) URL(pinHTTPS bool, lang, os, product string) (string, error) {
 	product, err := b.db.AliasFor(product)
 	if err != nil {
 		return "", err
@@ -190,7 +186,7 @@ func (b *BouncerHandler) URL(pinHttps bool, lang, os, product string) (string, e
 	locationPath = strings.Replace(locationPath, ":lang", lang, -1)
 
 	mirrorBaseURL := "http://" + b.PinnedBaseURLHttp
-	if pinHttps || sslOnly {
+	if pinHTTPS || sslOnly {
 		mirrorBaseURL = "https://" + b.PinnedBaseURLHttps
 	}
 
@@ -208,27 +204,27 @@ func (b *BouncerHandler) stubAttributionURL(reqParams *BouncerParams) string {
 	return b.StubRootURL + "?" + query.Encode()
 }
 
-func (b *BouncerHandler) shouldPinHttps(req *http.Request) bool {
-	if b.PinHttpsHeaderName == "" {
+func (b *BouncerHandler) shouldPinHTTPS(req *http.Request) bool {
+	if b.PinHTTPSHeaderName == "" {
 		return false
 	}
 
-	return req.Header.Get(b.PinHttpsHeaderName) == "https"
+	return req.Header.Get(b.PinHTTPSHeaderName) == "https"
 }
 
-func fromRTAMO(attribution_code string) bool {
+func fromRTAMO(attributionCode string) bool {
 	// base64 decode the attribution_code value to see if it matches the RTAMO regex
 	// This uses '.' as padding because Bedrock is using this library to encode the values:
 	// https://pypi.org/project/querystringsafe-base64/
 	var base64Decoder = base64.URLEncoding.WithPadding('.')
-	sDec, err := base64Decoder.DecodeString(attribution_code)
+	sDec, err := base64Decoder.DecodeString(attributionCode)
 	if err != nil {
-		log.Printf("Error decoding %s: %s ", attribution_code, err.Error())
+		log.Printf("Error decoding %s: %s ", attributionCode, err.Error())
 		return false
 	}
 	q, err := url.ParseQuery(string(sDec))
 	if err != nil {
-		log.Printf("Error parsing the attribution_code query parameters: %s", err.Error())
+		log.Printf("Error parsing the attribution_code query parameter: %s", err.Error())
 		return false
 	}
 
@@ -293,21 +289,22 @@ func (b *BouncerHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 	reqParams := BouncerParamsFromValues(req.URL.Query(), req.Header)
 
 	if reqParams.Product == "" {
-		http.Redirect(w, req, "https://www.mozilla.org/", 302)
+		http.Redirect(w, req, "https://www.mozilla.org/", http.StatusFound)
 		return
 	}
 
 	if reqParams.OS == "" {
-		reqParams.OS = DefaultOS
+		reqParams.OS = defaultOS
 	}
+
 	if reqParams.Lang == "" {
-		reqParams.Lang = DefaultLang
+		reqParams.Lang = defaultLang
 	}
 
 	// If attribution_code is set, redirect to the stub service.
 	if b.shouldAttribute(reqParams) {
 		stubURL := b.stubAttributionURL(reqParams)
-		http.Redirect(w, req, stubURL, 302)
+		http.Redirect(w, req, stubURL, http.StatusFound)
 		return
 	}
 
@@ -339,7 +336,7 @@ func (b *BouncerHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		reqParams.Product = pre2024Product(reqParams.Product)
 	}
 
-	url, err := b.URL(b.shouldPinHttps(req), reqParams.Lang, reqParams.OS, reqParams.Product)
+	url, err := b.URL(b.shouldPinHTTPS(req), reqParams.Lang, reqParams.OS, reqParams.Product)
 	if err != nil {
 		http.Error(w, "Internal Server Error.", http.StatusInternalServerError)
 		log.Println(err)
@@ -361,5 +358,5 @@ func (b *BouncerHandler) ServeHTTP(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 
-	http.Redirect(w, req, url, 302)
+	http.Redirect(w, req, url, http.StatusFound)
 }

handlers_test.go

@@ -8,34 +8,26 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/mozilla-services/go-bouncer/bouncer"
 	"github.com/stretchr/testify/assert"
 )
 
 const testDSN = "root@tcp(127.0.0.1:3306)/bouncer_test"
 
 var bouncerHandler *BouncerHandler
-var bouncerHandlerPinned *BouncerHandler
 
 func init() {
-	testDB, err := bouncer.NewDB(testDSN)
+	testDB, err := NewDB(testDSN)
 	if err != nil {
 		log.Fatal(err)
 	}
 
 	bouncerHandler = &BouncerHandler{
 		db:                 testDB,
 		StubRootURL:        "https://stub/",
-		PinHttpsHeaderName: "X-Forwarded-Proto",
+		PinHTTPSHeaderName: "X-Forwarded-Proto",
 		PinnedBaseURLHttp:  "download.cdn.mozilla.net/pub",
 		PinnedBaseURLHttps: "download-installer.cdn.mozilla.net/pub",
 	}
-	bouncerHandlerPinned = &BouncerHandler{
-		db:                 testDB,
-		PinnedBaseURLHttp:  "download-sha1.cdn.mozilla.net/pub",
-		PinnedBaseURLHttps: "download-sha1.cdn.mozilla.net/pub",
-		PinHttpsHeaderName: "X-Forwarded-Proto",
-	}
 }
 
 func TestShouldAttribute(t *testing.T) {
@@ -201,7 +193,7 @@ func TestBouncerHandlerAttributionCode(t *testing.T) {
 
 		bouncerHandler.ServeHTTP(w, req)
 		assert.Equal(t, 302, w.Code)
-		assert.Equal(t, test.Out, w.HeaderMap.Get("Location"))
+		assert.Equal(t, test.Out, w.Result().Header.Get("Location"))
 	}
 }
 
@@ -213,27 +205,27 @@ func TestBouncerHandlerParams(t *testing.T) {
 
 	bouncerHandler.ServeHTTP(w, req)
 	assert.Equal(t, 302, w.Code)
-	assert.Equal(t, "https://www.mozilla.org/", w.HeaderMap.Get("Location"))
+	assert.Equal(t, "https://www.mozilla.org/", w.Result().Header.Get("Location"))
 }
 
 func TestBouncerShouldPinHttps(t *testing.T) {
-	bouncerHandler.PinHttpsHeaderName = ""
+	bouncerHandler.PinHTTPSHeaderName = ""
 	req, err := http.NewRequest("GET", "http://test/?product=firefox-latest&os=osx&lang=en-US", nil)
 	assert.NoError(t, err)
-	assert.Equal(t, false, bouncerHandler.shouldPinHttps(req))
+	assert.Equal(t, false, bouncerHandler.shouldPinHTTPS(req))
 
 	req.Header.Set("X-Forwarded-Proto", "https")
-	assert.Equal(t, false, bouncerHandler.shouldPinHttps(req))
+	assert.Equal(t, false, bouncerHandler.shouldPinHTTPS(req))
 
-	bouncerHandler.PinHttpsHeaderName = "X-Forwarded-Proto"
+	bouncerHandler.PinHTTPSHeaderName = "X-Forwarded-Proto"
 
-	assert.Equal(t, true, bouncerHandler.shouldPinHttps(req))
+	assert.Equal(t, true, bouncerHandler.shouldPinHTTPS(req))
 
 	req.Header.Set("X-Forwarded-Proto", "http")
-	assert.Equal(t, false, bouncerHandler.shouldPinHttps(req))
+	assert.Equal(t, false, bouncerHandler.shouldPinHTTPS(req))
 
 	req.Header.Del("X-Forwarded-Proto")
-	assert.Equal(t, false, bouncerHandler.shouldPinHttps(req))
+	assert.Equal(t, false, bouncerHandler.shouldPinHTTPS(req))
 }
 
 func TestBouncerHandlerPrintQuery(t *testing.T) {
@@ -297,7 +289,7 @@ func TestBouncerHandlerValid(t *testing.T) {
 
 		bouncerHandler.ServeHTTP(w, req)
 		assert.Equal(t, 302, w.Code, "url: %v ua: %v", testRequest.URL, testRequest.UserAgent)
-		assert.Equal(t, testRequest.ExpectedLocation, w.HeaderMap.Get("Location"), "url: %v ua: %v", testRequest.URL, testRequest.UserAgent)
+		assert.Equal(t, testRequest.ExpectedLocation, w.Result().Header.Get("Location"), "url: %v ua: %v", testRequest.URL, testRequest.UserAgent)
 	}
 }
 
@@ -396,7 +388,7 @@ func TestBouncerHandlerForWindowsOnlyCompatibleWithESR115(t *testing.T) {
 			bouncerHandler.ServeHTTP(w, req)
 
 			assert.Equal(t, 302, w.Code, "userAgent: %v, url: %v", tc.userAgent, url)
-			assert.Equal(t, expectedLocation, w.HeaderMap.Get("Location"), "userAgent: %v, url: %v", tc.userAgent, url)
+			assert.Equal(t, expectedLocation, w.Result().Header.Get("Location"), "userAgent: %v, url: %v", tc.userAgent, url)
 		}
 
 		// This is for other firefox 32-bit products.
@@ -417,7 +409,7 @@ func TestBouncerHandlerForWindowsOnlyCompatibleWithESR115(t *testing.T) {
 
 			assert.Equal(t, 302, w.Code, "userAgent: %v, url: %v", tc.userAgent, url)
 			// We don't need to assert the scheme.
-			assert.True(t, strings.HasSuffix(w.HeaderMap.Get("Location"), expectedLocation), "userAgent: %v, url: %v", tc.userAgent, url)
+			assert.True(t, strings.HasSuffix(w.Result().Header.Get("Location"), expectedLocation), "userAgent: %v, url: %v", tc.userAgent, url)
 		}
 
 		// This is for other firefox 64-bit products.
@@ -438,7 +430,7 @@ func TestBouncerHandlerForWindowsOnlyCompatibleWithESR115(t *testing.T) {
 
 			assert.Equal(t, 302, w.Code, "userAgent: %v, url: %v", tc.userAgent, url)
 			// We don't need to assert the scheme.
-			assert.True(t, strings.HasSuffix(w.HeaderMap.Get("Location"), expectedLocation), "userAgent: %v, url: %v", tc.userAgent, url)
+			assert.True(t, strings.HasSuffix(w.Result().Header.Get("Location"), expectedLocation), "userAgent: %v, url: %v", tc.userAgent, url)
 		}
 
 		// This is for MSI builds.
@@ -451,7 +443,7 @@ func TestBouncerHandlerForWindowsOnlyCompatibleWithESR115(t *testing.T) {
 		bouncerHandler.ServeHTTP(w, req)
 
 		assert.Equal(t, 302, w.Code)
-		assert.Equal(t, expectedLocation, w.HeaderMap.Get("Location"))
+		assert.Equal(t, expectedLocation, w.Result().Header.Get("Location"))
 
 		// This is for unrelated products.
 		for _, url := range []string{
@@ -483,11 +475,11 @@ func TestBouncerHandlerForWindowsOnlyCompatibleWithESR115WithMozorgReferrer(t *t
 	bouncerHandler.ServeHTTP(w, req)
 
 	assert.Equal(t, 302, w.Code)
-	assert.Equal(t, expectedLocation, w.HeaderMap.Get("Location"))
+	assert.Equal(t, expectedLocation, w.Result().Header.Get("Location"))
 }
 
 func TestHealthHandler(t *testing.T) {
-	testDB, err := bouncer.NewDB(testDSN)
+	testDB, err := NewDB(testDSN)
 	if err != nil {
 		log.Fatal(err)
 	}
@@ -503,5 +495,5 @@ func TestHealthHandler(t *testing.T) {
 	h.ServeHTTP(w, req)
 
 	assert.Equal(t, 200, w.Code)
-	assert.Equal(t, fmt.Sprintf(`{"db":true,"healthy":true,"version":"%s"}`, bouncer.Version), w.Body.String())
+	assert.Equal(t, `{"db":true,"healthy":true}`, w.Body.String())
 }