-
Notifications
You must be signed in to change notification settings - Fork 245
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
Gzip support #20
Comments
You can't just gzip-compress files according to a regexp; the client has to present an appropriate Accept-Encoding: header. In my view this functionality belongs in Node.js itself. |
So if the client presents the Accept-Encoding: gzip header, then we should gzip what kind of file? All files? I'm not super familiar with how gzip works, but I'm guessing one might want to gzip all files or only a certain kind of file, etc. I don't think you're going to see gzip getting into node.js core anytime soon. Ry, Tim, et. al have deemed it 'user-land' for a while. |
If the client presents the Accept-Encoding: header, then the response can be compressed. The file type is irrelevant. I think it belongs in the core because the functionality should be uniform across all HTTP response handlers. I worry what might happen if the functionality is delegated to response handlers, many of which might do it improperly, and there may be a lot of duplicated code, all of which will have to be torn out when the functionality does make it into the core. |
Ok, but not every file should be gzipped was my point. Here's some background: "Servers choose what to gzip based on file type, but are typically too limited in what they decide to compress. Most web sites gzip their HTML documents. It's also worthwhile to gzip your scripts and stylesheets, but many web sites miss this opportunity. In fact, it's worthwhile to compress any text response including XML and JSON. Image and PDF files should not be gzipped because they are already compressed. Trying to gzip them not only wastes CPU but can potentially increase file sizes." |
Oh yes, quite right. I'd forgotten about that. You might want to take a look at Apache's mod_deflate for inspiration. |
BTW, nginx has a nice feature gzip_static. It means that it tries to push file <requested_file>.gz, if one exists. And it pushes the regular file otherwise. |
+1 to pavel's point about checking for a file with the added gz suffix. This is how they do in in Jetty's DefaultServlet. It is highly efficient because the server doesn't re-gzip things on the fly all the time. Instead, the user declares which static files he wants to send gzipped by gzipping them along with the uncompressed file. When done this way, it makes perfect sense to do it as part of the static file server because we're just deciding to serve up a different static file if the client presents the right header and we spot a '.gz' file ready for serving. Saves CPU time compared to an on-the-fly gzipping in the core of node and saves bandwidth, too! |
See #34 for a patch supplying an implementation along the lines of nginx's gzip_static. If you're serving any static files at all from your server, there's a good chance you'll want to be able to serve them compressed for clients that support that, and this is the most efficient way to do that (pre-compression rather than on the fly). |
dobesv, thank you. I planned to make the same patch, but now I can get yours. |
@pavel-koryagin If you used my earlier patch you may want to update from that repo again - I had introduced a bug where Content-Length was being sent with a 304 not modified response which throws Safari browsers for a loop. The new version fixes that bug. |
Oh interesting discussion about half a decade ago. I wonder why the state of this is still "gzip the files by hand" ;) It seems a bit cumbersome ;) |
Seems given the current implementation there is no way to put in a simple handler for gzip compression. This would be quite useful for speeding up some files being served.
I'm not saying put a full middleware stack in there for your static files, but just a single flag "gzip", which can optionally gzip files that match a given regexp.
Thoughts?
The text was updated successfully, but these errors were encountered: