Re: [PATCH] lib-httpd/apache.conf: check version only after mod_version loads

2013-06-21 Thread Junio C Hamano
Jeff King  writes:

> Cool. I think the patch should look like the one below, then.
>
> Just to double-check that I have explained the issue correctly, can you
> share the output of "apache2 -l"? Mine has:
>
>   $ apache2 -l
>   Compiled in modules:
> core.c
> mod_log_config.c
> mod_logio.c
> mod_version.c
> prefork.c
> http_core.c
> mod_so.c
>
> which explains why it works here. I'm assuming you will not have
> mod_version.c compiled in.
>
> -- >8 --
> Subject: lib-httpd/apache.conf: check version only after mod_version loads
>
> Commit 0442743 introduced an  directive near the
> top of the apache config file. However, at that point we
> have not yet checked for and loaded the mod_version module.
> This means that the directive will behave oddly if
> mod_version is dynamically loaded, failing to match when it
> should.
>
> We can fix this by moving the whole block below the
> LoadModule directive for mod_version.
>
> Reported-by: Brian Gernhardt 
> Signed-off-by: Jeff King 
> ---
>  t/lib-httpd/apache.conf | 7 ---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/t/lib-httpd/apache.conf b/t/lib-httpd/apache.conf
> index 56ae548..dd17e3a 100644
> --- a/t/lib-httpd/apache.conf
> +++ b/t/lib-httpd/apache.conf
> @@ -1,7 +1,4 @@ ServerName dummy
>  ServerName dummy
> -
> -LockFile accept.lock
> -
>  PidFile httpd.pid
>  DocumentRoot www
>  LogFormat "%h %l %u %t \"%r\" %>s %b" common
> @@ -26,6 +23,10 @@ ErrorLog error.log
>   LoadModule version_module modules/mod_version.so
>  
>  
> +
> +LockFile accept.lock
> +
> +

Once you see it in the patch form, it is very clear what this change
does and why it is necessary in the context ;-)

Thanks, both of you, for digging this down to the root cause; you
guys have done before it graduates to 'master', which I especially
appreciate.

Will queue.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] lib-httpd/apache.conf: check version only after mod_version loads

2013-06-21 Thread Jeff King
On Fri, Jun 21, 2013 at 02:15:39PM -0400, Brian Gernhardt wrote:

> > Cool. I think the patch should look like the one below, then.
> 
> Basically identical to what I've done, you're just faster to the actual 
> patch.  :-D

I started writing the rationale immediately after making the suggestion,
before you responded.  Lucky for me my guess turned out to be right. :)

> > which explains why it works here. I'm assuming you will not have
> > mod_version.c compiled in.
> 
> Indeed I do not.
> 
>  $ httpd -l
>  Compiled in modules:
>core.c
>prefork.c
>http_core.c
>mod_so.c

Makes sense. Thanks for the bug report.

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] lib-httpd/apache.conf: check version only after mod_version loads

2013-06-21 Thread Brian Gernhardt

On Jun 21, 2013, at 2:12 PM, Jeff King  wrote:

> On Fri, Jun 21, 2013 at 02:08:49PM -0400, Brian Gernhardt wrote:
> 
>> On Jun 21, 2013, at 2:03 PM, Jeff King  wrote:
>> 
>>> IfVersion comes from mod_version. I assume that if it were not
>>> loaded, apache would complain about the directive entirely. But it's
>>> true that we don't load it until later. Maybe try moving the
>>> IfVersion/Lockfile stanza down below the mod_version LoadModule
>>> line?
>> 
>> Apache is apparently overly accepting.  Moving the IfVersion below all
>> the IfModules fixes it.
> 
> Cool. I think the patch should look like the one below, then.

Basically identical to what I've done, you're just faster to the actual patch.  
:-D

+1

> Just to double-check that I have explained the issue correctly, can you
> share the output of "apache2 -l"? Mine has:
> 
>  $ apache2 -l
>  Compiled in modules:
>core.c
>mod_log_config.c
>mod_logio.c
>mod_version.c
>prefork.c
>http_core.c
>mod_so.c
> 
> which explains why it works here. I'm assuming you will not have
> mod_version.c compiled in.

Indeed I do not.

 $ httpd -l
 Compiled in modules:
   core.c
   prefork.c
   http_core.c
   mod_so.c

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html