httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stefan Fritsch ...@sfritsch.de>
Subject Re: Question and request for comments on patch
Date Tue, 26 Jul 2011 10:02:35 GMT
On Monday 25 July 2011, Daniel Ruggeri wrote:
> On 7/24/2011 2:12 AM, Stefan Fritsch wrote:
> > When parsing the list, look in ap_config_hash if a directive
> > exists. If not, either log a warning or error out (not sure
> > which is better).
> 
> I may be missing something, but isn't ap_config_hash private to
> config.c and unavailable to core.c where the configuration
> directive is implemented? If I'm off, I'd definitely like to add
> this as well as remove the need for the 'tolower' calls by looking
> up the expected CamelCase of a directive as it shows up in
> config.c.

This is valid use-case which justifies making it available somehow. 
But actually, I think something like

module *mod = &ap_top_module;
ap_find_command_in_modules(cmd, mod);

should already work without the need for an additional API. This way 
is not very efficient but as this is only done on server startup, 
that's not important.

> > It should be possible to reset AllowOverrideList to an empty list
> > in a subdir, even if it is set in the parent dir. This is
> > important in the case that there is a permissive
> > AllowOverrideList in main server scope and an empty one for some
> > virtual hosts.  In the case of an empty list, d->override_list
> > should be set to NULL instead of an empty table for better
> > performance.  Maybe one could allow "disabled" or "reset" as
> > alias for an empty list (like in UserDir/DirectoryIndex).
> 
> I realized I did not add this about five minutes after sending the
> patch along to the list. I have added this also. Your followup
> email about making the list NULL complicating things is also true
> - I had a lot of trouble merging properly when NULL was considered
> "nothing in the list" as opposed to "not configured".

htaccess parsing is notoriously slow anyway. One more table lookup 
won't hurt that much, so IMHO no need to complicate things here.

> > It may be possible that some modules react badly if a directive
> > is used in .htaccess that has previously not been allowed there.
> > For example if the module needs to do some global initialization
> > when a directive is used for the first time. I am not aware of
> > such a case, but it is something we may want to keep in mind if
> > this is backported. And it is definitely something that should
> > go into the new_api_2.4 document.
> 
> Yes, very good point - I did not realize this would become a side
> effect. I thought there were upstream context checks before a
> command is invoked to prevent this.
> I have taken the second suggestion and added a note in
> new_api_2.4.xml also.

Maybe we want to extend ap_check_cmd_context() to have something like 
NOT_IN_HTACCESS. In any case, I think your patch is good enough for 
the next beta. We can solve any remaining issues before GA.

Cheers,
Stefan

Mime
View raw message