Enhancements to the nginx configuration #133
Conversation
…(adding .htm and .php)
…es around, breaking the intended apache log format compatibility)
… out instead, that way it will align them to CPU cache lines for greater performance
…ny workers as there are CPU cores
…eople that don't know what the options do
…s for 10 minutes rather than constantly re-negotiating
… with old browsers
…and blocking requests that lack the Host header
…and "block bad guys" features
…ormance on very busy production sites
…tty much 100% of sites; avoids filling error logs with 404 for commonly sought yet commonly missing files such as robots.txt, favicon.ico and so on
|
Nice work! this is awesome. the the rest looks good, though i'm not too sharp with nginx.. can someone else take a look? |
|
Interesting, I'd love to know what's up with the font/opentype since "font/*" is an invalid mime type. Maybe we should settle this with a battle, like real men. Anyway, yeah I'd love to know what's up with this and if there's some good browser-quirk reason to use the invalid media type. |
|
quick blame search turned up i thought we had more detail than that though. ah i see the recent h5bp/html5-boilerplate#1317 see the old h5bp/html5-boilerplate#273 On Sun, Feb 24, 2013 at 2:42 PM, aitte notifications@github.com wrote:
|
|
Ahh, anyone with Chrome out there that can verify that Chrome understands application/x-font-opentype? It should, because it follows the law of "thou shalt not invent invalid mime types" and perfectly describes that it's a font and that it's of the OpenType sexuality. application/font-woff (registered) Now if only those darn ttf and otf folks could get their types registered, that'd be nice. |
|
chrome wont have a problem with any of them, On Sun, Feb 24, 2013 at 3:07 PM, aitte notifications@github.com wrote:
|
| @@ -0,0 +1,29 @@ | |||
| # Looks for common abusive programs and sets $is_crawler to 1 if found. | |||
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
|
To me the Nginx part is ok. The quality of comments also improved, which is a great thing. |
nginx only supports matching a single location-block before it stops execution, so the rule would have to go into the location-block that passes .php files over to FastCGI, and by that point it's too much of a hassle. modern browsers know not to cache .php files anyway, so let's simply remove the file extension.
|
@amenadiel: I know that you don't need server_name "" because it's the new default, but it's better to be explicit about it in case someone runs an older version, and even on the new version it aids in helping people understand what the server-block is matching (in this case, the empty "" name catches empty/missing "Host", and the default_server catches invalid hosts). As for the bad-guys list, I agree that it'd be a nice feature to add to the other web servers too if someone wants to port it. However, I do not recommend the 5G blacklist that you linked to. Mine is the result of a lot of experience with what user-agents are out there, including only the absolutely most common ones, in order to be as light-weight as possible (remember that this runs on every HTTP request). The 5G blacklist is a crude list of various query-URLs that people might be trying when they are trying SQL injection attacks and such, and scanning for that is not the job of the web server. There are much more efficient mods like mod_security to deal with that (and that module also exists for nginx). Or the best protection of all: Common sense; don't write web applications with SQL injection vulnerabilities - sanitize user input. No heavy mods/scanners/filters needed! ;-) If the bad-guys feature was extended to other web servers, then I suggest maintaining my approach, which is: Less is more. Aim for the big guys, to avoid slowing down the server with a massive list of 1200 user agents (the same author that created the "5G blacklist" proudly has a list of 1200 bad user agents, most of which come from the 90s and are never seen these days, heh). The other part of my approach is acknowledging the fact that trying to block all bad tools is futile. The really good ones all fake their user-agent. Instead, we shoot for the script kiddies - and the rules are designed to catch the vast majority of newbies while still being extremely light weight. We cannot stop a determined attacker from running their request with a spoofed agent, but by aiming for the most popular tools, we cut out a very good chunk without even hurting web server performance. And above all: The list is designed never to give any false positives. I'd rather let a few less common abuse tools through, than block a single legitimate visitor or search engine robot by having overly broad rules. |
|
By the way, anyone with Chrome that could check if application/x-font-opentype makes it log a warning in the error console? I don't use Chrome, nor do I want it on my system. ;-) |
@paulirish It still does. @aitte thanks for the pull request! (just one recommendation: in the future try to separate related commits into separate pull requests so that the comments don't get intermingled and there easier to review / pull / etc.)
@vendruscolo thanks for the review! (Cc: @AD7six) I don't have much experience with Nginx so, the only thing I can add to the discussion is that: we'll stick to |
|
@alrra: Thanks a lot for testing. Did you check "application/x-font-opentype"? Because your linked comment says that you tested "application/x-font-otf"? Just making sure. Edit: I installed Chrome in a VM and can confirm the behavior: "Resource interpreted as Font but transferred with MIME type application/x-font-opentype: "http://10.0.1.2/Delicious-Roman.otf"". Hmm, Chrome IS doing the wrong thing here, font/* is not a valid mime type (http://www.iana.org/assignments/media-types), and Chrome understands application/x-font-ttf and application/font-woff; it's just application/x-font-opentype (and otf) that it barfs on, but I guess we should change this for Nginx too. One moment, I'll make a commit. As for intermingled comments with multiple commits, you just have to click on "Commits" at the top of this page, then click on the commit. You can leave comments on a per-commit basis there. You can even click a particular line and comment on a line-level. |
…per type" This reverts commit 1e79dde.
Yes. (tested for multiple MIME types and at first I accidentally uploaded the wrong screenshot, sorry about that)
Yes I know but, even with line / commit comments the discussion is still intermingled as, for a pull request about "Enhancements to the nginx configuration", we're discussing commits / lines in Apache (I hope you understand what I mean). |
|
The opentype issue affected Nginx too in the old configs, and this discussion ended up leading to a fix in the Nginx configs (along with reverting the htaccess changes): Now all servers are brought in line with each other. I greped all the other configs to find ones that still use the incorrect values (or none at all) and will be pushing some more changes to fix those. |
|
I have to leave planet Earth pretty shortly just so you know that I can't stick around here forever. ;-) |
|
Okay I just finished re-reading all the changes, and basically, everything that's been brought up has been changed, and I even finalized the font mime type fixes for IIS, Nginx and Lighty, bringing every server in line with each other. Everything that's in here is in production use and has also been reviewed by several people now. If there's something you don't like, there's the "git cherry-pick" command to selectively merge all but a few of the patches, and I've given detailed motivations for the existence of every feature. I seriously gotta leave Earth in 4 hours, so goodbye and have fun with the merge. :-) Signing out. |
| location = /robots.txt { log_not_found off; } | ||
| location = /favicon.ico { log_not_found off; } | ||
| location = /apple-touch-icon.png { log_not_found off; } | ||
| location = /apple-touch-icon-precomposed.png { log_not_found off; } |
This comment was marked as abuse.
This comment was marked as abuse.
Sorry, something went wrong.
Personally, I don't think we should encourage that, it's important to know what generates 404 errors. |
|
I've already cherry-picked all the commits I intend to take from this PR, of those not taken: Not desired
Debatable
If these are desirable - they should be implemented for all servers (or at least, apache first) |
No, it should not be merged see: h5bp/html5-boilerplate#1317 (apache, gae, iis, lighttpd, nginx, and node, are all ok from this point of view). |
|
@alrra - cool, that leaves only the two proposed block features. I don't think these blocks are robust/appropriate (e.g. PHP is identified as a generic baddie - but if you use PHP without modifying the user agent string you are more likely not a baddie and e.g. making an api request to a server). These blocks are more likely to introduce maintenance overhead (or simply be unused/forgotten) than to bring benefit to the repo as a whole. As such that's everything that's to be taken from this PR. |
Implementations for most of the open issues in the issue tracker. These changes are all in use on various production servers that I administer and have been very well tested for months. I also added a few new features that we are using on our servers, which I am sure will come in very handy for most people. Code review should be simple since I've limited things to one feature per commit. Enjoy!