jetzig-framework / jetzig

Jetzig is a web framework written in Zig
MIT License
308 stars 15 forks source link

Add CompressionMiddleware #80

Closed Froxcey closed 3 weeks ago

Froxcey commented 4 weeks ago

This PR introduces a gzip middleware. This middleware compresses every response to decrease document size.

To use, add jetzig.middleware.GzipMiddleware to main.zig

Btw I don't have a lot of hours in zig yet, so please pick my code quality and ask for changes if necessary.

vibe
Froxcey commented 4 weeks ago

btw the minifier middleware that I demonstrated on Discord is not part of this PR because that depends on an external JavaScript library, which goes against Jetzig's requirement of having everything zig only. I will look into porting that to native zig, but no promises.

fvdb commented 4 weeks ago

Wouldn't this also have to check the Accept-Encoding request header to see if Gzip is something the client accepts, and then if so, also add a Vary: Accept-Encoding response header to ensure caching will work properly as it then varies the response based on that request header?

https://datatracker.ietf.org/doc/html/rfc9110#field.accept-encoding

fvdb commented 4 weeks ago

Also Zig's deflate seems to be an implementation of RFC 1951 which should be using the deflate content encoding and not gzip. Term Gzip for files is different than for HTTP. This SO answer outlines the confusion behind this pretty well:

https://stackoverflow.com/questions/883841/why-do-real-world-servers-prefer-gzip-over-deflate-encoding#1579506

Froxcey commented 4 weeks ago

Yeah silly me, I didn't think about that. The middleware now supports no encoding, gzip, and deflate. Btw, std.compress.gzip in the std is in fact gzip, and I tested it by setting the header to "deflate" which made the website not load. std.compress.flate is deflate, and I just added support for that. Br and Zstd writing is not available in zig std, so I'm ignoring those encoding for now.

Froxcey commented 4 weeks ago

Also, please help me with my code quality. I think it looks kinda bad but idk how to fix it.

fvdb commented 3 weeks ago

Great!

As for quality my Zig-fu isn't that strong just yet. I would suggest extracting the encoding selection into a separate function.

A future iteration might also take into account preference indicated by the browser E.g. if they indicate a higher preference for deflate than gzip, it should use that.

The current implementation ignores that and would not compress anything if a client would for instance send Accept-Encoding: gzip;q=1.0,deflate;q=0.5,*;q=0.

But perhaps that is out of scope. It is not often used by browsers in the specific Accept-Encoding header and I would imagine that kind of logic to be better placed in some generic accept-* header parsing functionality so it would also be easy to determine preferred content type or language, which uses the same format.

fvdb commented 3 weeks ago

Also perhaps some tests :)

bobf commented 3 weeks ago

@Froxcey Thanks for the PR. I added Headers.getAllIterator() and am now using that to fetch Accept-Encoding headers to avoid some allocs + some other refactors.

I think this is ready to merge now.

We can add tests using new testing framework once I have merged that branch, it is nearly complete.