[PATCH] catching malformed container directives

[Geoffrey Young <ge...@modperlcookbook.org>]

hi all...

currently, all core container directives have an issue (albeit a minor one)
- they require an argument in practice but are allowed to proceed without
one during configuration.

for example

<IfModule >

does not currently throw an error.  instead, the config is allowed to
proceed, soaking up the container.

while the subsequent behavior seems ok, to me the lack of feedback for a
malformed directive is less than ideal, especially if users are using a
template to configure apache - something like this

<IfModule [% mymod %]>

or whatever would silently proceed if the substitution variable was
unpopulated, which is a tricky thing to track down

anyway, attached is a patch that makes <IfModule>, <IfDefine>, <Directory>,
<Location>, <Files> (and their regex counterparts), <Limit>, <LimitExcept>,
and <VirtualHost> containers fail if no arguments are specified.

--Geoff

Re: [PATCH] catching malformed container directives

[Geoffrey Young <ge...@modperlcookbook.org>]

hi all

just following up on this to see if the interested parties have given the
issue any more thought.  if there is still some dis-harmony around the
<IfDefine > case then I'm happy to keep posting alternatives until a
solution is reached that everyone can live with.

otherwise, if there isn't sufficient interest in changing the current
(somewhat broken) core container args parsing, I'll just lop it off of my
todo list.

--Geoff

