[PATCH] ngx_conf_file: "include ./" acts relative to currently parsed file

[Maxim Dounin]

Hello!

On Fri, Aug 30, 2019 at 04:28:00PM +0200, Guillaume Outters wrote:

> Le 2019-08-30 16:02, Maxim Dounin a écrit :
> 
> > Changing this to resolve relative paths from the current included
> > file instead is possible, but would be a major change - I suspect
> > it will break a lot of configurations.  Not sure we are going to
> > do this.
> > 
> > On Thu, Aug 29, 2019 at 08:04:41AM +0200, Guillaume Outters wrote:
> > 
> >> The following patch adds a simple heuristic to include: if the
> >> includee starts with "./", it is considered relative to the
> >> current file. If not, the current heuristic applies […]
> > 
> > Certainly I'm against this approach, as it breaks POLA.  The
> > "include ./example.conf;" construct shouldn't be handled
> > differently from "include example.conf", these are clearly the
> > same thing.
> 
> Hmm, you're right. Perhaps an unused diacritical would be better there, 
> as:
>    include @example.conf;
> or an explicit flag:
>    include example.conf local;
>    include example.conf -l; # Hmm, maybe too Apache-styled?
>    include example.conf relative;
>    include example.conf from_there;
> This would be even better than the diacritical: prevents crash of 
> existing configurations who use strangely- at -prefixed names, more 
> explicit, and more respectful of nginx' config principles (optional 
> flags go at the end).
> On the other hand, this is longer to type, would make config reading 
> more prone to missing the keyword (from my point of view, as I tend to 
> read left-to-right and lazily stop when I have gathered what I was 
> looking for) and… looks more complex to implement (this patch was my 
> first peak ever at nginx' source).
> But even to me, the "pros" seems to overthrow the "cons".
> 
> I would be glad to know what you think of this last solution (and which 
> keyword you would choose then).

I can't say I like either of the variants.

Additionally, all variants with an additional explicit flags won't 
work in other cases where a configuration prefix is currently 
used, such as ssl_certificate or auth_basic_user_file.  On the 
other hand, obviously enough it should be possible to resolve from 
the current included file all paths which are currently resolved 
from the configuration prefix.

(Further, for things like auth_basic_user_file, which is evaluated 
at runtime, it would be non-trivial to actually implement 
resolving from the current included configuration file - we'll have 
to remember which file was current.  Approach which is used now is 
much simpler.)

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