Re: [PATCH] Make configuration file "include" directive handling more robust

Ian Barwick <ian(dot)barwick(at)2ndquadrant(dot)com> writes:
> On 7/17/19 5:34 PM, Kyotaro Horiguchi wrote:> Hello.
>>> I don't think this is new to 12.

> No, though I'm not sure how much this would be seen as a bugfix
> and how far back it would be sensible to patch.

I think this is worth considering as a bugfix; although I'm afraid
we can't change the signature of ParseConfigFile/ParseConfigFp in
released branches, since extensions could possibly be using those.
That limits what we can do --- but it's still possible to detect
direct recursion, which seems like enough to produce a nice error
message in typical cases.

I concur with Kyotaro-san that disallow-empty-include-directives.v1.patch
seems a bit brute-force, but where I would put the checks is in
ParseConfigFile and ParseConfigDirectory.

Also, I don't agree with the goals of prevent-disallowed-includes.patch.
I'm utterly not on board with breaking use of "include" in extension
files, for instance; while that may not be documented, it works fine,
and maybe somebody out there is relying on it. Likewise, while "include"
in pg.auto.conf is not really considered supported, I don't see the
point of going out of our way to break the historical behavior.

That leads me to propose the attached simplified patch. While I haven't
actually tried, I'm pretty sure this should back-patch without trouble.

regards, tom lane