Codeberg/pages-server
25
366
Fork
You've already forked pages-server
52

Implement static serving of compressed files #387

Merged
crapStone merged 3 commits from :accept-encoding into main 2024年09月29日 23:00:55 +02:00
Contributor
Copy link

This provides an option for #223 without fully resolving it. (I think.)

Essentially, it acts very similar to the gzip_static and similar options for nginx, where it will check for the existence of pre-compressed files and serve those instead if the client allows it. I couldn't find a pre-existing way to actually parse the Accept-Encoding header properly (admittedly didn't look very hard) and just implemented one on my own that should be fine.

This should hopefully not have the same DOS vulnerabilities as #302, since it relies on the existing caching system. Compressed versions of files will be cached just like any other files, and that includes cache for missing files as well.

The compressed files will also be accessible directly, and this won't automatically decompress them. So, if you have a tar.gz file that you access directly, it will still be downloaded as the gzipped version, although you will now gain the option to download the .tar directly and decompress it in transit. (Which doesn't affect the server at all, just the client's way of interpreting it.)


One key thing this change also adds is a short-circuit when accessing directories: these always return 404 via the API, although they'd try the cache anyway and go through that route, which was kind of slow. Adding in the additional encodings, it's going to try for .gz, .br, and .zst files in the worst case as well, which feels wrong. So, instead, it just always falls back to the index-check behaviour if the path ends in a slash or is empty. (Which is implicitly just a slash.)


For testing, I set up this repo: https://codeberg.org/clarfonthey/testrepo

I ended up realising that LFS wasn't supported by default with just dev, so, it ended up working until I made sure the files on the repo didn't use LFS.

Assuming you've run just dev, you can go directly to this page in the browser here: https://clarfonthey.localhost.mock.directory:4430/testrepo/
And also you can try a few cURL commands:

curl https://clarfonthey.localhost.mock.directory:4430/testrepo/ --verbose --insecure
curl -H 'Accept-Encoding: gz' https://clarfonthey.localhost.mock.directory:4430/testrepo/ --verbose --insecure | gunzip -
curl -H 'Accept-Encoding: br' https://clarfonthey.localhost.mock.directory:4430/testrepo/ --verbose --insecure | brotli --decompress -
curl -H 'Accept-Encoding: zst' https://clarfonthey.localhost.mock.directory:4430/testrepo/ --verbose --insecure | zstd --decompress -
This provides an option for #223 without fully resolving it. (I think.) Essentially, it acts very similar to the `gzip_static` and similar options for nginx, where it will check for the existence of pre-compressed files and serve those instead if the client allows it. I couldn't find a pre-existing way to actually parse the Accept-Encoding header properly (admittedly didn't look very hard) and just implemented one on my own that should be fine. This should hopefully not have the same DOS vulnerabilities as #302, since it relies on the existing caching system. Compressed versions of files will be cached just like any other files, and that includes cache for missing files as well. The compressed files will also be accessible directly, and this won't automatically decompress them. So, if you have a `tar.gz` file that you access directly, it will still be downloaded as the gzipped version, although you will now gain the option to download the `.tar` directly and decompress it in transit. (Which doesn't affect the server at all, just the client's way of interpreting it.) ---- One key thing this change also adds is a short-circuit when accessing directories: these always return 404 via the API, although they'd try the cache anyway and go through that route, which was kind of slow. Adding in the additional encodings, it's going to try for .gz, .br, and .zst files in the worst case as well, which feels wrong. So, instead, it just always falls back to the index-check behaviour if the path ends in a slash or is empty. (Which is implicitly just a slash.) ---- For testing, I set up this repo: https://codeberg.org/clarfonthey/testrepo I ended up realising that LFS wasn't supported by default with `just dev`, so, it ended up working until I made sure the files on the repo *didn't* use LFS. Assuming you've run `just dev`, you can go directly to this page in the browser here: https://clarfonthey.localhost.mock.directory:4430/testrepo/ And also you can try a few cURL commands: ```shell curl https://clarfonthey.localhost.mock.directory:4430/testrepo/ --verbose --insecure curl -H 'Accept-Encoding: gz' https://clarfonthey.localhost.mock.directory:4430/testrepo/ --verbose --insecure | gunzip - curl -H 'Accept-Encoding: br' https://clarfonthey.localhost.mock.directory:4430/testrepo/ --verbose --insecure | brotli --decompress - curl -H 'Accept-Encoding: zst' https://clarfonthey.localhost.mock.directory:4430/testrepo/ --verbose --insecure | zstd --decompress - ```
ltdk force-pushed accept-encoding from 37db7a06ed
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline failed
to da770cfb50
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline failed
2024年09月09日 05:41:55 +02:00
Compare
ltdk force-pushed accept-encoding from da770cfb50
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline failed
to 036d5b32f1
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
2024年09月09日 05:43:14 +02:00
Compare
ltdk force-pushed accept-encoding from 036d5b32f1
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
to 56969c10e8
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
2024年09月11日 03:43:28 +02:00
Compare
ltdk force-pushed accept-encoding from 56969c10e8
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
to baa096c4f4
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
2024年09月11日 03:57:18 +02:00
Compare
ltdk force-pushed accept-encoding from 4ceaf6b724
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
to 87defdc2da
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
2024年09月11日 04:26:58 +02:00
Compare
ltdk force-pushed accept-encoding from 87defdc2da
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
to 59af7f0e6a
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
2024年09月11日 04:54:08 +02:00
Compare
ltdk force-pushed accept-encoding from 59af7f0e6a
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
to a04d8721db
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline failed
2024年09月11日 05:31:31 +02:00
Compare
ltdk force-pushed accept-encoding from a04d8721db
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline failed
to abbb29df55
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
2024年09月11日 05:38:23 +02:00
Compare
Gusted left a comment
Owner
Copy link

