You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by pq...@apache.org on 2004/11/29 05:15:25 UTC

svn commit: r106879 - /httpd/httpd/trunk/CHANGES /httpd/httpd/trunk/server/core.c

Author: pquerna
Date: Sun Nov 28 20:15:24 2004
New Revision: 106879

URL: http://svn.apache.org/viewcvs?view=rev&rev=106879
Log:
* server/core.c: Give an error instead of silently going on when a section is missing an argument.

PR: 25460
Submitted By: Geoffrey Young

Modified:
   httpd/httpd/trunk/CHANGES
   httpd/httpd/trunk/server/core.c

Modified: httpd/httpd/trunk/CHANGES
Url: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?view=diff&rev=106879&p1=httpd/httpd/trunk/CHANGES&r1=106878&p2=httpd/httpd/trunk/CHANGES&r2=106879
==============================================================================
--- httpd/httpd/trunk/CHANGES	(original)
+++ httpd/httpd/trunk/CHANGES	Sun Nov 28 20:15:24 2004
@@ -2,6 +2,10 @@
 
   [Remove entries to the current 2.0 section below, when backported]
 
+  *) core: Error out on sections that are missing an argument instead of
+     silently consuming the section. PR 25460.
+     [Geoffrey Young, Paul Querna]
+
   *) mod_cache/mod_mem_cache/mod_disk_cache: Move out of experimental.
 
   *) Upgraded PCRE to version 5.0. [Brian Pane]

Modified: httpd/httpd/trunk/server/core.c
Url: http://svn.apache.org/viewcvs/httpd/httpd/trunk/server/core.c?view=diff&rev=106879&p1=httpd/httpd/trunk/server/core.c&r1=106878&p2=httpd/httpd/trunk/server/core.c&r2=106879
==============================================================================
--- httpd/httpd/trunk/server/core.c	(original)
+++ httpd/httpd/trunk/server/core.c	Sun Nov 28 20:15:24 2004
@@ -1662,6 +1662,15 @@
                        "> directive missing closing '>'", NULL);
 }
 
+/*
+ * Report a missing args in '<Foo >' syntax error.
+ */
+static char *missing_container_arg(cmd_parms *cmd)
+{
+    return apr_pstrcat(cmd->pool, cmd->cmd->name,
+                       "> directive requires additional arguments", NULL);
+}
+
 AP_CORE_DECLARE_NONSTD(const char *) ap_limit_section(cmd_parms *cmd,
                                                       void *dummy,
                                                       const char *arg)
@@ -1683,6 +1692,10 @@
 
     limited_methods = apr_pstrndup(cmd->pool, arg, endp - arg);
 
