From f2f943c0d800c513025a2fecfb1a48729052fdc1 Mon Sep 17 00:00:00 2001 From: Gusted Date: Tue, 15 Nov 2022 16:15:11 +0100 Subject: [PATCH] Remove unnecessary conversion (#139) - Remove unnecessary type conversion. - Enforce via CI Co-authored-by: Gusted Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/139 Reviewed-by: 6543 <6543@obermui.de> Co-authored-by: Gusted Co-committed-by: Gusted --- .golangci.yml | 20 ++++++++++ cmd/main.go | 2 +- integration/get_test.go | 2 +- integration/main_test.go | 2 +- server/certificates/certificates.go | 50 +++++++++++++------------ server/database/setup.go | 2 +- server/gitea/cache.go | 5 +-- server/handler/handler.go | 2 +- server/handler/handler_custom_domain.go | 2 +- server/handler/handler_sub_domain.go | 2 +- server/handler/try.go | 8 ++-- server/setup.go | 4 +- server/upstream/helper.go | 2 +- server/upstream/upstream.go | 4 +- 14 files changed, 64 insertions(+), 43 deletions(-) create mode 100644 .golangci.yml diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..6d9b95a --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,20 @@ +linters-settings: + gocritic: + enabled-tags: + - diagnostic + - experimental + - opinionated + - performance + - style + disabled-checks: + - importShadow + - ifElseChain + - hugeParam + +linters: + enable: + - unconvert + - gocritic + +run: + timeout: 5m diff --git a/cmd/main.go b/cmd/main.go index 6ad1aa8..b72013a 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -65,7 +65,7 @@ func Serve(ctx *cli.Context) error { } allowedCorsDomains := AllowedCorsDomains - if len(rawDomain) != 0 { + if rawDomain != "" { allowedCorsDomains = append(allowedCorsDomains, rawDomain) } diff --git a/integration/get_test.go b/integration/get_test.go index 8794651..81d8488 100644 --- a/integration/get_test.go +++ b/integration/get_test.go @@ -124,7 +124,7 @@ func TestLFSSupport(t *testing.T) { func TestGetOptions(t *testing.T) { log.Println("=== TestGetOptions ===") - req, _ := http.NewRequest(http.MethodOptions, "https://mock-pages.codeberg-test.org:4430/README.md", nil) + req, _ := http.NewRequest(http.MethodOptions, "https://mock-pages.codeberg-test.org:4430/README.md", http.NoBody) resp, err := getTestHTTPSClient().Do(req) assert.NoError(t, err) if !assert.NotNil(t, resp) { diff --git a/integration/main_test.go b/integration/main_test.go index 06d553f..406b33a 100644 --- a/integration/main_test.go +++ b/integration/main_test.go @@ -28,7 +28,7 @@ func TestMain(m *testing.M) { time.Sleep(10 * time.Second) - os.Exit(m.Run()) + m.Run() } func startServer(ctx context.Context) error { diff --git a/server/certificates/certificates.go b/server/certificates/certificates.go index 11cf0df..8af4be5 100644 --- a/server/certificates/certificates.go +++ b/server/certificates/certificates.go @@ -53,17 +53,19 @@ func TLSConfig(mainDomainSuffix string, if info.SupportedProtos != nil { for _, proto := range info.SupportedProtos { - if proto == tlsalpn01.ACMETLS1Protocol { - challenge, ok := challengeCache.Get(sni) - if !ok { - return nil, errors.New("no challenge for this domain") - } - cert, err := tlsalpn01.ChallengeCert(sni, challenge.(string)) - if err != nil { - return nil, err - } - return cert, nil + if proto != tlsalpn01.ACMETLS1Protocol { + continue } + + challenge, ok := challengeCache.Get(sni) + if !ok { + return nil, errors.New("no challenge for this domain") + } + cert, err := tlsalpn01.ChallengeCert(sni, challenge.(string)) + if err != nil { + return nil, err + } + return cert, nil } } @@ -195,7 +197,7 @@ func (a AcmeHTTPChallengeProvider) CleanUp(domain, token, _ string) error { func retrieveCertFromDB(sni, mainDomainSuffix, dnsProvider string, acmeUseRateLimits bool, certDB database.CertDB) (tls.Certificate, bool) { // parse certificate from database - res, err := certDB.Get(string(sni)) + res, err := certDB.Get(sni) if err != nil { panic(err) // TODO: no panic } @@ -216,7 +218,7 @@ func retrieveCertFromDB(sni, mainDomainSuffix, dnsProvider string, acmeUseRateLi } // renew certificates 7 days before they expire - if !tlsCertificate.Leaf.NotAfter.After(time.Now().Add(7 * 24 * time.Hour)) { + if tlsCertificate.Leaf.NotAfter.Before(time.Now().Add(7 * 24 * time.Hour)) { // TODO: add ValidUntil to custom res struct if res.CSR != nil && len(res.CSR) > 0 { // CSR stores the time when the renewal shall be tried again @@ -227,9 +229,9 @@ func retrieveCertFromDB(sni, mainDomainSuffix, dnsProvider string, acmeUseRateLi } go (func() { res.CSR = nil // acme client doesn't like CSR to be set - tlsCertificate, err = obtainCert(acmeClient, []string{string(sni)}, res, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) + tlsCertificate, err = obtainCert(acmeClient, []string{sni}, res, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) if err != nil { - log.Error().Msgf("Couldn't renew certificate for %s: %v", string(sni), err) + log.Error().Msgf("Couldn't renew certificate for %s: %v", sni, err) } })() } @@ -262,7 +264,7 @@ func obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Re defer obtainLocks.Delete(name) if acmeClient == nil { - return mockCert(domains[0], "ACME client uninitialized. This is a server error, please report!", string(mainDomainSuffix), keyDatabase), nil + return mockCert(domains[0], "ACME client uninitialized. This is a server error, please report!", mainDomainSuffix, keyDatabase), nil } // request actual cert @@ -305,12 +307,12 @@ func obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Re // avoid sending a mock cert instead of a still valid cert, instead abuse CSR field to store time to try again at renew.CSR = []byte(strconv.FormatInt(time.Now().Add(6*time.Hour).Unix(), 10)) if err := keyDatabase.Put(name, renew); err != nil { - return mockCert(domains[0], err.Error(), string(mainDomainSuffix), keyDatabase), err + return mockCert(domains[0], err.Error(), mainDomainSuffix, keyDatabase), err } return tlsCertificate, nil } } - return mockCert(domains[0], err.Error(), string(mainDomainSuffix), keyDatabase), err + return mockCert(domains[0], err.Error(), mainDomainSuffix, keyDatabase), err } log.Debug().Msgf("Obtained certificate for %v", domains) @@ -408,7 +410,7 @@ func SetupAcmeConfig(acmeAPI, acmeMail, acmeEabHmac, acmeEabKID string, acmeAcce func SetupCertificates(mainDomainSuffix, dnsProvider string, acmeConfig *lego.Config, acmeUseRateLimits, enableHTTPServer bool, challengeCache cache.SetGetKey, certDB database.CertDB) error { // getting main cert before ACME account so that we can fail here without hitting rate limits - mainCertBytes, err := certDB.Get(string(mainDomainSuffix)) + mainCertBytes, err := certDB.Get(mainDomainSuffix) if err != nil { return fmt.Errorf("cert database is not working") } @@ -452,7 +454,7 @@ func SetupCertificates(mainDomainSuffix, dnsProvider string, acmeConfig *lego.Co } if mainCertBytes == nil { - _, err = obtainCert(mainDomainAcmeClient, []string{"*" + string(mainDomainSuffix), string(mainDomainSuffix[1:])}, nil, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) + _, err = obtainCert(mainDomainAcmeClient, []string{"*" + mainDomainSuffix, mainDomainSuffix[1:]}, nil, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) if err != nil { log.Error().Err(err).Msg("Couldn't renew main domain certificate, continuing with mock certs only") } @@ -479,7 +481,7 @@ func MaintainCertDB(ctx context.Context, interval time.Duration, mainDomainSuffi } tlsCertificates, err := certcrypto.ParsePEMBundle(res.Certificate) - if err != nil || !tlsCertificates[0].NotAfter.After(now) { + if err != nil || tlsCertificates[0].NotAfter.Before(now) { err := certDB.Delete(string(key)) if err != nil { log.Error().Err(err).Msgf("Deleting expired certificate for %q failed", string(key)) @@ -501,18 +503,18 @@ func MaintainCertDB(ctx context.Context, interval time.Duration, mainDomainSuffi } // update main cert - res, err := certDB.Get(string(mainDomainSuffix)) + res, err := certDB.Get(mainDomainSuffix) if err != nil { log.Error().Msgf("Couldn't get cert for domain %q", mainDomainSuffix) } else if res == nil { - log.Error().Msgf("Couldn't renew certificate for main domain %q expected main domain cert to exist, but it's missing - seems like the database is corrupted", string(mainDomainSuffix)) + log.Error().Msgf("Couldn't renew certificate for main domain %q expected main domain cert to exist, but it's missing - seems like the database is corrupted", mainDomainSuffix) } else { tlsCertificates, err := certcrypto.ParsePEMBundle(res.Certificate) // renew main certificate 30 days before it expires - if !tlsCertificates[0].NotAfter.After(time.Now().Add(30 * 24 * time.Hour)) { + if tlsCertificates[0].NotAfter.Before(time.Now().Add(30 * 24 * time.Hour)) { go (func() { - _, err = obtainCert(mainDomainAcmeClient, []string{"*" + string(mainDomainSuffix), string(mainDomainSuffix[1:])}, res, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) + _, err = obtainCert(mainDomainAcmeClient, []string{"*" + mainDomainSuffix, mainDomainSuffix[1:]}, res, "", dnsProvider, mainDomainSuffix, acmeUseRateLimits, certDB) if err != nil { log.Error().Err(err).Msg("Couldn't renew certificate for main domain") } diff --git a/server/database/setup.go b/server/database/setup.go index 1c5a0af..097c63e 100644 --- a/server/database/setup.go +++ b/server/database/setup.go @@ -44,7 +44,7 @@ func (p aDB) Get(name string) (*certificate.Resource, error) { if resBytes == nil { return nil, nil } - if err = gob.NewDecoder(bytes.NewBuffer(resBytes)).Decode(cert); err != nil { + if err := gob.NewDecoder(bytes.NewBuffer(resBytes)).Decode(cert); err != nil { return nil, err } return cert, nil diff --git a/server/gitea/cache.go b/server/gitea/cache.go index b11a370..85cbcde 100644 --- a/server/gitea/cache.go +++ b/server/gitea/cache.go @@ -42,9 +42,8 @@ func (f FileResponse) IsEmpty() bool { return len(f.Body) != 0 } -func (f FileResponse) createHttpResponse(cacheKey string) (http.Header, int) { - header := make(http.Header) - var statusCode int +func (f FileResponse) createHttpResponse(cacheKey string) (header http.Header, statusCode int) { + header = make(http.Header) if f.Exists { statusCode = http.StatusOK diff --git a/server/handler/handler.go b/server/handler/handler.go index 1a0a285..78301e9 100644 --- a/server/handler/handler.go +++ b/server/handler/handler.go @@ -28,7 +28,7 @@ func Handler(mainDomainSuffix, rawDomain string, dnsLookupCache, canonicalDomainCache cache.SetGetKey, ) http.HandlerFunc { return func(w http.ResponseWriter, req *http.Request) { - log := log.With().Strs("Handler", []string{string(req.Host), req.RequestURI}).Logger() + log := log.With().Strs("Handler", []string{req.Host, req.RequestURI}).Logger() ctx := context.New(w, req) ctx.RespWriter.Header().Set("Server", "CodebergPages/"+version.Version) diff --git a/server/handler/handler_custom_domain.go b/server/handler/handler_custom_domain.go index 5ea7649..2f98085 100644 --- a/server/handler/handler_custom_domain.go +++ b/server/handler/handler_custom_domain.go @@ -54,7 +54,7 @@ func handleCustomDomain(log zerolog.Logger, ctx *context.Context, giteaClient *g // only redirect if the target is also a codeberg page! targetOwner, _, _ = dns.GetTargetFromDNS(strings.SplitN(canonicalDomain, "/", 2)[0], mainDomainSuffix, dnsLookupCache) if targetOwner != "" { - ctx.Redirect("https://"+canonicalDomain+string(targetOpt.TargetPath), http.StatusTemporaryRedirect) + ctx.Redirect("https://"+canonicalDomain+targetOpt.TargetPath, http.StatusTemporaryRedirect) return } diff --git a/server/handler/handler_sub_domain.go b/server/handler/handler_sub_domain.go index 73177b6..1d769d4 100644 --- a/server/handler/handler_sub_domain.go +++ b/server/handler/handler_sub_domain.go @@ -29,7 +29,7 @@ func handleSubDomain(log zerolog.Logger, ctx *context.Context, giteaClient *gite if targetOwner == "www" { // www.codeberg.page redirects to codeberg.page // TODO: rm hardcoded - use cname? - ctx.Redirect("https://"+string(mainDomainSuffix[1:])+string(ctx.Path()), http.StatusPermanentRedirect) + ctx.Redirect("https://"+mainDomainSuffix[1:]+ctx.Path(), http.StatusPermanentRedirect) return } diff --git a/server/handler/try.go b/server/handler/try.go index bae3c44..5a09b91 100644 --- a/server/handler/try.go +++ b/server/handler/try.go @@ -21,8 +21,8 @@ func tryUpstream(ctx *context.Context, giteaClient *gitea.Client, ) { // check if a canonical domain exists on a request on MainDomain if strings.HasSuffix(trimmedHost, mainDomainSuffix) { - canonicalDomain, _ := options.CheckCanonicalDomain(giteaClient, "", string(mainDomainSuffix), canonicalDomainCache) - if !strings.HasSuffix(strings.SplitN(canonicalDomain, "/", 2)[0], string(mainDomainSuffix)) { + canonicalDomain, _ := options.CheckCanonicalDomain(giteaClient, "", mainDomainSuffix, canonicalDomainCache) + if !strings.HasSuffix(strings.SplitN(canonicalDomain, "/", 2)[0], mainDomainSuffix) { canonicalPath := ctx.Req.RequestURI if options.TargetRepo != defaultPagesRepo { path := strings.SplitN(canonicalPath, "/", 3) @@ -35,8 +35,8 @@ func tryUpstream(ctx *context.Context, giteaClient *gitea.Client, } } - // add host for debugging - options.Host = string(trimmedHost) + // Add host for debugging. + options.Host = trimmedHost // Try to request the file from the Gitea API if !options.Upstream(ctx, giteaClient) { diff --git a/server/setup.go b/server/setup.go index e7194ed..282e692 100644 --- a/server/setup.go +++ b/server/setup.go @@ -15,13 +15,13 @@ func SetupHTTPACMEChallengeServer(challengeCache cache.SetGetKey) http.HandlerFu return func(w http.ResponseWriter, req *http.Request) { ctx := context.New(w, req) if strings.HasPrefix(ctx.Path(), challengePath) { - challenge, ok := challengeCache.Get(utils.TrimHostPort(ctx.Host()) + "/" + string(strings.TrimPrefix(ctx.Path(), challengePath))) + challenge, ok := challengeCache.Get(utils.TrimHostPort(ctx.Host()) + "/" + strings.TrimPrefix(ctx.Path(), challengePath)) if !ok || challenge == nil { ctx.String("no challenge for this token", http.StatusNotFound) } ctx.String(challenge.(string)) } else { - ctx.Redirect("https://"+string(ctx.Host())+string(ctx.Path()), http.StatusMovedPermanently) + ctx.Redirect("https://"+ctx.Host()+ctx.Path(), http.StatusMovedPermanently) } } } diff --git a/server/upstream/helper.go b/server/upstream/helper.go index 428976b..a84d4f0 100644 --- a/server/upstream/helper.go +++ b/server/upstream/helper.go @@ -13,7 +13,7 @@ import ( func (o *Options) GetBranchTimestamp(giteaClient *gitea.Client) (bool, error) { log := log.With().Strs("BranchInfo", []string{o.TargetOwner, o.TargetRepo, o.TargetBranch}).Logger() - if len(o.TargetBranch) == 0 { + if o.TargetBranch == "" { // Get default branch defaultBranch, err := giteaClient.GiteaGetRepoDefaultBranch(o.TargetOwner, o.TargetRepo) if err != nil { diff --git a/server/upstream/upstream.go b/server/upstream/upstream.go index b76b8e6..7c3c848 100644 --- a/server/upstream/upstream.go +++ b/server/upstream/upstream.go @@ -82,8 +82,8 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client) (fin // Check if the browser has a cached version if ctx.Response() != nil { - if ifModifiedSince, err := time.Parse(time.RFC1123, string(ctx.Response().Header.Get(headerIfModifiedSince))); err == nil { - if !ifModifiedSince.Before(o.BranchTimestamp) { + if ifModifiedSince, err := time.Parse(time.RFC1123, ctx.Response().Header.Get(headerIfModifiedSince)); err == nil { + if ifModifiedSince.After(o.BranchTimestamp) { ctx.RespWriter.WriteHeader(http.StatusNotModified) log.Trace().Msg("check response against last modified: valid") return true