On 06.02.2018 18:35, Branko Čibej wrote:
> Earlier today, Julian noted the following on IRC:
>
> $ svn ls http://svn.apache.org/foo
> svn: E170013: Unable to connect to a repository at URL
> 'http://svn.apache.org/foo'
> svn: E175009: The XML response contains invalid XML
> svn: E130003: Malformed XML: no element found
>
>
> While the first error (Unable to connect ...) makes sense, the errors
> about invalid XML do not. It turns out that these errors are generated
> when ra_serf tries to parse the OPTIONS response during server
> capability negotiation:
>
> $ curl -i -X OPTIONS http://svn.apache.org/foo
> HTTP/1.1 200 OK
> Date: Tue, 06 Feb 2018 16:31:51 GMT
> Server: Apache/2.4.7 (Ubuntu)
> Allow: GET,HEAD,POST,OPTIONS
> Content-Length: 0
>
>
> It turns out that our Expat response handler does not check the response
> Content-Length header nor the Content-Type. I was going to add these
> checks so that we can generate better error messages, but I'm not too
> familiar with ra_serf so I wonder if there's a chance that adding these
> checks would break anything.
>
> As far as I can see, mod_dav_svn will always correctly set at least
> Content-Type (Content-Length is not relevant for chunked responses).
Seeing that the OPTIONS response is expected to be empty, I implemented
a simple check that we received any DAV headers in the response. The
error message now looks like this:
$ svn ls http://svn.apache.org/foo
svn: E170013: Unable to connect to a repository at URL
'http://svn.apache.org/foo'
svn: E175003: The server does not support the HTTP/DAV protocol
Ideas for better wording wold be most welcome.
-- Brane
Index: subversion/libsvn_ra_serf/options.c
===================================================================
--- subversion/libsvn_ra_serf/options.c (revision 1823355)
+++ subversion/libsvn_ra_serf/options.c (working copy)
@@ -71,6 +71,9 @@
svn_ra_serf__response_handler_t inner_handler;
void *inner_baton;
+ /* Have we received any DAV headers at all? */
+ svn_boolean_t dav_header_received;
+
const char *activity_collection;
svn_revnum_t youngest_rev;
@@ -165,6 +168,8 @@
apr_array_header_t *vals = svn_cstring_split(val, ",", TRUE,
opt_ctx->pool);
+ opt_ctx->dav_header_received = TRUE;
+
/* Right now we only have a few capabilities to detect, so just
seek for them directly. This could be written slightly more
efficiently, but that wouldn't be worth it until we have many
@@ -396,6 +401,14 @@
serf_bucket_headers_do(hdrs, capabilities_headers_iterator_callback,
opt_ctx);
+ /* Bail out early if we're not talking to a DAV server. */
+ if (!opt_ctx->dav_header_received)
+ {
+ return svn_error_create(
+ SVN_ERR_RA_DAV_OPTIONS_REQ_FAILED, NULL,
+ _("The server does not support the HTTP/DAV protocol"));
+ }
+
/* Assume mergeinfo capability unsupported, if didn't receive information
about server or repository mergeinfo capability. */
if (!svn_hash_gets(session->capabilities, SVN_RA_CAPABILITY_MERGEINFO))
