[PATCH] Allowed nesting arbitrary prefix "location" in regex "location"

[Maxim Dounin]

Hello!

On Mon, Sep 04, 2023 at 10:11:47PM +0300, Valentin V. Bartenev wrote:

> # HG changeset patch
> # User Valentin Bartenev <vbart at wbsrv.ru>
> # Date 1693854233 -10800
> #      Mon Sep 04 22:03:53 2023 +0300
> # Node ID c706913db63c6862c13a0a540cdc37be0ccf0c81
> # Parent  daf8f5ba23d8e9955b22782d945f9c065f4b6baa
> Allowed nesting arbitrary prefix "location" in regex "location".
> 
> Previously, only prefix "location" blocks that literally matched the beginning
> of the regular expression were allowed inside.  This restriction makes no sense
> because regular expressions have different matching semantics.
> 
> diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
> --- a/src/http/ngx_http_core_module.c
> +++ b/src/http/ngx_http_core_module.c
> @@ -3202,6 +3202,7 @@ ngx_http_core_location(ngx_conf_t *cf, n
>  
>  #if (NGX_PCRE)
>          if (clcf->regex == NULL
> +            && pclcf->regex == NULL
>              && ngx_filename_cmp(clcf->name.data, pclcf->name.data, len) != 0)
>  #else
>          if (ngx_filename_cmp(clcf->name.data, pclcf->name.data, len) != 0)

Thank you for your patch.

Locations given by regular expressions are expected to be used as 
leaf locations to match a small subset of the requests.  Further, 
to make sure the configuration can be read and modified it is 
recommended to isolate regular expressions inside prefix 
locations, see here:

https://www.youtube.com/watch?v=YWRYbLKsS0I&t=630

The condition in question is indeed somewhat misleading, but the 
practical effect is that it prevents usage of prefix locations 
within regex locations, with the exception of regex locations 
misused as prefix ones, such as in:

    location ~ /foo/ {
        location /foo/bar {
            ...
        }
    }

While trying to create arbitrary prefix locations within a regex 
location might work, it will certainly complicate things a lot, 
making reading and modifying such configurations much harder.  
Consider:

    location / {
        location ~ \.php$ { ... }
    }

    location /foo/ {
        location ~ \.php$ { ... }
    }

    location ~ \.php$ {
        location /foo/ { ... }
    }

Which location is to be used for "/foo/file.php" request?  Within 
the existing logic, the answer is more or less obvious: the most 
specific configuration wins, so "location /foo/ { location ~ 
\.php$ { ... }}" will be used.

But with the "location ~ \.php$ { location /foo/ { ... }}", as 
allowed with the patch in question, things become more 
complicated.  Both "location /foo/ { location ~ \.php$ { ... }}" 
and "location ~ \.php$ { location /foo/ { ... }}" are equivalent, 
and there is no obvious answer.

Given the above, I tend to think that it would be better to 
preserve existing behaviour.

-- 
Maxim Dounin
http://mdounin.ru/