Porównaj commity

...

5 Commity

Autor SHA1 Wiadomość Data
Jean-Marie 'Histausse' Mineau ec2a9e6f28
fix lint 2024-03-26 13:17:35 +01:00
Jean-Marie 'Histausse' Mineau 6fc224b359
simplify wildecard logic 2024-03-26 13:07:33 +01:00
6543 376d4e0b7c Merge branch 'main' into main 2024-03-24 09:29:49 +00:00
caelandb dd6d8bd60f fixed one grammar error. (#297)
Just a small grammar change in the README.

Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/297
Reviewed-by: crapStone <codeberg@crapstone.dev>
Co-authored-by: caelandb <bothacaelan@gmail.com>
Co-committed-by: caelandb <bothacaelan@gmail.com>
2024-03-12 21:50:17 +00:00
Hoernschen a6e9510c07 FIX blank internal pages (#164) (#292)
Hello 👋

since it affected my deployment of the pages server I started to look into the problem of the blank pages and think I found a solution for it:

1. There is no check if the file response is empty, neither in cache retrieval nor in writing of a cache. Also the provided method for checking for empty responses had a bug.
2. I identified the redirect response to be the issue here. There is a cache write with the full cache key (e. g. rawContent/user/repo|branch|route/index.html) happening in the handling of the redirect response. But the written body here is empty. In the triggered request from the redirect response the server then finds a cache item to the key and serves the empty body. A quick fix is the check for empty file responses mentioned in 1.
3. The decision to redirect the user comes quite far down in the upstream function. Before that happens a lot of stuff that may not be important since after the redirect response comes a new request anyway. Also, I suspect that this causes the caching problem because there is a request to the forge server and its error handling with some recursions happening before. I propose to move two of the redirects before "Preparing"
4. The recursion in the upstream function makes it difficult to understand what is actually happening. I added some more logging to have an easier time with that.
5. I changed the default behaviour to append a trailing slash to the path to true. In my tested scenarios it happened anyway. This way there is no recursion happening before the redirect.

I am not developing in go frequently and rarely contribute to open source -> so feedback of all kind is appreciated

closes #164

Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/292
Reviewed-by: 6543 <6543@obermui.de>
Reviewed-by: crapStone <codeberg@crapstone.dev>
Co-authored-by: Hoernschen <julian.hoernschemeyer@mailbox.org>
Co-committed-by: Hoernschen <julian.hoernschemeyer@mailbox.org>
2024-02-26 22:21:42 +00:00
6 zmienionych plików z 43 dodań i 25 usunięć

Wyświetl plik

@ -117,7 +117,7 @@ Thank you very much.
Make sure you have [golang](https://go.dev) v1.21 or newer and [just](https://just.systems/man/en/) installed.
run `just dev`
now this pages should work:
now these pages should work:
- <https://cb_pages_tests.localhost.mock.directory:4430/images/827679288a.jpg>
- <https://momar.localhost.mock.directory:4430/ci-testing/>

Wyświetl plik

@ -216,9 +216,6 @@ func (c *AcmeClient) retrieveCertFromDB(sni, mainDomainSuffix string, useDnsProv
func (c *AcmeClient) obtainCert(acmeClient *lego.Client, domains []string, renew *certificate.Resource, user string, useDnsProvider bool, mainDomainSuffix string, keyDatabase database.CertDB) (*tls.Certificate, error) {
name := strings.TrimPrefix(domains[0], "*")
if useDnsProvider && domains[0] != "" && domains[0][0] == '*' {
domains = domains[1:]
}
// lock to avoid simultaneous requests
_, working := c.obtainLocks.LoadOrStore(name, struct{}{})
@ -237,11 +234,7 @@ func (c *AcmeClient) obtainCert(acmeClient *lego.Client, domains []string, renew
if acmeClient == nil {
if useDnsProvider {
mock_domain := domains[0]
if name == mainDomainSuffix {
mock_domain = "*" + mainDomainSuffix
}
return mockCert(mock_domain, "DNS ACME client is not defined", mainDomainSuffix, keyDatabase)
return mockCert(domains[0], "DNS ACME client is not defined", mainDomainSuffix, keyDatabase)
} else {
return mockCert(domains[0], "ACME client uninitialized. This is a server error, please report!", mainDomainSuffix, keyDatabase)
}

Wyświetl plik

@ -42,7 +42,7 @@ type FileResponse struct {
}
func (f FileResponse) IsEmpty() bool {
return len(f.Body) != 0
return len(f.Body) == 0
}
func (f FileResponse) createHttpResponse(cacheKey string) (header http.Header, statusCode int) {
@ -75,13 +75,14 @@ type BranchTimestamp struct {
type writeCacheReader struct {
originalReader io.ReadCloser
buffer *bytes.Buffer
rileResponse *FileResponse
fileResponse *FileResponse
cacheKey string
cache cache.ICache
hasError bool
}
func (t *writeCacheReader) Read(p []byte) (n int, err error) {
log.Trace().Msgf("[cache] read %q", t.cacheKey)
n, err = t.originalReader.Read(p)
if err != nil && err != io.EOF {
log.Trace().Err(err).Msgf("[cache] original reader for %q has returned an error", t.cacheKey)
@ -93,12 +94,20 @@ func (t *writeCacheReader) Read(p []byte) (n int, err error) {
}
func (t *writeCacheReader) Close() error {
if !t.hasError {
fc := *t.rileResponse
fc.Body = t.buffer.Bytes()
_ = t.cache.Set(t.cacheKey, fc, fileCacheTimeout)
doWrite := !t.hasError
fc := *t.fileResponse
fc.Body = t.buffer.Bytes()
if fc.IsEmpty() {
log.Trace().Msg("[cache] file response is empty")
doWrite = false
}
log.Trace().Msgf("cacheReader for %q saved=%t closed", t.cacheKey, !t.hasError)
if doWrite {
err := t.cache.Set(t.cacheKey, fc, fileCacheTimeout)
if err != nil {
log.Trace().Err(err).Msgf("[cache] writer for %q has returned an error", t.cacheKey)
}
}
log.Trace().Msgf("cacheReader for %q saved=%t closed", t.cacheKey, doWrite)
return t.originalReader.Close()
}
@ -111,7 +120,7 @@ func (f FileResponse) CreateCacheReader(r io.ReadCloser, cache cache.ICache, cac
return &writeCacheReader{
originalReader: r,
buffer: bytes.NewBuffer(make([]byte, 0)),
rileResponse: &f,
fileResponse: &f,
cache: cache,
cacheKey: cacheKey,
}

Wyświetl plik

@ -113,26 +113,27 @@ func (client *Client) GiteaRawContent(targetOwner, targetRepo, ref, resource str
func (client *Client) ServeRawContent(targetOwner, targetRepo, ref, resource string) (io.ReadCloser, http.Header, int, error) {
cacheKey := fmt.Sprintf("%s/%s/%s|%s|%s", rawContentCacheKeyPrefix, targetOwner, targetRepo, ref, resource)
log := log.With().Str("cache_key", cacheKey).Logger()
log.Trace().Msg("try file in cache")
// handle if cache entry exist
if cache, ok := client.responseCache.Get(cacheKey); ok {
cache := cache.(FileResponse)
cachedHeader, cachedStatusCode := cache.createHttpResponse(cacheKey)
// TODO: check against some timestamp mismatch?!?
if cache.Exists {
log.Debug().Msg("[cache] exists")
if cache.IsSymlink {
linkDest := string(cache.Body)
log.Debug().Msgf("[cache] follow symlink from %q to %q", resource, linkDest)
return client.ServeRawContent(targetOwner, targetRepo, ref, linkDest)
} else {
log.Debug().Msg("[cache] return bytes")
} else if !cache.IsEmpty() {
log.Debug().Msgf("[cache] return %d bytes", len(cache.Body))
return io.NopCloser(bytes.NewReader(cache.Body)), cachedHeader, cachedStatusCode, nil
} else if cache.IsEmpty() {
log.Debug().Msg("[cache] is empty")
}
} else {
return nil, cachedHeader, cachedStatusCode, ErrorNotFound
}
}
log.Trace().Msg("file not in cache")
// not in cache, open reader via gitea api
reader, resp, err := client.sdkClient.GetFileReader(targetOwner, targetRepo, ref, resource, client.supportLFS)
if resp != nil {
@ -156,12 +157,14 @@ func (client *Client) ServeRawContent(targetOwner, targetRepo, ref, resource str
linkDest = path.Join(path.Dir(resource), linkDest)
// we store symlink not content to reduce duplicates in cache
if err := client.responseCache.Set(cacheKey, FileResponse{
fileResponse := FileResponse{
Exists: true,
IsSymlink: true,
Body: []byte(linkDest),
ETag: resp.Header.Get(ETagHeader),
}, fileCacheTimeout); err != nil {
}
log.Trace().Msgf("file response has %d bytes", len(fileResponse.Body))
if err := client.responseCache.Set(cacheKey, fileResponse, fileCacheTimeout); err != nil {
log.Error().Err(err).Msg("[cache] error on cache write")
}

Wyświetl plik

@ -26,6 +26,7 @@ func Handler(
dnsLookupCache, canonicalDomainCache, redirectsCache cache.ICache,
) http.HandlerFunc {
return func(w http.ResponseWriter, req *http.Request) {
log.Debug().Msg("\n----------------------------------------------------------")
log := log.With().Strs("Handler", []string{req.Host, req.RequestURI}).Logger()
ctx := context.New(w, req)

Wyświetl plik

@ -56,6 +56,8 @@ type Options struct {
func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client, redirectsCache cache.ICache) bool {
log := log.With().Strs("upstream", []string{o.TargetOwner, o.TargetRepo, o.TargetBranch, o.TargetPath}).Logger()
log.Debug().Msg("Start")
if o.TargetOwner == "" || o.TargetRepo == "" {
html.ReturnErrorPage(ctx, "forge client: either repo owner or name info is missing", http.StatusBadRequest)
return true
@ -104,13 +106,16 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client, redi
// Handle not found error
if err != nil && errors.Is(err, gitea.ErrorNotFound) {
log.Debug().Msg("Handling not found error")
// Get and match redirects
redirects := o.getRedirects(giteaClient, redirectsCache)
if o.matchRedirects(ctx, giteaClient, redirects, redirectsCache) {
log.Trace().Msg("redirect")
return true
}
if o.TryIndexPages {
log.Trace().Msg("try index page")
// copy the o struct & try if an index page exists
optionsForIndexPages := *o
optionsForIndexPages.TryIndexPages = false
@ -121,6 +126,7 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client, redi
return true
}
}
log.Trace().Msg("try html file with path name")
// compatibility fix for GitHub Pages (/example → /example.html)
optionsForIndexPages.appendTrailingSlash = false
optionsForIndexPages.redirectIfExists = strings.TrimSuffix(ctx.Path(), "/") + ".html"
@ -130,8 +136,11 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client, redi
}
}
log.Trace().Msg("not found")
ctx.StatusCode = http.StatusNotFound
if o.TryIndexPages {
log.Trace().Msg("try not found page")
// copy the o struct & try if a not found page exists
optionsForNotFoundPages := *o
optionsForNotFoundPages.TryIndexPages = false
@ -142,6 +151,7 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client, redi
return true
}
}
log.Trace().Msg("not found page missing")
}
return false
@ -173,10 +183,12 @@ func (o *Options) Upstream(ctx *context.Context, giteaClient *gitea.Client, redi
// Append trailing slash if missing (for index files), and redirect to fix filenames in general
// o.appendTrailingSlash is only true when looking for index pages
if o.appendTrailingSlash && !strings.HasSuffix(ctx.Path(), "/") {
log.Trace().Msg("append trailing slash and redirect")
ctx.Redirect(ctx.Path()+"/", http.StatusTemporaryRedirect)
return true
}
if strings.HasSuffix(ctx.Path(), "/index.html") && !o.ServeRaw {
log.Trace().Msg("remove index.html from path and redirect")
ctx.Redirect(strings.TrimSuffix(ctx.Path(), "index.html"), http.StatusTemporaryRedirect)
return true
}