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>
A database bug in xorm.go prevents the pages-server from saving a
renewed certificate for a domain that already has one in the database.
Co-authored-by: crystal <crystal@noreply.codeberg.org>
Co-authored-by: 6543 <6543@obermui.de>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/209
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Crystal <crystal@noreply.codeberg.org>
Co-committed-by: Crystal <crystal@noreply.codeberg.org>
- Currently if the canonical domain validations fails(either for
legitimate reasons or for bug reasons like the request to Gitea/Forgejo
failing) it will use main domain certificate, which in the case for
custom domains will warrant a security error as the certificate isn't
issued to the custom domain.
- This patch handles this situation more gracefully and instead only
disallow obtaining a certificate if the domain validation fails, so in
the case that a certificate still exists it can still be used even if
the canonical domain validation fails. There's a small side effect,
legitimate users that remove domains from `.domain` will still be able
to use the removed domain(as long as the DNS records exists) as long as
the certificate currently hold by pages-server isn't expired.
- Given the increased usage in custom domains that are resulting in
errors, I think it ways more than the side effect.
- In order to future-proof against future slowdowns of instances, add a retry mechanism to the domain validation function, such that it's more likely to succeed even if the instance is not responding.
- Refactor the code a bit and add some comments.
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-authored-by: 6543 <6543@obermui.de>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/160
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Gusted <gusted@noreply.codeberg.org>
Co-committed-by: Gusted <gusted@noreply.codeberg.org>
- Currently any error generated by requesting the `.domains` file of a repository would be logged under the info log level, which isn't the correct log level when we exclude the not found error.
- Use warn log level if the error isn't the not found error.
Co-authored-by: Gusted <postmaster@gusted.xyz>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/162
Reviewed-by: Otto <otto@codeberg.org>
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>
- 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>
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>
As per [the documentation](https://pkg.go.dev/net/http#Serve), it doesn't enable HTTP2 by-default, unless we enable it via the `NextProtos` option.
Co-authored-by: Gusted <williamzijl7@hotmail.com>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/137
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Gusted <gusted@noreply.codeberg.org>
Co-committed-by: Gusted <gusted@noreply.codeberg.org>
we have big functions that handle all stuff ... we should split this into smaler chuncks so we could test them seperate and make clear cuts in what happens where
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/135
- For production(*cough* Codeberg *cough*), it's important to not use
mock certs. So fail right from the start if this is the case and not try
to "handle it gracefully", as it would break production.
- Resolves#131
CC @6543
Co-authored-by: Gusted <williamzijl7@hotmail.com>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/133
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Gusted <gusted@noreply.codeberg.org>
Co-committed-by: Gusted <gusted@noreply.codeberg.org>
- Logs are currently indicating that it's returning `nil` in valid
scenarios, therefor this patch adds extra logging in this code to
better understand what it is doing in this function.
Co-authored-by: Gusted <williamzijl7@hotmail.com>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/130
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Gusted <gusted@noreply.codeberg.org>
Co-committed-by: Gusted <gusted@noreply.codeberg.org>
- It's likely that the tryBranch is returning false when it should be returning true, make these logs more verbose so they show up on production logs.
Co-authored-by: Gusted <williamzijl7@hotmail.com>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/127
Reviewed-by: 6543 <6543@obermui.de>
Co-authored-by: Gusted <gusted@noreply.codeberg.org>
Co-committed-by: Gusted <gusted@noreply.codeberg.org>
- Add the host to the Handler's logging fields, so you don't just see the path, but also which domain was being requested.
Co-authored-by: Gusted <williamzijl7@hotmail.com>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/123
Reviewed-by: 6543 <6543@noreply.codeberg.org>
Co-authored-by: Gusted <gusted@noreply.codeberg.org>
Co-committed-by: Gusted <gusted@noreply.codeberg.org>
- Actually log useful information at their respective log level.
- Add logs in hot-paths to be able to deep-dive and debug specific requests (see server/handler.go)
- Add more information to existing fields(e.g. the host that the user is visiting, this was noted by @fnetX).
Co-authored-by: Gusted <williamzijl7@hotmail.com>
Reviewed-on: https://codeberg.org/Codeberg/pages-server/pulls/116
Reviewed-by: 6543 <6543@noreply.codeberg.org>
Co-authored-by: Gusted <gusted@noreply.codeberg.org>
Co-committed-by: Gusted <gusted@noreply.codeberg.org>
