You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Dave Brown <da...@wandisco.com> on 2009/10/29 17:22:29 UTC
[PATCH] ensure mod_proxy present when mod_dav_svn configured for
mirroring
[[[
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
]]]
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=2412741
Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured
for mirroring
Posted by Dave Brown <da...@wandisco.com>.
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
Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured
for mirroring
Posted by Justin Erenkrantz <je...@apache.org>.
On Wed, Nov 11, 2009 at 12:41 PM, Philip Martin
<ph...@wandisco.com> wrote:
> Philip Martin <ph...@codematters.co.uk> writes:
>
>> Philip Martin <ph...@wandisco.com> writes:
>>
>>> but it doesn't contain the magic word dav_proxy_html.
>>>
>>> Should we add a similar check for dav_proxy_html?
>>
>> mod_proxy_html not dav_proxy_html
>
> Gah! That's mod_proxy_http. MOD and HTTP!
Yah, I was going to say - html made no sense there.
+1 for adding the check for mod_proxy_http. -- justin
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416663
Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured
for mirroring
Posted by Philip Martin <ph...@wandisco.com>.
Philip Martin <ph...@codematters.co.uk> writes:
> Philip Martin <ph...@wandisco.com> writes:
>
>> but it doesn't contain the magic word dav_proxy_html.
>>
>> Should we add a similar check for dav_proxy_html?
>
> mod_proxy_html not dav_proxy_html
Gah! That's mod_proxy_http. MOD and HTTP!
--
Philip
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416510
Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured
for mirroring
Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@wandisco.com> writes:
> but it doesn't contain the magic word dav_proxy_html.
>
> Should we add a similar check for dav_proxy_html?
mod_proxy_html not dav_proxy_html
--
Philip
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416434
Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured
for mirroring
Posted by Philip Martin <ph...@wandisco.com>.
Julian Foad <ju...@btopenworld.com> writes:
> C. Michael Pilato wrote:
>> Sorry, Dave, this patch (which I rather witnessed the conception of) skipped
>> past my radar somehow.
>>
>> +1 to commit, Julian (with the small request that a blank line precede the
>> newly added comment).
>
> Thanks, Mike.
>
> Committed revision 40385.
It's better, but still not enough on my box. Without mod_proxy I get
a 405:
svn: Server sent unexpected return value (405 Method Not Allowed) in response to MKACTIVITY request for '/slave/!svn/act/3e451258-0c27-4616-b515-1b4d1bbc0424'
but with mod_proxy and without mod_proxy_html I get a 500:
svn: Server sent unexpected return value (500 Internal Server Error) in response to MKACTIVITY request for '/slave/!svn/act/8cc2a957-2956-4ea2-8287-ade9c64bb873'
With the 500 I do get an error in error_log:
[Wed Nov 11 09:18:49 2009] [warn] proxy: No protocol handler was valid for the URL /slave/!svn/act/8cc2a957-2956-4ea2-8287-ade9c64bb873. If you are using a DSO version of mod_proxy, make sure the proxy submodules are included in the configuration using LoadModule.
but it doesn't contain the magic word dav_proxy_html.
Should we add a similar check for dav_proxy_html?
--
Philip
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2416431
Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured
for mirroring
Posted by Daniel Rall <dl...@finemaltcoding.com>.
On Thu, Nov 5, 2009 at 7:58 AM, Julian Foad <ju...@btopenworld.com> wrote:
> C. Michael Pilato wrote:
>> Sorry, Dave, this patch (which I rather witnessed the conception of) skipped
>> past my radar somehow.
>>
>> +1 to commit, Julian (with the small request that a blank line precede the
>> newly added comment).
>
> Thanks, Mike.
>
> Committed revision 40385.
Looks great.
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414979
Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured
for mirroring
Posted by Julian Foad <ju...@btopenworld.com>.
C. Michael Pilato wrote:
> Sorry, Dave, this patch (which I rather witnessed the conception of) skipped
> past my radar somehow.
>
> +1 to commit, Julian (with the small request that a blank line precede the
> newly added comment).
Thanks, Mike.
Committed revision 40385.
- Julian
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414756
Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured
for mirroring
Posted by "C. Michael Pilato" <cm...@collab.net>.
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.
>
> 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.
Sorry, Dave, this patch (which I rather witnessed the conception of) skipped
past my radar somehow.
+1 to commit, Julian (with the small request that a blank line precede the
newly added comment).
--
C. Michael Pilato <cm...@collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2414725
Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured
for mirroring
Posted by Julian Foad <ju...@btopenworld.com>.
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.
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 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=2414702