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;
             }
 

Reply via email to