From f2f943c0d800c513025a2fecfb1a48729052fdc1 Mon Sep 17 00:00:00 2001
From: Gusted <gusted@noreply.codeberg.org>
Date: Tue, 15 Nov 2022 16:15:11 +0100
Subject: [PATCH 1/8] Remove unnecessary conversion (#139)
- Remove unnecessary type conversion.
- Enforce via CI
Co-authored-by: Gusted <williamzijl7@hotmail.com>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/139
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Gusted <gusted@noreply.codeberg.org>
Co-committed-by: Gusted <gusted@noreply.codeberg.org>
---
.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
From caeb1a4acb0d35c78988fcd4e30d9ac51847f13a Mon Sep 17 00:00:00 2001
From: jklippel <jklippel@noreply.codeberg.org>
Date: Tue, 22 Nov 2022 21:26:10 +0000
Subject: [PATCH 2/8] Return a 404 if there is no repository (#141)
If no repository is found the user expects a 404 status code
instead of a dependency failed status code (as it was before).
Signed-off-by: Jan Klippel <c0d3b3rg@kl1pp3l.de>
Fixes: https://codeberg.org/Codeberg/Community/issues/809
Co-authored-by: Jan Klippel <c0d3b3rg@kl1pp3l.de>
Co-authored-by: 6543 <6543@obermui.de>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/141
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: jklippel <jklippel@noreply.codeberg.org>
Co-committed-by: jklippel <jklippel@noreply.codeberg.org>
---
server/handler/handler_sub_domain.go | 4 ++--
server/handler/handler_test.go | 35 ++++++++++++++--------------
2 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/server/handler/handler_sub_domain.go b/server/handler/handler_sub_domain.go
index 1d769d4..2a75e9f 100644
--- a/server/handler/handler_sub_domain.go
+++ b/server/handler/handler_sub_domain.go
@@ -115,6 +115,6 @@ func handleSubDomain(log zerolog.Logger, ctx *context.Context, giteaClient *gite
// Couldn't find a valid repo/branch
html.ReturnErrorPage(ctx,
- fmt.Sprintf("couldn't find a valid repo[%s]", targetRepo),
- http.StatusFailedDependency)
+ fmt.Sprintf("could not find a valid repository[%s]", targetRepo),
+ http.StatusNotFound)
}
diff --git a/server/handler/handler_test.go b/server/handler/handler_test.go
index f5538c9..626564a 100644
--- a/server/handler/handler_test.go
+++ b/server/handler/handler_test.go
@@ -23,26 +23,27 @@ func TestHandlerPerformance(t *testing.T) {
)
testCase := func(uri string, status int) {
- req := httptest.NewRequest("GET", uri, nil)
- w := httptest.NewRecorder()
+ t.Run(uri, func(t *testing.T) {
+ req := httptest.NewRequest("GET", uri, nil)
+ w := httptest.NewRecorder()
- log.Printf("Start: %v\n", time.Now())
- start := time.Now()
- testHandler(w, req)
- end := time.Now()
- log.Printf("Done: %v\n", time.Now())
+ log.Printf("Start: %v\n", time.Now())
+ start := time.Now()
+ testHandler(w, req)
+ end := time.Now()
+ log.Printf("Done: %v\n", time.Now())
- resp := w.Result()
+ resp := w.Result()
- if resp.StatusCode != status {
- t.Errorf("request failed with status code %d", resp.StatusCode)
- } else {
- t.Logf("request took %d milliseconds", end.Sub(start).Milliseconds())
- }
+ if resp.StatusCode != status {
+ t.Errorf("request failed with status code %d", resp.StatusCode)
+ } else {
+ t.Logf("request took %d milliseconds", end.Sub(start).Milliseconds())
+ }
+ })
}
- testCase("https://mondstern.codeberg.page/", 424) // TODO: expect 200
- testCase("https://mondstern.codeberg.page/", 424) // TODO: expect 200
- testCase("https://example.momar.xyz/", 424) // TODO: expect 200
- testCase("https://codeberg.page/", 424) // TODO: expect 200
+ testCase("https://mondstern.codeberg.page/", 404) // TODO: expect 200
+ testCase("https://codeberg.page/", 404) // TODO: expect 200
+ testCase("https://example.momar.xyz/", 424)
}
From 5e72753e916d1076cfd772d3d6c7f86474c6c87a Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Tue, 22 Nov 2022 22:30:53 +0100
Subject: [PATCH 3/8] ci: "docker-tag" use `tags`
---
.woodpecker.yml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/.woodpecker.yml b/.woodpecker.yml
index 20254fe..7e6d694 100644
--- a/.woodpecker.yml
+++ b/.woodpecker.yml
@@ -102,7 +102,7 @@ pipeline:
registry: codeberg.org
dockerfile: Dockerfile
repo: codeberg.org/codeberg/pages-server
- tag: [ latest, "${CI_COMMIT_TAG}" ]
+ tags: [ latest, "${CI_COMMIT_TAG}" ]
username:
from_secret: bot_user
password:
From dcf03fc078228a286384757c3501ab5df5306062 Mon Sep 17 00:00:00 2001
From: crapStone <crapstone@noreply.codeberg.org>
Date: Fri, 2 Dec 2022 15:25:25 +0000
Subject: [PATCH 4/8] Fix error page (#144)
Co-authored-by: crapStone <crapstone01@gmail.com>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/144
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: crapStone <crapstone@noreply.codeberg.org>
Co-committed-by: crapStone <crapstone@noreply.codeberg.org>
---
html/error.go | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/html/error.go b/html/error.go
index 826c42b..ac222c4 100644
--- a/html/error.go
+++ b/html/error.go
@@ -1,6 +1,7 @@
package html
import (
+ "html/template"
"net/http"
"strconv"
"strings"
@@ -39,7 +40,8 @@ func errorMessage(statusCode int) string {
// TODO: use template engine
func errorBody(statusCode int) string {
- return strings.ReplaceAll(NotFoundPage,
- "%status%",
- strconv.Itoa(statusCode)+" "+errorMessage(statusCode))
+ return template.HTMLEscapeString(
+ strings.ReplaceAll(NotFoundPage,
+ "%status%",
+ strconv.Itoa(statusCode)+" "+errorMessage(statusCode)))
}
From 9d769aeee7bdbdce03bbb0f6fb213d4b7d45a701 Mon Sep 17 00:00:00 2001
From: crapStone <crapstone@noreply.codeberg.org>
Date: Sun, 4 Dec 2022 21:24:58 +0000
Subject: [PATCH 5/8] Fix error page generation (#145)
Co-authored-by: crapStone <crapstone01@gmail.com>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/145
Reviewed-by: Gusted <gusted@noreply.codeberg.org>
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: crapStone <crapstone@noreply.codeberg.org>
Co-committed-by: crapStone <crapstone@noreply.codeberg.org>
---
Justfile | 4 ++--
html/error.go | 31 +++++++++++++++++--------------
html/error_test.go | 38 ++++++++++++++++++++++++++++++++++++++
3 files changed, 57 insertions(+), 16 deletions(-)
create mode 100644 html/error_test.go
diff --git a/Justfile b/Justfile
index f7ea414..7d72fe8 100644
--- a/Justfile
+++ b/Justfile
@@ -38,10 +38,10 @@ tool-gofumpt:
fi
test:
- go test -race codeberg.org/codeberg/pages/server/...
+ go test -race codeberg.org/codeberg/pages/server/... codeberg.org/codeberg/pages/html/
test-run TEST:
- go test -race -run "^{{TEST}}$" codeberg.org/codeberg/pages/server/...
+ go test -race -run "^{{TEST}}$" codeberg.org/codeberg/pages/server/... codeberg.org/codeberg/pages/html/
integration:
go test -race -tags integration codeberg.org/codeberg/pages/integration/...
diff --git a/html/error.go b/html/error.go
index ac222c4..206b123 100644
--- a/html/error.go
+++ b/html/error.go
@@ -15,16 +15,27 @@ func ReturnErrorPage(ctx *context.Context, msg string, statusCode int) {
ctx.RespWriter.Header().Set("Content-Type", "text/html; charset=utf-8")
ctx.RespWriter.WriteHeader(statusCode)
- if msg == "" {
- msg = errorBody(statusCode)
- } else {
- // TODO: use template engine
- msg = strings.ReplaceAll(strings.ReplaceAll(ErrorPage, "%message%", msg), "%status%", http.StatusText(statusCode))
- }
+ msg = generateResponse(msg, statusCode)
_, _ = ctx.RespWriter.Write([]byte(msg))
}
+// TODO: use template engine
+func generateResponse(msg string, statusCode int) string {
+ if msg == "" {
+ msg = strings.ReplaceAll(NotFoundPage,
+ "%status%",
+ strconv.Itoa(statusCode)+" "+errorMessage(statusCode))
+ } else {
+ msg = strings.ReplaceAll(
+ strings.ReplaceAll(ErrorPage, "%message%", template.HTMLEscapeString(msg)),
+ "%status%",
+ http.StatusText(statusCode))
+ }
+
+ return msg
+}
+
func errorMessage(statusCode int) string {
message := http.StatusText(statusCode)
@@ -37,11 +48,3 @@ func errorMessage(statusCode int) string {
return message
}
-
-// TODO: use template engine
-func errorBody(statusCode int) string {
- return template.HTMLEscapeString(
- strings.ReplaceAll(NotFoundPage,
- "%status%",
- strconv.Itoa(statusCode)+" "+errorMessage(statusCode)))
-}
diff --git a/html/error_test.go b/html/error_test.go
new file mode 100644
index 0000000..f5da08c
--- /dev/null
+++ b/html/error_test.go
@@ -0,0 +1,38 @@
+package html
+
+import (
+ "net/http"
+ "strings"
+ "testing"
+)
+
+func TestValidMessage(t *testing.T) {
+ testString := "requested blacklisted path"
+ statusCode := http.StatusForbidden
+
+ expected := strings.ReplaceAll(
+ strings.ReplaceAll(ErrorPage, "%message%", testString),
+ "%status%",
+ http.StatusText(statusCode))
+ actual := generateResponse(testString, statusCode)
+
+ if expected != actual {
+ t.Errorf("generated response did not match: expected: '%s', got: '%s'", expected, actual)
+ }
+}
+
+func TestMessageWithHtml(t *testing.T) {
+ testString := `abc<img src=1 onerror=alert("xss");`
+ escapedString := "abc<img src=1 onerror=alert("xss");"
+ statusCode := http.StatusNotFound
+
+ expected := strings.ReplaceAll(
+ strings.ReplaceAll(ErrorPage, "%message%", escapedString),
+ "%status%",
+ http.StatusText(statusCode))
+ actual := generateResponse(testString, statusCode)
+
+ if expected != actual {
+ t.Errorf("generated response did not match: expected: '%s', got: '%s'", expected, actual)
+ }
+}
From 98d198d4197f35af4a50dafab6ff6766eac06faf Mon Sep 17 00:00:00 2001
From: Gusted <gusted@noreply.codeberg.org>
Date: Wed, 4 Jan 2023 04:51:27 +0000
Subject: [PATCH 6/8] Safely get certificate's leaf (#150)
- It's not guaranteed that `tls.X509KeyPair` will set `c.Leaf`.
- This patch fixes this by using a wrapper that parses the leaf
certificate(in bytes) if `c.Leaf` wasn't set.
- Resolves #149
Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/150
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Gusted <gusted@noreply.codeberg.org>
Co-committed-by: Gusted <gusted@noreply.codeberg.org>
---
server/certificates/certificates.go | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/server/certificates/certificates.go b/server/certificates/certificates.go
index 8af4be5..e3cdbfb 100644
--- a/server/certificates/certificates.go
+++ b/server/certificates/certificates.go
@@ -303,7 +303,11 @@ func obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Re
log.Error().Err(err).Msgf("Couldn't obtain again a certificate or %v", domains)
if renew != nil && renew.CertURL != "" {
tlsCertificate, err := tls.X509KeyPair(renew.Certificate, renew.PrivateKey)
- if err == nil && tlsCertificate.Leaf.NotAfter.After(time.Now()) {
+ if err != nil {
+ return mockCert(domains[0], err.Error(), mainDomainSuffix, keyDatabase), err
+ }
+ leaf, err := leaf(&tlsCertificate)
+ if err == nil && leaf.NotAfter.After(time.Now()) {
// 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 {
@@ -529,3 +533,12 @@ func MaintainCertDB(ctx context.Context, interval time.Duration, mainDomainSuffi
}
}
}
+
+// leaf returns the parsed leaf certificate, either from c.leaf or by parsing
+// the corresponding c.Certificate[0].
+func leaf(c *tls.Certificate) (*x509.Certificate, error) {
+ if c.Leaf != nil {
+ return c.Leaf, nil
+ }
+ return x509.ParseCertificate(c.Certificate[0])
+}
From f7fad2a5ae0e4942848604a77199f4f1f8a1c3b4 Mon Sep 17 00:00:00 2001
From: 6543 <6543@obermui.de>
Date: Wed, 4 Jan 2023 06:08:06 +0100
Subject: [PATCH 7/8] Integration Tests use https://codeberg.org/cb_pages_tests
---
integration/get_test.go | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/integration/get_test.go b/integration/get_test.go
index 81d8488..f13ce8b 100644
--- a/integration/get_test.go
+++ b/integration/get_test.go
@@ -31,7 +31,7 @@ func TestGetRedirect(t *testing.T) {
func TestGetContent(t *testing.T) {
log.Println("=== TestGetContent ===")
// test get image
- resp, err := getTestHTTPSClient().Get("https://magiclike.localhost.mock.directory:4430/images/827679288a.jpg")
+ resp, err := getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/images/827679288a.jpg")
assert.NoError(t, err)
if !assert.EqualValues(t, http.StatusOK, resp.StatusCode) {
t.FailNow()
@@ -42,7 +42,7 @@ func TestGetContent(t *testing.T) {
assert.Len(t, resp.Header.Get("ETag"), 42)
// specify branch
- resp, err = getTestHTTPSClient().Get("https://momar.localhost.mock.directory:4430/pag/@master/")
+ resp, err = getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/pag/@master/")
assert.NoError(t, err)
if !assert.NotNil(t, resp) {
t.FailNow()
@@ -53,7 +53,7 @@ func TestGetContent(t *testing.T) {
assert.Len(t, resp.Header.Get("ETag"), 44)
// access branch name contains '/'
- resp, err = getTestHTTPSClient().Get("https://blumia.localhost.mock.directory:4430/pages-server-integration-tests/@docs~main/")
+ resp, err = getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/blumia/@docs~main/")
assert.NoError(t, err)
if !assert.EqualValues(t, http.StatusOK, resp.StatusCode) {
t.FailNow()
@@ -81,7 +81,7 @@ func TestCustomDomain(t *testing.T) {
func TestGetNotFound(t *testing.T) {
log.Println("=== TestGetNotFound ===")
// test custom not found pages
- resp, err := getTestHTTPSClient().Get("https://crystal.localhost.mock.directory:4430/pages-404-demo/blah")
+ resp, err := getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/pages-404-demo/blah")
assert.NoError(t, err)
if !assert.NotNil(t, resp) {
t.FailNow()
@@ -95,7 +95,7 @@ func TestGetNotFound(t *testing.T) {
func TestFollowSymlink(t *testing.T) {
log.Printf("=== TestFollowSymlink ===\n")
- resp, err := getTestHTTPSClient().Get("https://6543.localhost.mock.directory:4430/tests_for_pages-server/@main/link")
+ resp, err := getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/tests_for_pages-server/@main/link")
assert.NoError(t, err)
if !assert.NotNil(t, resp) {
t.FailNow()
@@ -111,7 +111,7 @@ func TestFollowSymlink(t *testing.T) {
func TestLFSSupport(t *testing.T) {
log.Printf("=== TestLFSSupport ===\n")
- resp, err := getTestHTTPSClient().Get("https://6543.localhost.mock.directory:4430/tests_for_pages-server/@main/lfs.txt")
+ resp, err := getTestHTTPSClient().Get("https://cb_pages_tests.localhost.mock.directory:4430/tests_for_pages-server/@main/lfs.txt")
assert.NoError(t, err)
if !assert.NotNil(t, resp) {
t.FailNow()
From c286b3b1d0a21bc5cc447df7744da9894d857e6a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Felipe=20Leopoldo=20Sologuren=20Guti=C3=A9rrez?=
<fsologureng@noreply.codeberg.org>
Date: Wed, 4 Jan 2023 05:26:14 +0000
Subject: [PATCH 8/8] Added TokenBucket to limit the rate of validation
failures (#151)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Added new TockenBucket named `acmeClientFailLimit` to avoid being banned because of the [Failed validation limit](https://letsencrypt.org/docs/failed-validation-limit/) of Let's Encrypt.
The behaviour is similar to the other limiters blocking the `obtainCert` func ensuring rate under limit.
Co-authored-by: fsologureng <sologuren@estudiohum.cl>
Co-authored-by: 6543 <6543@obermui.de>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/151
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Felipe Leopoldo Sologuren Gutiérrez <fsologureng@noreply.codeberg.org>
Co-committed-by: Felipe Leopoldo Sologuren Gutiérrez <fsologureng@noreply.codeberg.org>
---
server/certificates/certificates.go | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/server/certificates/certificates.go b/server/certificates/certificates.go
index e3cdbfb..1aa90a0 100644
--- a/server/certificates/certificates.go
+++ b/server/certificates/certificates.go
@@ -163,6 +163,9 @@ var acmeClientOrderLimit = equalizer.NewTokenBucket(25, 15*time.Minute)
// rate limit is 20 / second, we want 5 / second (especially as one cert takes at least two requests)
var acmeClientRequestLimit = equalizer.NewTokenBucket(5, 1*time.Second)
+// rate limit is 5 / hour https://letsencrypt.org/docs/failed-validation-limit/
+var acmeClientFailLimit = equalizer.NewTokenBucket(5, 1*time.Hour)
+
type AcmeTLSChallengeProvider struct {
challengeCache cache.SetGetKey
}
@@ -278,6 +281,9 @@ func obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Re
res, err = acmeClient.Certificate.Renew(*renew, true, false, "")
if err != nil {
log.Error().Err(err).Msgf("Couldn't renew certificate for %v, trying to request a new one", domains)
+ if acmeUseRateLimits {
+ acmeClientFailLimit.Take()
+ }
res = nil
}
}
@@ -298,6 +304,9 @@ func obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Re
Bundle: true,
MustStaple: false,
})
+ if acmeUseRateLimits && err != nil {
+ acmeClientFailLimit.Take()
+ }
}
if err != nil {
log.Error().Err(err).Msgf("Couldn't obtain again a certificate or %v", domains)