[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] overflow check in ra_serf + malfunction -> error

From: Stefan Sperling <stsp_at_elego.de>
Date: Thu, 11 Jul 2013 12:47:55 +0200

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

This is an archived mail posted to the Subversion Dev mailing list.