Re: mod_dav_svn assert on root location

[Daniel Shahaf]

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