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:
>
> DAV svn
> SVNPath /scratch/repo
>
> SVNMasterURI"http://master.server.com/svn";
> ...
>
>
> 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 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 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
Re: mod_dav_svn assert on root location
Erez Zarum writes: > I am trying to create a master slave configuration with proxy requests > through the slave, i have used this configuration on the slave: > > DAV svn > SVNPath /scratch/repo > > SVNMasterURI"http://master.server.com/svn"; > ... > > > In 1.7.7 the SVNMasterURI won't let me use http://master.server.com/ > in 1.6.19 it will. That was a deliberate change in r1064734. I'm not sure what went wrong with that setup but the log message is: Disallow attempts to proxy to the server root of a master server. It really just doesn't pan out well when you do. > But when i use 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 then everything works as expected. I've raised http://subversion.tigris.org/issues/show_bug.cgi?id=4272 for the assert. -- Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
Re: mod_dav_svn assert on root location
I have recompiled with the patch,
When trying to commit to the slave, the client receive:
"Commit failed (details follow):
Server sent unexpected return value (500 Internal Server Error) in response to
MKACTIVITY request for '/!svn/act/12aad366-69cf-2b48-9303-bfb1220af919'
And now the error is logged in the VirtualHost error log (as defined)
instead of the main apache error log.
[Wed Dec 12 16:47:33 2012] [error] [client 10.0.2.15] (125009)APR does
not understand this error code: Invalid URI segment
'!svn/act/779db2c6-6390-e147-a8be-cc8434b0919f' in slave fixup
As i understand it will be best to switch from using root dir to
something like "/svn".
Thanks for your help.
On Wed, Dec 12, 2012 at 3:55 PM, Daniel Shahaf wrote:
> 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:
>>
>> DAV svn
>> SVNPath /scratch/repo
>>
>> SVNMasterURI"http://master.server.com/svn";
>> ...
>>
>>
>> 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 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 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
Re: mod_dav_svn assert on root location
Philip Martin wrote on Wed, Dec 12, 2012 at 14:13:34 +: > Erez Zarum writes: > > > I am trying to create a master slave configuration with proxy requests > > through the slave, i have used this configuration on the slave: > > > > DAV svn > > SVNPath /scratch/repo > > > > SVNMasterURI"http://master.server.com/svn"; > > ... > > > > > > In 1.7.7 the SVNMasterURI won't let me use http://master.server.com/ > > in 1.6.19 it will. > > That was a deliberate change in r1064734. I'm not sure what went wrong > with that setup but the log message is: > > Disallow attempts to proxy to the server root of a master server. It > really just doesn't pan out well when you do. > > > But when i use 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 then everything works as expected. > > I've raised > http://subversion.tigris.org/issues/show_bug.cgi?id=4272 > for the assert. Do you plan to work on this, or should I apply the patch I posted elsethread? Even if it's not the best fix it does resolve an assertion, so we could apply it now and look at further improvements later.
Re: mod_dav_svn assert on root location
Daniel Shahaf writes: > Do you plan to work on this, or should I apply the patch I posted > elsethread? I'm not working on it. -- Certified & Supported Apache Subversion Downloads: http://www.wandisco.com/subversion/download
