Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Static gzip #64

Closed
wants to merge 10 commits into from
Closed

Static gzip #64

wants to merge 10 commits into from

Conversation

bruce-one
Copy link

Bit of an RFC pull request...

This PR adds support for serving pregzipped files in response to requests; eg /index.html will be served the file /index.html.gz if the client accepts gzip responses, and the file exists. If the .gz file doesn't exist, then the original file is compressed and sent - as before.

The cache is filled with the gzipped file if found.

Some other static gzipping implementations check things like timestamps to try and ensure that the .gz file and the requested file are the same thing, however this implementation doesn't do that...

I'm not sure if this is the best implementation; but it felt the least invasive, and seems to work - hence I went with it :-p

Happy to tweak :-) For now I just wanted to make the PR to see if it's something that could be considered for inclusion :-) (Before then considering more polish etc)

(Ps, also happy to squash the commits or anything too :-) )

getPath is passed the pathname, so no need to parse it again to get the
pathname (again).
req.sturl is only set here, so there is no need for the if statement
because it's definitely not set yet.
A working, but possibly not architecturally clean, implementation of
serving statically gzipped files.

The implementation just looks for a file which has the extra `.gz`
extension and serves that if it exists, it doesn't check whether that's
actually a sane file to try.
To ensure that their is a statically gzipped file for all of the common
tests as well.
@rvagg
Copy link
Collaborator

rvagg commented Jun 29, 2015

While I totally agree with this and would love it in, I'm hesitant for the same reasons as with #56 in that it touches the fd caching algorithm which is already very complicated and ripe for leaks so it's a very difficult review.

One day, soon hopefully, I'll sit down and sort through it all and figure out a way forward, I'm just time-poor at the moment.

@bruce-one
Copy link
Author

No stress :-)

I actually tried to write this in such a way that it was as noninvasive as possible - which I actually think means it isn't very well integrated with the fd cache (it sort of just ignores it when looking for a gzipped file...)

I'd been sitting on the code for a while, so mostly made the PR to see if it's something that you felt appropriate to put in st :-) (And less about this implementation being correct.)

If I get a chance, I might similarly try and consider how it interacts with the fd cache :-)

The fd cache stuff did seem a bit scary when I first saw it ;-)

@rvagg
Copy link
Collaborator

rvagg commented Nov 11, 2019

closing this, it's 4 years old, very sorry for not staying on task. the code has changed a lot thanks to linting but if you'd like to do this then please open a new PR

@rvagg rvagg closed this Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants