[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] ensure mod_proxy present when mod_dav_svn configured for mirroring

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Thu, 05 Nov 2009 13:03:47 +0000

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
Received on 2009-11-05 14:04:59 CET

This is an archived mail posted to the Subversion Dev mailing list.