Feature request: Pre-compressed static files

[mollymorphous]

Thanks so much for the static files feature; it simplifies deployment for my hobby project!

I think it would be nice to have support for pre-compressed static files in the static file server, like nginx's gzip_static directive.

When enabled, a request matching a static file with an appropriate Accept-Encoding header would also check for files matching the path with a .gz, .br, or .zst suffix and respond with the smallest matching compressed version and appropriate Content-Encoding and Vary headers.

Please let me know if you think this feature makes sense for Granian. I'm still new to Rust, but I'd be happy to make a PR if interested!

[gi0baro]

@mollymorphous I'd say it's a nice feature to add.

I won't say this would fit into 2.3 release – I'd like to have that out next week hopefully – but I added 2.4 as the ideal target.
So feel free to work on a PR, the static file code won't change anymore before the 2.3 release (excepts for bugs).

[gi0baro]

@mollymorphous fyi in the next few weeks I gonna start working on 2.4 (https://github.com/emmett-framework/granian/milestone/21).
If you have any status updates, feel free to share. It's also fine to move the target to a different milestone if you think it would require more time.

[mollymorphous]

I'm hoping to have some time to get to it towards the second half of this month. Maybe targeting 2.5 makes more sense?

[gi0baro]

I'm hoping to have some time to get to it towards the second half of this month. Maybe targeting 2.5 makes more sense?

Yeah no rush :) I'll move this to 2.5

[gi0baro]

@mollymorphous in the next weeks I'm going to work on 2.5 release. Do you have any status updates to share on this?

[mollymorphous]

Hi @gi0baro, I've got some time and I'm taking a look at this this week!

I noticed that the call to Path::canonicalize in match_static_file may block to resolve symlinks:

granian/src/files.rs

Line 21 in 0324e6c

match Path::new(&fpath).canonicalize() {

Is this an issue in the context it's called from? If so, I'm happy to file a new issue for refactoring it, or roll it into this work.

[gi0baro]

@mollymorphous good news!

In regards of

Is this an issue in the context it's called from?

we definitely want to keep that in place to prevent security attack vectors like /static/../../../{at some point root}. And at the same time, I'm not quite sure why supporting symlinks in the mounted static folder would be any good – as, again, it potentially introduce a lot of security exposure.

[mollymorphous]

Agreed, I wouldn't want a stray symlink to potentially expose content outside of the mount point. My concern was that Path::canonicalize would block the worker thread while it was checking the file system. I don't know enough about where the service function that calls match_static_file runs to be able to say if it is an issue or not.

[gi0baro]

My concern was that Path::canonicalize would block the worker thread while it was checking the file system.

You're definitely right here, good catch. I'm not also sure whether the amount of time it blocks the runtime actually matters.
Anyways, I guess we can make that portion of code async and wrap that call in a tokio blocking task. But feel free to keep that out of your work, as not exactly related. We can fix that separately/later on.