Tested it locally and it works flawlessly.

Tested it locally and it works flawlessly.
@ -313,0 +323,4 @@
fallthrough
case".br":
fallthrough
case".zst":
Owner
Copy link

Can be simplified to:

case".gz",".br",".zst":
Can be simplified to: ```go case ".gz", ".br", ".zst": ```
Author
Contributor
Copy link

Didn't know that was an option, so, I'll do that.

Didn't know that was an option, so, I'll do that.
Gusted marked this conversation as resolved
@ -313,0 +324,4 @@
case".br":
fallthrough
case".zst":
innerExt=path.Ext(resource[:len(resource)-len(rawExt)])
Owner
Copy link

Might be nicer to use strings.TrimSuffix instead of doing slice operations. No hard preference in this trivial case.

Might be nicer to use [`strings.TrimSuffix`](https://pkg.go.dev/strings#TrimSuffix) instead of doing slice operations. No hard preference in this trivial case.
Author
Contributor
Copy link

Choosing between the multi-case statement or having a dedicated TrimSuffix per case, I'll just go with the multi-case version.

Choosing between the multi-case statement or having a dedicated `TrimSuffix` per case, I'll just go with the multi-case version.
Gusted marked this conversation as resolved
@ -55,0 +73,4 @@
qualities:=make(map[string]float64)
for_,encoding:=rangestrings.Split(header,","){
splits:=strings.SplitN(encoding,";q=",2)
Owner
Copy link

I'm not sure, but it seems like the code can be made a bit nicer by using strings.Cut

I'm not sure, but it seems like the code can be made a bit nicer by using [`strings.Cut`](https://pkg.go.dev/strings#Cut)
Author
Contributor
Copy link

I thinks so!

I thinks so!
Gusted marked this conversation as resolved
@ -55,0 +120,4 @@
// sort in reverse order; big quality comes first
returncmp.Compare(qualities[y],qualities[x])
})
log.Trace().Msgf("decided encoding order: %#v",encodings)
Owner
Copy link

Nit: Its type is []string, using %v is good enough.

Nit: Its type is `[]string`, using `%v` is good enough.
Author
Contributor
Copy link

So, I had initially switched it over because I made a mistake trimming spaces, and wanted to see the quotes in there. I can remove this now, though.

So, I had initially switched it over because I made a mistake trimming spaces, and wanted to see the quotes in there. I can remove this now, though.
Gusted marked this conversation as resolved
Author
Contributor
Copy link

I've made the suggested fixes, hopefully it looks a little nicer now.

I've made the suggested fixes, hopefully it looks a little nicer now.
ltdk force-pushed accept-encoding from 6a959a3909
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline failed
to dc0d7aae56
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
2024年09月28日 07:59:09 +02:00
Compare
ltdk force-pushed accept-encoding from dc0d7aae56
Some checks failed
ci/woodpecker/pr/lint Pipeline failed
ci/woodpecker/pr/build Pipeline was successful
to d0c07b775e 2024年09月28日 08:04:07 +02:00
Compare
Gusted left a comment
Owner
Copy link

Works in practice. Code looks good. I'll leave it a few days for anyone else to review it - otherwise I'll merge it.

Works in practice. Code looks good. I'll leave it a few days for anyone else to review it - otherwise I'll merge it.
Author
Contributor
Copy link

I mean, this has been sitting around for a long time, so unless you're anticipating someone to review it, you probably won't get anyone else to review it.

I mean, this has been sitting around for a long time, so unless you're anticipating someone to review it, you probably won't get anyone else to review it.
crapStone force-pushed accept-encoding from d0c07b775e to 2f4beb57ba
All checks were successful
ci/woodpecker/pr/lint Pipeline was successful
ci/woodpecker/pr/build Pipeline was successful
2024年09月29日 22:50:40 +02:00
Compare

Thanks for your contribution :)

Thanks for your contribution :)
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
Codeberg/pages-server!387
Reference in a new issue
Codeberg/pages-server
No description provided.
Delete branch ":accept-encoding"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?