Welcome! Log In Create A New Profile

Advanced

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

Maxim Dounin
September 03, 2019 10:40AM
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-@-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/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

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

Guillaume Outters 83 August 29, 2019 03:02PM

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

Maxim Dounin 19 August 30, 2019 10:04AM

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

Guillaume Outters 13 August 30, 2019 10:28AM

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

Maxim Dounin 13 September 03, 2019 10:40AM

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

Guillaume Outters 15 September 03, 2019 12:24PM

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

Maxim Dounin 13 September 09, 2019 06:50AM

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

Guillaume Outters 15 September 09, 2019 12:16PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 85
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready