Re: SEGFAULT committing against tigris.org with HEAD client code.

[C. Michael Pilato]

On 06/15/2012 03:40 AM, C. Michael Pilato wrote:
> I noticed tonight that my attempts to commit to the ViewVC repository on
> Tigris.org are now failing due to a client-side SEGFAULT.  So far, all I
> know is that the failure is due to a call to svn_ra_serf__set_prop() (at
> subversion/libsvn_ra_serf/commit.c:1470) with a NULL path.  Because
> Tigris.org is running 1.6.x code, this path comes via the !HTTPv2 branch of
> the preceding logic, which deals with CHECKOUT requests and such.
> Ultimately, it's a NULL baseline URL that's passed to the __set_prop()
> function, eventually leading to the SEGFAULT in APR's hash code (NULL hash
> keys aren't allowed).
> 
> This code has worked as recently as yesterday, as I was committing
> successfully to ViewVC at that time.

More on this:

There is a NULL baseline URL because in handle_checkout(), this check:

  if (handler->done && handler->sline.code == 201)

fails due to handler->done having a value of 0.  This prevent the code from
harvesting the Location: header value, in which is stored the very baseline
URL we need.

The attached patch allowed me to get back to committing again.  It removes a
bunch of code that appears to be already encapsulated elsewhere (which is a
good thing!), but it also completely disregards the handler->done check.  I
suspect that this latter change is wrong, and that the bug simply reveals
some other ra_serf state management problem.

-- 
C. Michael Pilato <cmpil...@collab.net>
CollabNet   <>   www.collab.net   <>   Enterprise Cloud Development

* subversion/libsvn_ra_serf/commit.c
  (handle_checkout): Use svn_ra_serf__response_get_location() to do
    work that was done inline here.  Also, ignore handler->done (which
    is probably the wrong thing to do).

Index: subversion/libsvn_ra_serf/commit.c
===================================================================
--- subversion/libsvn_ra_serf/commit.c  (revision 1350441)
+++ subversion/libsvn_ra_serf/commit.c  (working copy)
@@ -297,30 +297,17 @@
     return err;
 
   /* Get the resulting location. */
-  if (handler->done && handler->sline.code == 201)
+  if (handler->sline.code == 201)
     {
-      serf_bucket_t *hdrs;
-      apr_uri_t uri;
-      const char *location;
-      apr_status_t status;
-
-      hdrs = serf_bucket_response_get_headers(response);
-      location = serf_bucket_headers_get(hdrs, "Location");
-      if (!location)
+      const char *location = 
+        svn_ra_serf__response_get_location(response, ctx->result_pool);
+      if (! location)
         return svn_error_create(SVN_ERR_RA_DAV_MALFORMED_DATA, err,
                                 _("No Location header received"));
-
-      status = apr_uri_parse(pool, location, &uri);
-
-      if (status)
-        err = svn_error_compose_create(svn_error_wrap_apr(status, NULL), err);
-
-      SVN_ERR_ASSERT(ctx->result_pool != NULL);
-      ctx->resource_url = svn_urlpath__canonicalize(uri.path,
-                                                    ctx->result_pool);
+      ctx->resource_url = location;
     }
 
-  return err;
+  return SVN_NO_ERROR;
 }
 

signature.asc
Description: OpenPGP digital signature