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