(current patch viewable from
http://nagoya.apache.org/bugzilla/showattachment.cgi?attach_id=9526)


Geoffrey Young wrote:
>>>Now you have me thinking.  For Apache 2.1 (perhaps 2.0) I'd like to see that
>>>particular nonsense go away.  I sympathize with André's observation that it's
>>>useful, but what he wants to do can be accomplished with
>>>
>>><IfDefine NEVER>
>>>   DangerousDirective
>>></IfDefine>
>>>
>>>which serves the same purpose, but it much more legible.
>>
>>
>>not to speak for andre, but he pointed out to me on irc that what he was
>>after was an <IfDefine> that could not be overridden with -D, and I suppose
>>-DNEVER would expose the config block.  or are you suggesting a literal
>>"<IfDefine NEVER>" as a special case?  one thing I suggested was perhaps
>>using <IfDefine 0>, but he pointed out that -D0 works (but -D"" doesn't).
>>so maybe we can make -D0 not work as well and keep with something that feels
>>programmatically familiar.
> 
> 
> yet another try :)
> 
> this patch makes 'httpd -D0' invalid, thus making <IfDefine 0> a special
> define case that is guaranteed to evaluate to false.  the rest remains as
> before - arguments are enforced across all containers.
> 
> it actually feels a bit strange to fail the command line args without some
> kind of message, so I suppose it might be wiser to implement this in core.c
> instead, tossing an error message to the error_log if "0" is both caught and
> defined.  but for the moment I guess I'm just seeing if the idea is
> appealing, after which the implementation can be adjusted as required.
> 
> --Geoff

Re: [PATCH] catching malformed container directives

[Geoffrey Young <ge...@modperlcookbook.org>]

>>Now you have me thinking.  For Apache 2.1 (perhaps 2.0) I'd like to see that
>>particular nonsense go away.  I sympathize with André's observation that it's
>>useful, but what he wants to do can be accomplished with
>>
>><IfDefine NEVER>
>>    DangerousDirective
>></IfDefine>
>>
>>which serves the same purpose, but it much more legible.
> 
> 
> not to speak for andre, but he pointed out to me on irc that what he was
> after was an <IfDefine> that could not be overridden with -D, and I suppose
> -DNEVER would expose the config block.  or are you suggesting a literal
> "<IfDefine NEVER>" as a special case?  one thing I suggested was perhaps
> using <IfDefine 0>, but he pointed out that -D0 works (but -D"" doesn't).
> so maybe we can make -D0 not work as well and keep with something that feels
> programmatically familiar.

yet another try :)

this patch makes 'httpd -D0' invalid, thus making <IfDefine 0> a special
define case that is guaranteed to evaluate to false.  the rest remains as
before - arguments are enforced across all containers.

it actually feels a bit strange to fail the command line args without some
kind of message, so I suppose it might be wiser to implement this in core.c
instead, tossing an error message to the error_log if "0" is both caught and
defined.  but for the moment I guess I'm just seeing if the idea is
appealing, after which the implementation can be adjusted as required.

--Geoff

Re: [PATCH] catching malformed container directives

[Geoffrey Young <ge...@modperlcookbook.org>]

William A. Rowe, Jr. wrote:
> At 09:36 AM 12/9/2003, Geoffrey Young wrote:
> 
>>>André Malo wrote:
>>>
>>>>I'd like to keep <IfDefine > possible. Simply because it's a very efficient
>>>>way to comment a whole part out (reliably, since one cannot specify an
>>>>empty -D argument). And it's in use out there.
>>
>>ok, here is a new patch that excludes <IfDefine >.
> 
> 
> Now you have me thinking.  For Apache 2.1 (perhaps 2.0) I'd like to see that
> particular nonsense go away.  I sympathize with André's observation that it's
> useful, but what he wants to do can be accomplished with
> 
> <IfDefine NEVER>
>     DangerousDirective
> </IfDefine>
> 
> which serves the same purpose, but it much more legible.

not to speak for andre, but he pointed out to me on irc that what he was
after was an <IfDefine> that could not be overridden with -D, and I suppose
-DNEVER would expose the config block.  or are you suggesting a literal
"<IfDefine NEVER>" as a special case?  one thing I suggested was perhaps
using <IfDefine 0>, but he pointed out that -D0 works (but -D"" doesn't).
so maybe we can make -D0 not work as well and keep with something that feels
programmatically familiar.

> 
> You point out that containers that expect args (e.g. not <Perl> blocks) can 
> be very difficult to debug in any sort of dynamic (mod_macro) sort of config
> environment. 

that was part of the rationale, yes.  the other being that it just didn't
seem to make sense that the directive arguments were required in the docs
(and in practice) but that illegally formed configs were allowed to pass
without so much as a warning.

> But also consider that many conf rewriting tools could probably 
> crack under the strain of parsing such <IfFoo> containers, if they specify
> no argument.

it must be that it's post-dinner here and I need a rejolt of coffee, but I
don't understand that :)

> 
> On the balance, for 2.1 forward we should disallow <IfDefine >.  It's a coin
> toss to me if we continue to accept them in 2.0 - so I'd argue the principle
> of least surprise.  Disallow in 2.1, but continue to allow in 2.0 <IfDefine >.

either way, my patches were against 2.1, so if the community agrees to 2.1
inclusion then I suppose 2.0 backport gets voted on afterward.

> 
> 
>>also included is a patch (that applies on top of the other one) that fixes
>>broken <Limit> containers.  currently
>>
>><Limit GET
>>
>>does not throw an error (note the missing final '>').
> 
> 
> That would be goodness!

:)

if you'd like that as a separate issue before the <IfFoo > stuff is resolved
(as it's probably less controversial :) I'll whip up a new patch - just let
me know.

--Geoff

Re: [PATCH] catching malformed container directives

["William A. Rowe, Jr." <wr...@apache.org>]

At 09:36 AM 12/9/2003, Geoffrey Young wrote:
>> 
>> André Malo wrote:
>>>
>>>I'd like to keep <IfDefine > possible. Simply because it's a very efficient
>>>way to comment a whole part out (reliably, since one cannot specify an
>>>empty -D argument). And it's in use out there.
>
>ok, here is a new patch that excludes <IfDefine >.

Now you have me thinking.  For Apache 2.1 (perhaps 2.0) I'd like to see that
particular nonsense go away.  I sympathize with André's observation that it's
useful, but what he wants to do can be accomplished with

<IfDefine NEVER>
    DangerousDirective
</IfDefine>

which serves the same purpose, but it much more legible.

You point out that containers that expect args (e.g. not <Perl> blocks) can 
be very difficult to debug in any sort of dynamic (mod_macro) sort of config
environment.  But also consider that many conf rewriting tools could probably 
crack under the strain of parsing such <IfFoo> containers, if they specify
no argument.

On the balance, for 2.1 forward we should disallow <IfDefine >.  It's a coin
toss to me if we continue to accept them in 2.0 - so I'd argue the principle
of least surprise.  Disallow in 2.1, but continue to allow in 2.0 <IfDefine >.

>also included is a patch (that applies on top of the other one) that fixes
>broken <Limit> containers.  currently
>
><Limit GET
>
>does not throw an error (note the missing final '>').

That would be goodness!

Bill 

Re: [PATCH] catching malformed container directives

[Geoffrey Young <ge...@modperlcookbook.org>]

Geoffrey Young wrote:
> 
> André Malo wrote:
> 
>>* Geoffrey Young <ge...@modperlcookbook.org> wrote:
>>
>>
>>
>>>anyway, attached is a patch that makes <IfModule>, <IfDefine>,
>>
>>
>>I'd like to keep <IfDefine > possible. Simply because it's a very efficient
>>way to comment a whole part out (reliably, since one cannot specify an
>>empty -D argument). And it's in use out there.

ok, here is a new patch that excludes <IfDefine >.

also included is a patch (that applies on top of the other one) that fixes
broken <Limit> containers.  currently

<Limit GET

does not throw an error (note the missing final '>').

if people don't like the first idea but would like to fix <Limit> I can do
that separately as well.

HTH

--Geoff

Re: [PATCH] catching malformed container directives

[Geoffrey Young <ge...@modperlcookbook.org>]

André Malo wrote:
> * Geoffrey Young <ge...@modperlcookbook.org> wrote:
> 
> 
>>anyway, attached is a patch that makes <IfModule>, <IfDefine>,
> 
> 
> I'd like to keep <IfDefine > possible. Simply because it's a very efficient
> way to comment a whole part out (reliably, since one cannot specify an
> empty -D argument). And it's in use out there.

ah, cool.  thanks.

I wasn't aware that people were taking advantage of this "feature" :)

so, ok, any other exceptions?  this was just an idea, so I won't push it or
anything - it just felt weird :)

--Geoff

Re: [PATCH] catching malformed container directives

[André Malo <nd...@perlig.de>]

* Geoffrey Young <ge...@modperlcookbook.org> wrote:

> anyway, attached is a patch that makes <IfModule>, <IfDefine>,

I'd like to keep <IfDefine > possible. Simply because it's a very efficient
way to comment a whole part out (reliably, since one cannot specify an
empty -D argument). And it's in use out there.

nd