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;
}
Received on 2013-07-11 12:48:35 CEST