Precompressed files #156

Nehliin · 2021-11-11T11:29:44Z

Motivation

Suggestion for an implementation that should fix #120.

Solution

The user specifies which types of precompression is available in the same directory as the original file by specifying precompressed_[gzip|br|deflate] to the ServeFile service. It is not required to have the compression feature enabled since no compression/decompression actually happens on the server. The ServeFile service looks for copies of the original file with the extension .gz, .bror .zz depending on the compression used/specified. If a client requests the file and doesn't support any of the precompressed variants the "normal" file will be served instead. This means it's expected that the uncompressed version of the file also exists if all clients should be able to request the file. Otherwise a 404 will be returned if the client doesn't support the compression algorithm.

Nehliin · 2021-11-12T09:57:24Z

Cleaned up a bit and added more tests and support for ServeDir so I think its ready for review now

davidpdrsn

Great work! I think it looks good as it.

I'll put this on 0.2 milestone.

davidpdrsn · 2021-11-12T13:41:15Z

tower-http/src/services/fs/serve_file.rs

@@ -74,7 +117,7 @@ impl ServeFile {
    }
 }

-impl<R> Service<R> for ServeFile {
+impl<ReqBody> Service<Request<ReqBody>> for ServeFile {


Its a shame that this required a breaking change. I think for 0.2 we should audit all services and make sure they implement Service<Request<B>> so we can make changes like this in the future.

tower-http/src/services/fs/serve_dir.rs

Nehliin · 2021-11-12T15:07:56Z

Thanks! I'll fix the CI as well either today or tomorrow

davidpdrsn · 2021-11-12T15:12:40Z

There were some general CI issues which I'm fixing in #158

Nehliin · 2021-11-12T15:16:17Z

Ah great, yeah the the cargo hack issue should have been there before my pr as well unless I'm missing something 🤔

shepmaster · 2021-11-12T19:06:20Z

tower-http/src/services/fs/serve_dir.rs

+    async fn only_precompressed_variant_existing() {
+        let svc = ServeDir::new("../test-files").precompressed_gzip();
+
+        let request = Request::builder().body(Body::empty()).unwrap();


Should this specify the .uri to match?

shepmaster · 2021-11-12T19:07:44Z

tower-http/src/content_encoding.rs

+        }
+    }
+
+    pub(crate) fn to_file_extention(self) -> Option<&'static OsStr> {


typo: "extension"

shepmaster · 2021-11-12T19:14:35Z

tower-http/src/services/fs/serve_dir.rs

+    /// Informs the service that it should look for a precompressed (by the gzip algorithm)
+    /// version of _any_ file in the directory with the correct file extension  that can be
+    /// served if the client have the correct corresponding `Accept-Encoding`.
+    /// (i.e `dir/foo.txt.gz` when the served directory is `dir` and the requested file is `foo.txt`)
+    /// If not the uncompressed version will be served instead. Note that this means
+    /// that both the precompressed version(s) and the uncompressed file is expected
+    /// to be present in the same directory.


Suggested change

/// Informs the service that it should look for a precompressed (by the gzip algorithm)

/// version of _any_ file in the directory with the correct file extension that can be

/// served if the client have the correct corresponding `Accept-Encoding`.

/// (i.e `dir/foo.txt.gz` when the served directory is `dir` and the requested file is `foo.txt`)

/// If not the uncompressed version will be served instead. Note that this means

/// that both the precompressed version(s) and the uncompressed file is expected

/// to be present in the same directory.

/// Informs the service that it should look for a precompressed gzip

/// version of _any_ file in the directory.

///

/// If the client has an `Accept-Encoding` header that allows the gzip encoding,

/// the file `dir/foo.txt.gz` will be served instead of `dir/foo.txt`.

/// If the precompressed file is not available, or the client doesn't support it,

/// the uncompressed version will be served instead.

/// Both the precompressed version and the uncompressed version are expected

/// to be present in the directory.

(ditto similar suggestions to the sibling methods)

I'm not sure where it should be mentioned about the q settings for browser preference.

fixup! Support precompressed files

davidpdrsn · 2021-11-15T10:43:05Z

tower-http/CHANGELOG.md

@@ -7,7 +7,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

 # Unreleased

- None.
+- `ServeDir` and `ServeFile`: Ability to serve precompressed files ([#156])


I think its missing a link to [#156] 🤔

Huh? Isn't that the last part ([#156]) or do I need to do something else as well?

You need to add link for it like its done for the other changelog entries. GH doesn't pick it up automatically unfortunately.

It'll look like this otherwise:

ah right completely missed that, fixed now!

c5soft · 2021-11-17T02:38:34Z

Hi there, I'm wondering if it is a design problem that ServeDir/File will fail when precompressed_gzip enabled and path/file.ext.gz not exists but path/file.ext exists. this should works without content-encoding.
In my opinion, negotiated_encoding should be determined by: 1. precompressed_variants 2.client Accept-Encoding, 3.precompressed file exists. Any one of three conditions is not statisfied will cause uncompressed file used. Multiple prcompressed type co-statisfied, we my consider br first, then gzip, the third deflate.

Nehliin · 2021-11-17T08:01:27Z

Hi @c5soft if the developer has explicitly indicated that there should exist an precompressed variant I think its fair to return 404 to clients that supports the encoding (all browsers) since it would be an error on the side of the developer and it makes it easy to notice after a basic check. Otherwise the developer might think it serves precompressed files without them actually existing.

Another option would be to remove the precompressed_[gzip|br|deflate| and automatically always look for precompressed variants and fallback if they don't exist. However that wouldn't be zero cost for developers who know they don't have precompressed files. We would have to do additional system calls and checks to look for the precompressed variants in almost all cases since again, all large browsers supports the content encodings.

davidpdrsn · 2021-11-17T08:17:23Z

if the developer has explicitly indicated that there should exist an precompressed variant I think its fair to return 404

I didn't catch that about this implementation. Not sure thats ideal since ServeDir serves any file in the directory and all subdirectories. So you'd have to precompress all files even if you're only really interested in precompressing a single file. You might also be serving images which don't benefit much from compression afaik.

Nehliin · 2021-11-17T09:19:23Z

@davidpdrsn Yeah that's fair, I didn't think about the ServeDir case becoming quite inconvenient to work with if you have images etc. I can create a PR to fallback if it doesn't exist. I presume we want to change the ServeFile service then as well to keep it consistent?

davidpdrsn · 2021-11-17T09:24:04Z

That would be great 😊 and yeah adding it to ServeFile as well would be nice.

davidpdrsn added this to the 0.2.0 milestone Nov 11, 2021

Nehliin force-pushed the precompressed-files branch 2 times, most recently from 7d2692b to b5bb0a3 Compare November 12, 2021 09:56

Nehliin marked this pull request as ready for review November 12, 2021 09:56

Nehliin mentioned this pull request Nov 12, 2021

Don't return file content for HEAD requests in ServeFile, ServeDir #113

Closed

davidpdrsn approved these changes Nov 12, 2021

View reviewed changes

davidpdrsn added the breaking change A PR that makes a breaking change. label Nov 12, 2021

davidpdrsn mentioned this pull request Nov 12, 2021

Support side-by-side precompressed static files tokio-rs/axum#66

Closed

shepmaster reviewed Nov 12, 2021

View reviewed changes

Nehliin force-pushed the precompressed-files branch 2 times, most recently from 4e71029 to 7ab662d Compare November 15, 2021 08:37

davidpdrsn mentioned this pull request Nov 15, 2021

Make sure all middleware implement Service<Request<ReqBody>> and require Response = Response<ResBody> #164

Closed

davidpdrsn approved these changes Nov 15, 2021

View reviewed changes

Nehliin added 6 commits November 15, 2021 11:33

Split out content encoding enum

28d321e

Support precompressed files

c5ffc9b

fixup! Support precompressed files

Add some more tests

aa7129a

Support precompressed directory files

3e58ae3

Fix pr comments and improve docs

62f2030

Fix cargo hack issue

1ced4af

Nehliin force-pushed the precompressed-files branch from 7ab662d to fbf1147 Compare November 15, 2021 10:35

davidpdrsn reviewed Nov 15, 2021

View reviewed changes

davidpdrsn mentioned this pull request Nov 15, 2021

Switch from pin-project to pin-project-lite #151

Merged

Update changelog

9e58410

Nehliin force-pushed the precompressed-files branch from fbf1147 to 9e58410 Compare November 15, 2021 10:55

davidpdrsn enabled auto-merge (squash) November 15, 2021 10:59

Merge branch 'master' into precompressed-files

807ac48

davidpdrsn disabled auto-merge November 15, 2021 11:04

davidpdrsn merged commit 754aafe into tower-rs:master Nov 15, 2021

Nehliin deleted the precompressed-files branch November 15, 2021 11:05

Nehliin mentioned this pull request Nov 16, 2021

Redo PR:ServeDir: Add optional cache control feature: max-age . Based on 0.1.2 #168

Closed

davidpdrsn mentioned this pull request Dec 1, 2021

Release 0.2.0 #193

Merged

palant mentioned this pull request Feb 20, 2024

Allow for optionally serving pre-compressed files. rwf2/Rocket#2716

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Precompressed files #156

Precompressed files #156

Nehliin commented Nov 11, 2021 •

edited

Loading

Nehliin commented Nov 12, 2021

davidpdrsn left a comment

davidpdrsn Nov 12, 2021

Nehliin commented Nov 12, 2021

davidpdrsn commented Nov 12, 2021

Nehliin commented Nov 12, 2021

shepmaster Nov 12, 2021

shepmaster Nov 12, 2021

shepmaster Nov 12, 2021

davidpdrsn Nov 15, 2021

Nehliin Nov 15, 2021

davidpdrsn Nov 15, 2021

Nehliin Nov 15, 2021

c5soft commented Nov 17, 2021

Nehliin commented Nov 17, 2021

davidpdrsn commented Nov 17, 2021

Nehliin commented Nov 17, 2021

davidpdrsn commented Nov 17, 2021

-    /// Informs the service that it should look for a precompressed (by the gzip algorithm)
-    /// version of _any_ file in the directory with the correct file extension  that can be
-    /// served if the client have the correct corresponding `Accept-Encoding`.
-    /// (i.e `dir/foo.txt.gz` when the served directory is `dir` and the requested file is `foo.txt`)
-    /// If not the uncompressed version will be served instead. Note that this means
-    /// that both the precompressed version(s) and the uncompressed file is expected
-    /// to be present in the same directory.
+    /// Informs the service that it should look for a precompressed gzip
+    /// version of _any_ file in the directory.
+    ///
+    /// If the client has an `Accept-Encoding` header that allows the gzip encoding,
+    /// the file `dir/foo.txt.gz` will be served instead of `dir/foo.txt`.
+    /// If the precompressed file is not available, or the client doesn't support it,
+    /// the uncompressed version will be served instead.
+    /// Both the precompressed version and the uncompressed version are expected
+    /// to be present in the directory.

Precompressed files #156

Precompressed files #156

Conversation

Nehliin commented Nov 11, 2021 • edited Loading

Motivation

Solution

Nehliin commented Nov 12, 2021

davidpdrsn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Nehliin commented Nov 12, 2021

davidpdrsn commented Nov 12, 2021

Nehliin commented Nov 12, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

c5soft commented Nov 17, 2021

Nehliin commented Nov 17, 2021

davidpdrsn commented Nov 17, 2021

Nehliin commented Nov 17, 2021

davidpdrsn commented Nov 17, 2021

Nehliin commented Nov 11, 2021 •

edited

Loading