Keep Alive piles up

[Maxim Dounin]

Hello!

On Mon, Jun 29, 2009 at 01:00:00PM -0400, meto wrote:

> 
> server {
>         listen   94.23.96.82:80;
>         server_name     gimpuj.info www.gimpuj.info
>                 gimpuj.org www.gimpuj.org
>                 gimpuj.net www.gimpuj.net;
> 
>                 if ($host != www.gimpuj.info)
>                 {
>                 rewrite ^(.*)$ http://www.gimpuj.info$1 permanent;
>                 }
> 
>         access_log  /var/www/vhosts/gimpuj.info/statistics/logs/access_log combined buffer=32k;
> 
>                 location = / {
>                         rewrite  / index.php?action=glowna permanent;
>                 }
> 
>                 location / {
>                         index index.html index.php;
>                         root   /var/www/vhosts/gimpuj.info/httpdocs;
>                         include php_support;
> 
>                                 if ( !-e $request_filename ) {
>                                 # rewrites
>                                 rewrite  ^/avatar/(.*)\.png /avatar/index.php?user=$1 last;
>                                 .... 
>                                 some rewrites
>                                 ....
>                                 rewrite  ^/index\.php/(.*) /index.php?$1;
>                                 # end rewrites
>                                 }

I was able to reproduce coredump with the following reduced 
config:

        location / {

            set $true 1;

            if ($true) {
                # fastcgi_pass here
                fastcgi_pass  127.0.0.1:9000;
            }

            if ($true) {
                # no handler here
            }

            # and no handler here
        }

It does happily coredumps on 0.7.59 though.  Just for completeness 
I've tested 0.7.0 - it coredumps too.  No idea why 0.7.59 works 
for you.

The problem is that first matching if() installs fastcgi handler 
for request, while actual processing takes place in context of 
last matching if() (second in the example) and there is no 
required fastcgi configs in this context.

Fix is to explicitly use break in if with fastcgi_pass to avoid 
further rewrite processing, i.e.:

        location / {

            set $true 1;

            if ($true) {
                # fastcgi_pass here
                fastcgi_pass  127.0.0.1:9000;

                # break to avoid coredump
                break;
            }

            if ($true) {
                # no handler here
            }

            # and no handler here
        }

It's not clear for me how to fix code without breaking 
more-or-less correct configs using if's, so probably it's a good 
idea to wait for Igor's return from vacation.

>                 }
> 
> 
>             # PLESK statistics below
>             location ^~  /plesk-stat/ {
>                     autoindex on;
>                     alias /var/www/vhosts/gimpuj.info/statistics/;
>             }
>             # End of PLESK statistics
> 
>         include settings;
> }
> 
> every domain is built with such config template
> 
> php_support
> 
> #php support
>         if ( $request_filename ~* \.php(.*)$ ) { # is it php?
>         fastcgi_pass    127.0.0.1:9000;
>         expires off;
>         }
> 
> fastcgi_index  index.php;
> fastcgi_param  SCRIPT_FILENAME  $document_root$fastcgi_script_name;
> include fastcgi_params;
> 
> 
> settings
> 
> #limit conns for ddos and slowloris
> limit_conn limit_per_ip 25;
> 
> #errors
> error_page 404 /errors_pages/404.html;
> 
> location ^~ /errors_pages {
>         alias /var/www/errors_pages;
> }
> 
> 
> #Phpmyadmin
> 
> location ^~ /phpmyadmin {
>         index index.php;
>         root /usr/share;
>         include php_support2;
> }
> 
> #xcache-admin
> 
> location ^~ /xcache-admin {
>         index index.php;
>         alias /usr/share/xcache/admin;
>         #root /usr/share/xcache/admin;
>         include php_support2;
>         fastcgi_param  SCRIPT_FILENAME  $request_filename;
> }
> # AW-Stat Icons
> location ^~ /awstats-icon/ {
>         alias /usr/share/awstats/icon/;
> }
> 
> # deny access to .htaccess files, if Apache's document root
> # concurs with nginx's one
> location ~ /\.ht {
>         deny  all;
> }
> 
> 
> php_support2 is same as php_support but passes queries only to local PHP.
> 
> Here you have it. As I said nothing special here.
> 
> >Using if() on $request_uri/$request_filename suggests that config
> >should be rewritten, but anyway nginx shouldn't coredump.
> 
> Why? I know that if is in rewrite module in docs but can't we use it in other places?

Basically, if()'s are evil.  Mostly due to the fact that nginx 
configuration are declarative while if's are imperative - and this 
produces some nasty implementation details as in your problem.

In most situations it's a good idea to avoid if's.  And test for 
$request_uri/$request_filename means you can do so using 
appropriate [regex] locations.

> Do you suggest to use nested location rather than if() in that case? But still (and dont yell) - it worked before.

Yes, separate normal locations or nested ones is the way to go.

Maxim Dounin