I'd like to change an SVN_ERR_MALFUNCTION() in ra_serf into an error return, because aborting doesn't help with diagnosing problems.
I also think that we should check for overflow in this case, since code further down depends on that not happening. Not very likely, since it's an off_t, but still. Is this patch correct? [[[ * subversion/libsvn_ra_serf/util.c (svn_ra_serf__handle_xml_parser): Check for read_size overflow and return an EOF error in case of aborting the process. ]]] Index: subversion/libsvn_ra_serf/util.c =================================================================== --- subversion/libsvn_ra_serf/util.c (revision 1502182) +++ subversion/libsvn_ra_serf/util.c (working copy) @@ -1645,6 +1645,12 @@ svn_ra_serf__handle_xml_parser(serf_request_t *req return svn_ra_serf__wrap_err(status, NULL); } + /* Check for overflow. */ + if (ctx->read_size + len < ctx->read_size) + return svn_error_createf(SVN_ERR_RA_DAV_REQUEST_FAILED, NULL, + _("Request response is too large (%" + APR_OFF_T_FMT " bytes already read)"), + ctx->read_size); ctx->read_size += len; if (ctx->skip_size) @@ -1654,17 +1660,13 @@ svn_ra_serf__handle_xml_parser(serf_request_t *req if (ctx->skip_size >= ctx->read_size) { - /* Eek. What did the file shrink or something? */ - if (APR_STATUS_IS_EOF(status)) + /* Eek. What did the file shrink or something? */ + if (APR_STATUS_IS_EOF(status) || APR_STATUS_IS_EAGAIN(status)) { - SVN_ERR_MALFUNCTION(); + return svn_ra_serf__wrap_err(status, NULL); } - /* Skip on to the next iteration of this loop. */ - if (APR_STATUS_IS_EAGAIN(status)) - { - return svn_ra_serf__wrap_err(status, NULL); - } + /* Skip on to the next iteration of this loop. */ continue; }