Re: mod_dav_svn assert on root location

Erez Zarum wrote on Wed, Dec 12, 2012 at 15:33:08 +0200:
> I am trying to create a master slave configuration with proxy requests
> through the slave, i have used this configuration on the slave:
> <Location />
> DAV svn
> SVNPath /scratch/repo
> ....
> SVNMasterURI "http://master.server.com/svn"
> ...
> </Location>
>
> In 1.7.7 the SVNMasterURI won't let me use http://master.server.com/
> in 1.6.19 it will.
> But when i use <Location /> i get the following assert in the apache
> error_log (the main log): "httpd: subversion/mod_dav_svn/mirror.c:47:
> proxy_request_fixup: Assertion `(uri_segment[0] == '\0') ||
> (uri_segment[0] == '/')' failed."
> If i use <Location /url> then everything works as expected.
>
> Thanks,
> Erez

Hi :)

Thanks for the due diligent report on IRC, it was helpful.

This should at least fix the assertion (compiled, but untested):

[[[
Index: subversion/mod_dav_svn/mirror.c
===================================================================
--- subversion/mod_dav_svn/mirror.c (revision 1420650)
+++ subversion/mod_dav_svn/mirror.c (working copy)
@@ -39,12 +39,17 @@
    URI_SEGMENT is the URI bits relative to the repository root (but if
    non-empty, *does* have a leading slash delimiter).
    MASTER_URI and URI_SEGMENT are not URI-encoded. */
-static void proxy_request_fixup(request_rec *r,
- const char *master_uri,
- const char *uri_segment)
+static int proxy_request_fixup(request_rec *r,
+ const char *master_uri,
+ const char *uri_segment)
 {
- assert((uri_segment[0] == '\0')
- || (uri_segment[0] == '/'));
+ if (uri_segment[0] != '\0' && uri_segment[0] != '/')
+ {
+ ap_log_rerror(APLOG_MARK, APLOG_ERR, SVN_ERR_BAD_CONFIG_VALUE, r,
+ "Invalid URI segment '%s' in slave fixup",
+ uri_segment);
+ return HTTP_INTERNAL_SERVER_ERROR;
+ }
 
     r->proxyreq = PROXYREQ_REVERSE;
     r->uri = r->unparsed_uri;
@@ -67,6 +72,7 @@
     ap_add_output_filter("LocationRewrite", NULL, r, r->connection);
     ap_add_output_filter("ReposRewrite", NULL, r, r->connection);
     ap_add_input_filter("IncomingRewrite", NULL, r, r->connection);
+ return OK;
 }
 
 
@@ -101,8 +107,10 @@ int dav_svn__proxy_request_fixup(request_rec *r)
                                                     "/txn/", (char *)NULL))
                     || ap_strstr_c(seg, apr_pstrcat(r->pool, special_uri,
                                                     "/txr/", (char *)NULL))) {
+ int rv;
                     seg += strlen(root_dir);
- proxy_request_fixup(r, master_uri, seg);
+ rv = proxy_request_fixup(r, master_uri, seg);
+ if (rv) return rv;
                 }
             }
             return OK;
@@ -116,8 +124,10 @@ int dav_svn__proxy_request_fixup(request_rec *r)
                     r->method_number == M_LOCK ||
                     r->method_number == M_UNLOCK ||
                     ap_strstr_c(seg, special_uri))) {
+ int rv;
             seg += strlen(root_dir);
- proxy_request_fixup(r, master_uri, seg);
+ rv = proxy_request_fixup(r, master_uri, seg);
+ if (rv) return rv;
             return OK;
         }
     }
]]]

I think the actual problem is that root_dir is "/", so after skipping
strlen(root_dir) bytes the result doesn't start with a slash. We could
fix that by using svn_uri__skip_ancestor()... but we already don't allow
SVNMasterURI to be a http://host:port/ URL (i.e., lacking /path
componets after the root), so maybe we shouldn't bother to try
supporting root_dir == "/" here.

Thoughts?

Daniel
Received on 2012-12-12 14:56:29 CET