Nice catch Kamesh!
Kamesh Jayachandran wrote:
> I got the crux of the problem.
>
> Our 'serf_response_handler_t' implementation is wrong.
>
> According to 'serf_response_handler_t' doc implementation can return
> either one of the following,
>
> APR_EAGAIN, APR_EOF, APR_SUCCESS.
>
> Our libsvn_ra_serf's implementation of
> 'serf_response_handler_t'(handle_response) returns arbitrary svn error
> codes and hence 'serf system' thinks that 'EOF on a request has not
> reached' and hence this failure.
>
Well, we do return other status codes, and those will be returned from
the call to serf_context_run, but as you found out, you shouldn't reuse
that connection anymore. I think there are a few places where we rely on
serf returning our particular error code. Probably that's why you found
some of the tests failing.
What you're trying to fix is how ra_serf handles invalid XML response
bodies. While this is a bug on its own, the real reason the tests are
failing is that the server returns invalid XML for a locations-segment
report on a non-existant node. Attached patch fixes mod_dav_svn to
return an empty (opening & closing tag) report in that case.
Lieven
Index: subversion/mod_dav_svn/reports/get-location-segments.c
===================================================================
--- subversion/mod_dav_svn/reports/get-location-segments.c (revision 32013)
+++ subversion/mod_dav_svn/reports/get-location-segments.c (working copy)
@@ -93,6 +93,7 @@
apr_status_t apr_err;
dav_svn__authz_read_baton arb;
struct location_segment_baton location_segment_baton;
+ svn_error_t *err;
if ((apr_err = ap_fprintf(output, bb, DAV_XML_HEADER DEBUG_CR
"<S:get-location-segments-report xmlns:S=\""
@@ -107,16 +108,21 @@
/* Do what we came here for. */
location_segment_baton.output = output;
location_segment_baton.bb = bb;
- SVN_ERR(svn_repos_node_location_segments(resource->info->repos->repos,
- path, peg_rev,
- start_rev, end_rev,
- location_segment_receiver,
- &location_segment_baton,
- dav_svn__authz_read_func(&arb),
- &arb, resource->pool));
- if ((apr_err = ap_fprintf(output, bb,
- "</S:get-location-segments-report>" DEBUG_CR)))
+ /* Cache any error here, so that we can output valid XML to the client. */
+ err = svn_repos_node_location_segments(resource->info->repos->repos,
+ path, peg_rev,
+ start_rev, end_rev,
+ location_segment_receiver,
+ &location_segment_baton,
+ dav_svn__authz_read_func(&arb),
+ &arb, resource->pool);
+
+ apr_err = ap_fprintf(output, bb,
+ "</S:get-location-segments-report>" DEBUG_CR);
+ SVN_ERR(err);
+
+ if (apr_err)
return svn_error_create(apr_err, 0, NULL);
return SVN_NO_ERROR;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-07-08 21:46:30 CEST