+    if (!limited_methods[0]) {
+        return missing_container_arg(cmd);
+    }
+
     while (limited_methods[0]) {
         char *method = ap_getword_conf(cmd->pool, &limited_methods);
         int methnum;
@@ -1750,6 +1763,10 @@
 
     arg = apr_pstrndup(cmd->pool, arg, endp - arg);
 
+    if (!arg[0]) {
+        return missing_container_arg(cmd);
+    }
+
     if (!arg) {
         if (thiscmd->cmd_data)
             return "<DirectoryMatch > block must specify a path";
@@ -1850,6 +1867,10 @@
 
     arg = apr_pstrndup(cmd->pool, arg, endp - arg);
 
+    if (!arg[0]) {
+        return missing_container_arg(cmd);
+    }
+
     cmd->path = ap_getword_conf(cmd->pool, &arg);
     cmd->override = OR_ALL|ACCESS_CONF;
 
@@ -1915,6 +1936,10 @@
 
     arg = apr_pstrndup(cmd->pool, arg, endp - arg);
 
+    if (!arg[0]) {
+        return missing_container_arg(cmd);
+    }
+
     cmd->path = ap_getword_conf(cmd->pool, &arg);
     /* Only if not an .htaccess file */
     if (!old_path) {
@@ -1986,6 +2011,10 @@
         arg++;
     }
 
+    if (!arg[0]) {
+        return missing_container_arg(cmd);
+    }
+
     found = ap_find_linked_module(arg);
 
     /* search prelinked stuff */
@@ -2059,6 +2088,10 @@
         arg++;
     }
 
+    if (!arg[0]) {
+        return missing_container_arg(cmd);
+    }
+
     defined = ap_exists_config_define(arg);
     if ((!not && defined) || (not && !defined)) {
         ap_directive_t *parent = NULL;
@@ -2096,6 +2129,10 @@
     }
 
     arg = apr_pstrndup(cmd->pool, arg, endp - arg);
+
+    if (!arg[0]) {
+        return missing_container_arg(cmd);
+    }
 
     /* FIXME: There's another feature waiting to happen here -- since you
         can now put multiple addresses/names on a single <VirtualHost>

Re: mod_comment (was: svn commit: r106879 ...)

Posted by Rici Lake <ri...@ricilake.net>.
On 29-Nov-04, at 10:10 AM, Greg Stein wrote:

> So no... I don't like mod_comment. It uses a feature that I dearly
> wish to see removed from httpd.

I agree 100% on removing that feature. If it were gone, something
like mod_comment would have to be implemented in the core, I suppose.

However, I was not really proposing it as a module (except maybe
cheekily); rather as a proposed syntax which seems to me to be
more user-friendly than <IfDefine 0>. Of course, tastes differ.

I'll leave my comments on rewriting the config parsing (which I
agree needs to be done) for a longer message; Paul Querna and I
have been talking about this, too. However, I don't think it is
fair to simply point the finger at the invasive mod_perl.

For example, mod_macro is a very simple, easily audited module,
which is extremely useful, and which requires exactly the same
access to the configuration stream. By the same token, the
environment variable expansion in config files is a hack which
would have been better put into an optional module.

All of this suggests that some mechanism ought to exist for modules
to intervene in the configuration process; I think we are
completely in agreement that the current mechanism is not the
right one.

Rici


Re: mod_comment (was: svn commit: r106879 ...)

Posted by Rici Lake <ri...@ricilake.net>.
On 29-Nov-04, at 11:02 AM, William A. Rowe, Jr. wrote:

> Yes, but our API dictates that they are looking for ...>
> (in the case of this module) - which is a bad thing IMHO (and in
> Greg's and Ryan's as well)...

Sorry, I don't understand that. The search for > is in the first line
of the directive; arg is simply the arg delivered by the config 
apparatus.
The point of the following "offending" code is to ensure that the
directive was:
   <Comment This is a comment>
and not:
   <Comment This is a comment
I'm actually neutral on whether it is worth doing this check, but all 
the
other section directives do it, and while it seems a bit puritanical, it
does enforce the vaguely HTML-like section syntax.

> This is the offending code;
>
>     endp = ap_strrchr_c(arg, '>');
>     if (endp == NULL) {
>         return unclosed_directive(cmd);
>     }

So how is that different from parsing any other standard directive? It
is no different than mod_access, for example, checking that the first
argument to Allow is the word "from".

> and this is the good code;
>
>     *(ap_directive_t **)dummy = NULL;
>     return ap_soak_end_container(cmd, "<Comment");
>

It will be noted that this module does not in any way refer to
the fp hidden away in the structs. All it does is use the
public API. If the internal functioning of that API changes, the
module will continue to work.

> The problem is, I doubt this module inserts its contents into
> the config tree.

That is true, it doesn't. The *(ap_directive_t **)dummy = NULL;
statement tells the config apparatus that there are no directives
to insert. This is the part of the module that I personally object
to; that behaviour is non-standard, undocumented, and a kludge.
However, "everybody does it".

> So if we spew the contents (using mod_info,
> or any other tool based on our config tree API) we lose the
> contents.

I would be happy to fix this :) It is worth noting that EXEC_ON_READ
directives are all removed from the config_tree, so that mod_info
cannot show useful things like LoadModule directives.


Re: mod_comment

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Nov 29, 2004 at 02:56:39PM -0500, Stas Bekman wrote:
>...
> Not so fast Greg. You can't just cut an arm because *you* decided you 
> don't like it anymore.  The feature was never called experimental and 
> wasn't scheduled to go away. Now there are multiple users who rely on this 
> feature, both in apache 1.3 and apache 2.x land. So if you suggest a 
> different approach to solve the problem that's fine. But just dropping it 
> on the floor in Apache 2.2 is not an option.

Dood. Don't get your panties in a bunch. I *never* said the feature
would be removed. I'm complaining that it exists, but noting that
(since it does) it must be maintained.

Change the API? Sure. Remove the feature? No.

Please re-read my posts. There might be a way to change the API but
keep the feature, but I'm not sure. I gave up years ago and just left
the darned fp in there. Wasn't worth it to me.

Thanks,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: mod_comment

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Nov 29, 2004 at 02:11:26PM -0500, Geoffrey Young wrote:
>...
> but what's the alternative to reading directly from the file under the
> current, file-based design?

The system could do a container *if* the contents followed the
standard mechanism for defining directives. IOW, the central parser
would snarf in all the data for the container. The external module
could then process the contents as it chose.

The problem is that this doesn't mesh well with dynamic definitions of
directives, nor with non-"standard" syntax for the directives.

> or are you suggesting that nobody but core
> should be allowed to create <Foo> blocks?

That's exactly what I'm suggesting :-)

> surely mod_perl's <Perl>
> containers can't be the only one out there in the wild?

No idea. Possibly, but I haven't heard/seen any of them. And even so,
mod_perl is my favorite whipping boy, so it'll serve just fine as a
demo of the problem :-)

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: mod_comment

Posted by Stas Bekman <st...@stason.org>.
Greg Stein wrote:
> This is a desired change. In no way is it close to being a
> showstopper. Further, I'm not even sure how to provide the feature and
> (at the same time) keep the fp private. Some callback nonsense or
> something with buffers. Blech...
> 
> I'd like to just toss the feature and be done with it :-)
> 
> But no... if somebody happens to get the desire to fix it by 2.2, then
> great. If nobody does, then great.

Not so fast Greg. You can't just cut an arm because *you* decided you 
don't like it anymore.  The feature was never called experimental and 
wasn't scheduled to go away. Now there are multiple users who rely on this 
feature, both in apache 1.3 and apache 2.x land. So if you suggest a 
different approach to solve the problem that's fine. But just dropping it 
on the floor in Apache 2.2 is not an option.

-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Re: mod_comment

Posted by Paul Querna <ch...@force-elite.com>.
Greg Stein wrote:
> This is a desired change. In no way is it close to being a
> showstopper.

+1, exactly my feelings.

Re: mod_comment

Posted by Greg Stein <gs...@lyra.org>.
This is a desired change. In no way is it close to being a
showstopper. Further, I'm not even sure how to provide the feature and
(at the same time) keep the fp private. Some callback nonsense or
something with buffers. Blech...

I'd like to just toss the feature and be done with it :-)

But no... if somebody happens to get the desire to fix it by 2.2, then
great. If nobody does, then great.


On Mon, Nov 29, 2004 at 09:19:04PM +0200, Graham Leggett wrote:
> Greg Stein wrote:
> 
> >Yes, we can change the API (that was allowed going from 1.3 to 2.0),
> >but we were not allowed to remove features. Removing the fp from the
> >API would have disabled this "feature" in mod_perl, among others.
> 
> When APR v1.0's first release candidate came out, there were loud calls 
> saying "code XXX's API is broken, we can't release like this". So code 
> XXX got fixed, and the process repeated for six release candidates. But 
> APR v1.0 was cleansed of a lot of nonsense at it's final release.
> 
> I strongly suspect that when the first release candidate of httpd v2.2 
> comes out, people will say "wait, there is brokenness in code YYY, we 
> can't go GA like this", and a call will be made for a fix and another 
> release candidate.
> 
> So - do we embark on an effort to identify (and hopefully fix) broken 
> code such as the exposing of the file pointer to modules before v2.2 
> goes GA, or do we target these sorts of breakage for httpd v2.4?
> 
> (There are other issues that need fixing, apparently there are some four 
> or so config directives that once turned on cannot be turned off again 
> inside a more specific config container.)
> 
> Regards,
> Graham
> --



-- 
-- 
Greg Stein, http://www.lyra.org/

Re: mod_comment

Posted by Geoffrey Young <ge...@modperlcookbook.org>.
>>but in that
>>respect, we are no different from any other module that wants to implement a
>>RAW_ARGS directive...
> 
> 
> Hardly. RAW_ARGS takes a single line of text. No file pointer. That
> line could come from anywhere.

yeah, ok.  I _meant_ create your own <Foo> container, which I guess is the
typical (though certainly not only) use for RAW_ARGS :)

> 
> The container stuff takes a file pointer and reads arbitrary amounts
> of input from the file. There is no way that the core parser can snarf
> up the text and pass that block to you [independent of the fp] since
> the syntax could be arbitrary.

right, that's the issue.

but what's the alternative to reading directly from the file under the
current, file-based design?  or are you suggesting that nobody but core
should be allowed to create <Foo> blocks?  surely mod_perl's <Perl>
containers can't be the only one out there in the wild?

--Geoff

Re: mod_comment

Posted by Stas Bekman <st...@stason.org>.
Greg Stein wrote:
> On Mon, Nov 29, 2004 at 01:54:18PM -0500, Geoffrey Young wrote:
> 
>>Justin Erenkrantz wrote:
>>
>>>--On Monday, November 29, 2004 10:41 AM -0800 Greg Stein
>>><gs...@lyra.org> wrote:
>>>
>>>
>>>>but we were not allowed to remove features. Removing the fp from the
>>>>API would have disabled this "feature" in mod_perl, among others.
>>>
>>>I can't tell you how often I've been bitten personally by mod_perl
>>>trying to reparse our configuration because of this 'feature.'  It needs
>>>to go away big time. 
>>
>>sorry, but I'm not following.  can you guys be specific (code wise) about
>>what is supposed to be forbidden?  my only knowledge of mod_perl using that
>>file pointer is when implementing <Foo> containers where we (of course) need
>>to know what is between the opening and closing markers.
> 
> 
> Yup. That's the one. And to do that, you use the file pointer which
> gets passed out thru the API.

so what's the proposed replacement? If we get memmapped pointer or any 
other pointer that can be parsed instead, that will work just fine. Just 
hiding the fp from the API will make it impossible to implement custom 
containers.


-- 
__________________________________________________________________
Stas Bekman            JAm_pH ------> Just Another mod_perl Hacker
http://stason.org/     mod_perl Guide ---> http://perl.apache.org
mailto:stas@stason.org http://use.perl.org http://apacheweek.com
http://modperlbook.org http://apache.org   http://ticketmaster.com

Re: mod_comment

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Nov 29, 2004 at 01:54:18PM -0500, Geoffrey Young wrote:
> Justin Erenkrantz wrote:
> > --On Monday, November 29, 2004 10:41 AM -0800 Greg Stein
> > <gs...@lyra.org> wrote:
> > 
> >> but we were not allowed to remove features. Removing the fp from the
> >> API would have disabled this "feature" in mod_perl, among others.
> > 
> > I can't tell you how often I've been bitten personally by mod_perl
> > trying to reparse our configuration because of this 'feature.'  It needs
> > to go away big time. 
> 
> sorry, but I'm not following.  can you guys be specific (code wise) about
> what is supposed to be forbidden?  my only knowledge of mod_perl using that
> file pointer is when implementing <Foo> containers where we (of course) need
> to know what is between the opening and closing markers.

Yup. That's the one. And to do that, you use the file pointer which
gets passed out thru the API.

> but in that
> respect, we are no different from any other module that wants to implement a
> RAW_ARGS directive...

Hardly. RAW_ARGS takes a single line of text. No file pointer. That
line could come from anywhere.

The container stuff takes a file pointer and reads arbitrary amounts
of input from the file. There is no way that the core parser can snarf
up the text and pass that block to you [independent of the fp] since
the syntax could be arbitrary.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: mod_comment

Posted by Graham Leggett <mi...@sharp.fm>.
Geoffrey Young wrote:

> sorry, but I'm not following.  can you guys be specific (code wise) about
> what is supposed to be forbidden?  my only knowledge of mod_perl using that
> file pointer is when implementing <Foo> containers where we (of course) need
> to know what is between the opening and closing markers.  but in that
> respect, we are no different from any other module that wants to implement a
> RAW_ARGS directive...

The way the config code works (when I last looked at it, which was a 
while ago) is that it opens up a file, and swallows up lines of the file 
one by one. Sometimes lines are comments and are ignored, sometimes 
lines are containers, and code is run to handle the nesting, and 
sometimes the line is a normal directive to be processed.

If a module (like mod_perl) grabs the fp and reads the config file 
itself, it is simply also doing line by line processing as described 
above, in other words two pieces of code doing the same thing.

Ideally (shoot me down if I am wrong) a hook should be provided that 
provides each line - an underlying module can then create "lines" from 
lines of a text file, or .htaccess file, or by walking an XML tree, 
whatever. As each "line" is a config directive + optional argument(s) 
(or container directive + option argument(s)), you could break it up 
into a name/value pair instead.

The ability to pass container <Foo> directives to modules in addition to 
directives would need to be sorted out so as to solve mod_perl's config 
problems. There currently isn't a mechanism to do this as I understand 
(yet).

Regards,
Graham
--

Re: mod_comment

Posted by Geoffrey Young <ge...@modperlcookbook.org>.

Justin Erenkrantz wrote:
> --On Monday, November 29, 2004 10:41 AM -0800 Greg Stein
> <gs...@lyra.org> wrote:
> 
>> but we were not allowed to remove features. Removing the fp from the
>> API would have disabled this "feature" in mod_perl, among others.
> 
> 
> I can't tell you how often I've been bitten personally by mod_perl
> trying to reparse our configuration because of this 'feature.'  It needs
> to go away big time. 

sorry, but I'm not following.  can you guys be specific (code wise) about
what is supposed to be forbidden?  my only knowledge of mod_perl using that
file pointer is when implementing <Foo> containers where we (of course) need
to know what is between the opening and closing markers.  but in that
respect, we are no different from any other module that wants to implement a
RAW_ARGS directive...

--Geoff

Re: mod_comment (was: svn commit: r106879 ...)

Posted by Justin Erenkrantz <ju...@erenkrantz.com>.
--On Monday, November 29, 2004 10:41 AM -0800 Greg Stein <gs...@lyra.org> 
wrote:

> but we were not allowed to remove features. Removing the fp from the
> API would have disabled this "feature" in mod_perl, among others.

I can't tell you how often I've been bitten personally by mod_perl trying to 
reparse our configuration because of this 'feature.'  It needs to go away big 
time.  -- justin

Re: mod_comment

Posted by Paul Querna <ch...@force-elite.com>.
Graham Leggett wrote:
  > So - do we embark on an effort to identify (and hopefully fix) broken
> code such as the exposing of the file pointer to modules before v2.2 
> goes GA, or do we target these sorts of breakage for httpd v2.4?

If 2.4 is < 1 year after 2.2, then yes, 2.4.

Re: mod_comment

Posted by Graham Leggett <mi...@sharp.fm>.
Greg Stein wrote:

> Yes, we can change the API (that was allowed going from 1.3 to 2.0),
> but we were not allowed to remove features. Removing the fp from the
> API would have disabled this "feature" in mod_perl, among others.

When APR v1.0's first release candidate came out, there were loud calls 
saying "code XXX's API is broken, we can't release like this". So code 
XXX got fixed, and the process repeated for six release candidates. But 
APR v1.0 was cleansed of a lot of nonsense at it's final release.

I strongly suspect that when the first release candidate of httpd v2.2 
comes out, people will say "wait, there is brokenness in code YYY, we 
can't go GA like this", and a call will be made for a fix and another 
release candidate.

So - do we embark on an effort to identify (and hopefully fix) broken 
code such as the exposing of the file pointer to modules before v2.2 
goes GA, or do we target these sorts of breakage for httpd v2.4?

(There are other issues that need fixing, apparently there are some four 
or so config directives that once turned on cannot be turned off again 
inside a more specific config container.)

Regards,
Graham
--

Re: mod_comment (was: svn commit: r106879 ...)

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Nov 29, 2004 at 07:33:35PM +0100, Andr? Malo wrote:
> * "William A. Rowe, Jr." <wr...@rowe-clan.net> wrote:
> 
> > This is the offending code;
> > 
> >     endp = ap_strrchr_c(arg, '>');
> >     if (endp == NULL) {
> >         return unclosed_directive(cmd);
> >     }
> 
> Aah! understood, thanks ;-)
> So I'd guess, there's currently no "good" way for implementing containers?

It has nothing to do with containers (I have no idea how Bill's code
relates). It is all about exposing the file pointer *outside* of the
config parsing code. Once you do that, then you're locked into
*files*.

The original design for the config parsing revamp was to encapsulate
the notion of "parse a file". But at some point in the game, somebody
allowed third-party modules to reach in and take over the parsing
process. That sucks. It means we have to keep exposing a file pointer
(somewhere) if we want to remain feature-compatible.

Yes, we can change the API (that was allowed going from 1.3 to 2.0),
but we were not allowed to remove features. Removing the fp from the
API would have disabled this "feature" in mod_perl, among others.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: mod_comment (was: svn commit: r106879 ...)

Posted by André Malo <nd...@perlig.de>.
* "William A. Rowe, Jr." <wr...@rowe-clan.net> wrote:

> This is the offending code;
> 
>     endp = ap_strrchr_c(arg, '>');
>     if (endp == NULL) {
>         return unclosed_directive(cmd);
>     }

Aah! understood, thanks ;-)
So I'd guess, there's currently no "good" way for implementing containers?

nd

Re: mod_comment (was: svn commit: r106879 ...)

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
At 09:31 AM 11/29/2004, André Malo wrote:
>* Greg Stein <gs...@lyra.org> wrote:
>
>> Taking over the reading/parsing of the file pointer. We pass the fp to
>> modules (when they process a directive), allowing them to read from
>> it. Bad bad bad. Especially when you're trying to read your config
>> from some place funky.
>
>I seem to miss something ;-). AFAICS the module just uses our API? (assuming
>you mean the things happening in ap_soak_end_container).

Yes, but our API dictates that they are looking for ...>
(in the case of this module) - which is a bad thing IMHO (and in
Greg's and Ryan's as well)...

This is the offending code;

    endp = ap_strrchr_c(arg, '>');
    if (endp == NULL) {
        return unclosed_directive(cmd);
    }

and this is the good code;

    *(ap_directive_t **)dummy = NULL;
    return ap_soak_end_container(cmd, "<Comment");

(sort of)...

The problem is, I doubt this module inserts its contents into
the config tree.  So if we spew the contents (using mod_info,
or any other tool based on our config tree API) we lose the
contents.  (We had an earlier discussion on list which went
into whether or not we wanted to waste the ram on comments,
and the concensus was, at some point add a flag to the parser
to either include or discard comments from the tree.)

The hope was to allow multiple .conf file syntaxes, such that
we could insert an xml based format some day.  mod_perl, and
to a lesser degree, mod_macro, make this impossible.

Config parsing is a one (or two or three) pass operation at
server startup, we should take liberties with the cpu cycles 
to achieve an effective result.

Bill 


Re: mod_comment (was: svn commit: r106879 ...)

Posted by André Malo <nd...@perlig.de>.
* Greg Stein <gs...@lyra.org> wrote:

> Taking over the reading/parsing of the file pointer. We pass the fp to
> modules (when they process a directive), allowing them to read from
> it. Bad bad bad. Especially when you're trying to read your config
> from some place funky.

I seem to miss something ;-). AFAICS the module just uses our API? (assuming
you mean the things happening in ap_soak_end_container).

nd

Re: mod_comment (was: svn commit: r106879 ...)

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Nov 29, 2004 at 04:20:21PM +0100, Andr? Malo wrote:
> * Greg Stein <gs...@lyra.org> wrote:
> 
> > > https://ssl.bulix.org/projects/rici/file/apache/mod_comment.c?rev=49
> > 
> > That module uses a feature of the configuration parser that should be
> > removed. The capability of code external to the config parser "taking
> > over" the reading/parsing of the config file severely limits the
> > revamping of the config system in httpd.
> 
> Ehm, what do you mean? Raw args?

Taking over the reading/parsing of the file pointer. We pass the fp to
modules (when they process a directive), allowing them to read from
it. Bad bad bad. Especially when you're trying to read your config
from some place funky.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: mod_comment (was: svn commit: r106879 ...)

Posted by André Malo <nd...@perlig.de>.
* Greg Stein <gs...@lyra.org> wrote:

> > https://ssl.bulix.org/projects/rici/file/apache/mod_comment.c?rev=49
> 
> That module uses a feature of the configuration parser that should be
> removed. The capability of code external to the config parser "taking
> over" the reading/parsing of the config file severely limits the
> revamping of the config system in httpd.

Ehm, what do you mean? Raw args?

nd

mod_comment (was: svn commit: r106879 ...)

Posted by Greg Stein <gs...@lyra.org>.
On Mon, Nov 29, 2004 at 09:44:14AM -0500, Rici Lake wrote:
>...
> Just out of curiousity, what do people think is wrong with the
> <Comment> directive implemented by this micro-module:
> 
> https://ssl.bulix.org/projects/rici/file/apache/mod_comment.c?rev=49

That module uses a feature of the configuration parser that should be
removed. The capability of code external to the config parser "taking
over" the reading/parsing of the config file severely limits the
revamping of the config system in httpd.

A couple years ago, Ryan and I dove in to redo the config parsing. The
idea was to parse everything into an internal structure, and "run
through" that structure calling out to the various modules to perform
the directives. The idea was to have different front-ends be able to
put together that structure (e.g. parse from xml, load from a db).

The problem is that certain third-party modules used that "take over
reading" feature, which completely messed up a lot of where we were
going. We couldn't move the API in certain directions, we had to
limit certain kinds of changes, etc etc. It was way depressing. Even
worse was that we felt we couldn't just say "screw those modules" as
the primary user was the ever-present super-invasive mod_perl module.
That damned module sticks its fingers into too many places of httpd
and takes too many liberties. It's a pain in the ass, but that's
another story.

So no... I don't like mod_comment. It uses a feature that I dearly
wish to see removed from httpd.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: svn commit: r106879 - /httpd/httpd/trunk/CHANGES /httpd/httpd/trunk/server/core.c

Posted by Rici Lake <ri...@ricilake.net>.
Rephrasing the question slightly:

On 29-Nov-04, at 9:44 AM, Rici Lake wrote:

> Just out of curiousity, what do people think is wrong with the
> <Comment> directive implemented by this micro-module:

I meant to ask: What do people think about a <Comment>
directive, which would simply absorb the configuration
stream up to the matching </Comment> line, parsing only
to the extent necessary to match section opens and closes?

Thanks to Greg Stein for pushing me to be clearer about the
question.

Rici


Re: svn commit: r106879 - /httpd/httpd/trunk/CHANGES /httpd/httpd/trunk/server/core.c

Posted by Rici Lake <ri...@ricilake.net>.
On 29-Nov-04, at 4:05 AM, André Malo wrote:

> * Paul Querna wrote:
>
>> What is wrong with <IfDefine 0>?

It's ugly and non-intuitive.

Just out of curiousity, what do people think is wrong with the
<Comment> directive implemented by this micro-module:

https://ssl.bulix.org/projects/rici/file/apache/mod_comment.c?rev=49



Re: svn commit: r106879 - /httpd/httpd/trunk/CHANGES /httpd/httpd/trunk/server/core.c

Posted by André Malo <nd...@perlig.de>.
* Paul Querna wrote:

> What is wrong with <IfDefine 0>?

First, that <IfDefine > is now no longer reliable ;-)
And second -D 0, of course.

> I didn't feel it was correct to never allow -D 0 from the command line,

You mean, -D0 is not allowed?

But wait, <IfDefine ""> might work...

nd
-- 
print "Just Another Perl Hacker";

# André Malo, <http://www.perlig.de/> #

Re: svn commit: r106879 - /httpd/httpd/trunk/CHANGES /httpd/httpd/trunk/server/core.c

Posted by Paul Querna <ch...@force-elite.com>.
André Malo wrote:
> * pquerna@apache.org wrote:
> 
> 
>>+  *) core: Error out on sections that are missing an argument instead of
>>+     silently consuming the section. PR 25460.
>>+     [Geoffrey Young, Paul Querna]
> 
> 
> :-( Actually, for IfDefine I've sold that that as feature, since it's the 
> only reliable way to produce multiline comments.
> 
> I'd said that before here on the list, could we *please* let that in or 
> provide an alternative?
> 
> nd

What is wrong with <IfDefine 0>?

I didn't feel it was correct to never allow -D 0 from the command line, 
but is having '<IfDefine 0>' that much more difficult than '<IfDefine >'?

-Paul Querna

Re: svn commit: r106879 - /httpd/httpd/trunk/CHANGES /httpd/httpd/trunk/server/core.c

Posted by André Malo <nd...@perlig.de>.
* pquerna@apache.org wrote:

> +  *) core: Error out on sections that are missing an argument instead of
> +     silently consuming the section. PR 25460.
> +     [Geoffrey Young, Paul Querna]

:-( Actually, for IfDefine I've sold that that as feature, since it's the 
only reliable way to produce multiline comments.

I'd said that before here on the list, could we *please* let that in or 
provide an alternative?

nd
-- 
Das einzige, das einen Gebäudekollaps (oder auch einen
thermonuklearen Krieg) unbeschadet übersteht, sind Kakerlaken
und AOL-CDs.
                                      -- Bastian Lipp in dcsm