Julian Foad wrote:
> On Thu, 2009-10-29, Dave Brown wrote:
>> [[[
>> In mod_dav_svn, when processing an SVNMasterURI directive,
>> check that mod_proxy is available in the httpd runtime.
>> Forwarding writes to a master from a slave requires
>> the mod_proxy handler, and when it isn't present, the
>> failure is ugly & opaque. Apache's core, default handler
>> sends back a "405 Not Allowed," for non-GET which looks
>> like an authz failure.
>>
>> BTW, there's precedent for using ap_find_linked_module()
>> to check that module dependencies are present. Namely,
>> mod_rewrite looks for mod_proxy, and mod_authnz_ldap looks
>> for util_ldap.
>>
>> * subversion/mod_dav_svn/mod_dav_svn.c
>> (SVNMasterURI_cmd): use ap_find_linked_module() to
>> ensure that mod_proxy is available
>> ]]]
>
> Hi Dave.
>
> As I understood your explanation to me the other day:
>
> The problem is if you try to use the DAV write-through proxy, but don't
> have mod_proxy installed, then Subversion would issue an obscure and
> unhelpful error message at the time of trying to use it.
Yes. The symptoms I saw were the same as what these guys were seeing on
this thread:
http://old.nabble.com/Problem-committing-to-a-write-thru-proxy-td20524286.html
And it's not at all obvious that "405" means "mod_proxy unavailable."
Further, you don't get mod_proxy if you configure httpd-2.2.x with
"most" shared modules (--enable-mods-shared=most). I don't know how
common configuring with "most" modules are, but it's how I ran into this.
My one concern was getting a false negative on the check that this patch
enables, i.e., mod_proxy is present in the configuration, but
ap_find_linked_module() doesn't find it for some reason. But again,
there's precendent in httpd's own modules for using ap_find_linked()
module in this way. I also tried with mod_proxy statically linked, with
the expected result as well.
>
> The fix is twofold: check for mod_proxy being installed as soon as we
> know we need it, and issue a nice error message.
>
> This sounds great, exactly the sort of improvement we need.
>
> If nobody thinks of a problem with it, I will commit it. I will try to
> test it first, but I know you have done so.
Thanks for looking at it. I think it's a righteous fix, but would love
any more confirmation we can get.
-dave
>
> Thanks for the enhancement!
> - Julian
>
>
>> Index: subversion/mod_dav_svn/mod_dav_svn.c
>> ===================================================================
>> --- subversion/mod_dav_svn/mod_dav_svn.c (revision 40267)
>> +++ subversion/mod_dav_svn/mod_dav_svn.c (working copy)
>> @@ -219,6 +219,12 @@
>> SVNMasterURI_cmd(cmd_parms *cmd, void *config, const char *arg1)
>> {
>> dir_conf_t *conf = config;
>> + /* Since SVNMasterURI requires mod_proxy's handler
>> + * (r->handler = "proxy-server" in mirror.c) make
>> + * sure it is present.
>> + */
>> + if (ap_find_linked_module("mod_proxy.c") == NULL)
>> + return "module mod_proxy not loaded, required for SVNMasterURI";
>>
>> conf->master_uri = apr_pstrdup(cmd->pool, arg1);
>
>
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414720
Received on 2009-11-05 15:23:00 CET