[PATCH] Limit req, Limit conn: The "off" parameter of the 'limit_conn' and 'limit_req' directives

[Maxim Dounin]

Hello!

On Mon, Feb 08, 2016 at 10:13:48AM +0600, Pavel V. wrote:

[...]

> > This variant of the code doesn't check "off" if additional 
> > arguments are used.  Limit req's version however does (and 
> > complains if number of arguments is incorrect).  Reasons for the 
> > difference are not clear.
> 
> These directives has different syntax at the moment. Directive 'limit_conn'
> currently allows exactly two arguments and someone can configure Nginx to use
> zone 'off' by a line
> 
>    ...
>    limit_conn off 42;
>    ...
> 
> The 'limit_req' directive expects prefix 'zone=' and there is no way to use
> keyword 'off' as a standalone argument.
> 
> So, the reason for the difference is backward compatibility with existing
> configurations. Should we care about this or not?

I don't think that supporting "off" as a name of a zone worth the 
effort.  Moreover, it's likely to confuse users if supported.

[...]

> > This code silently allows "limit_conn off" to be used multiple 
> > times, as well as with other limit_conn directives.  This leads to 
> > a situation where:
> 
> >    limit_conn off;
> >    limit_conn foo 10;
> 
> > will not use "limit_conn foo 10" configured without any 
> > indication, which looks incorrect and confusing.  It should either 
> > complain, or use limits configured.
> 
> I used http_log_module as a guide for such behaviour. It has directive
> "access_log" with similar syntax "access_log off;" and it does not complains
> when other directives specified with 'access_log off'. Why this should be
> handled in the other way here?

Because "access_log off" behaviour is confusing, too?  General 
approach in nginx configs is to don't ignore/redefine somethin 
defined at the current level, but to complain instead.

I don't suggest to change current "access_log off" behaviour as it 
is likely to cause compatibility problems with existing 
configurations (or at least we have to introduce warnings first), 
but we probably don't want to introduce additional places with 
such confusing behaviour.

-- 
Maxim Dounin
http://nginx.org/