svn-3524

Version:

1.7.0

Bug Link:

http://subversion.tigris.org/issues/show_bug.cgi?id=3524

Patch Link:

source code patch: http://svn.apache.org/viewvc?diff_format=h&view=revision&revision=880567

Symptom:

Attempting to lock a WC path which doesn’t exist at HEAD failed. In this case, it triggers an SVN_ERR_ASSERT over the ‘ra_serf’ module

NOTE:

Assertion in SVN is enabled by default even in production release!

How it is diagnosed:

source code analysis / discussion / dump

 

Error message with the module ‘neon’:

svn: Server sent unexpected return value (405 Method Not Allowed) in response to

LOCK request for '/repos/svn/branches/1.6.x/TODO-1.6'

 

Error message with the module ‘ra_serf’:

[debugging message] ..\..\..\subversion\libsvn_client\ra.c:275: (apr_err=235000)

svn: In file '..\..\..\subversion\libsvn_ra_serf\util.c' line 1120: assertion failed (ctx->status_code)

]]]

 

apr_status_t

svn_ra_serf__handle_xml_parser(serf_request_t *request,

                                   serf_bucket_t *response,

                                   void *baton,

                                   apr_pool_t *pool)

{

  …

  while (1)

        {

          …

          xml_status = XML_Parse(ctx->xmlp, data, len, 0);

          if (xml_status == XML_STATUS_ERROR && ctx->ignore_errors == FALSE)

            {

              …

          /* ctx->status_code = 0 , so it just prints out error message and kill the program. It should NOT be 0! */

          /* This ctx->status_code is actually a POINTER! And it will be derefernced!*/

              SVN_ERR_ASSERT_NO_RETURN(ctx->status_code);

              …

            }

          …

          }

}

 

Root Cause:

Brief:

When ‘405 error’ results from ‘svn lock’, we need to return an error with the appropriate error type, “SVN_ERR_FS_OUT_OF_DATE”.

Detail:

/* The error message is printed in the function, ‘svn_ra_serf__handle_xml_parser’.

This function is called from ‘handle_lock’. */

 

static apr_status_t

handle_lock(...)

{

  ...

  if (ctx->read_headers == FALSE)

        {

          ...

          ctx->status_code = sl.code;

          ctx->reason = sl.reason;

 

          /* 423 == Locked */

          if (sl.code == 423)

            {

              ctx->error = svn_ra_serf__handle_server_error(request, response,

                                                        pool);

 

              /* Older servers may not give a descriptive error. */

              if (!ctx->error)

                {

                  ctx->error = svn_error_createf(SVN_ERR_FS_PATH_ALREADY_LOCKED,

                                             NULL,

                                             _("Lock request failed: %d %s"),

                                             ctx->status_code, ctx->reason);

                }

              return ctx->error->apr_err;

            }

        }

 

  /* Forbidden when a lock doesn't exist. */

  if (ctx->status_code == 403)

        {

          status = svn_ra_serf__handle_discard_body(request, response, NULL, pool);

          if (APR_STATUS_IS_EOF(status))

            {

              ctx->done = TRUE;

              ctx->error = svn_error_createf(SVN_ERR_RA_DAV_REQUEST_FAILED, NULL,

                                         _("Lock request failed: %d %s"),

                                             ctx->status_code, ctx->reason);

            }

        }

  else

        {

/* only error code 403 is handled so that ‘svn_ra_serf__handle_xml_parser’ is called when error code 405 */

          status = svn_ra_serf__handle_xml_parser(request, response, handler_baton,

                                              pool);

        }

 

  return status;

}

 

The patch:

 

--- subversion/trunk/subversion/libsvn_ra_serf/locks.c          2009/11/12 23:37:51              880566

+++ subversion/trunk/subversion/libsvn_ra_serf/locks.c   2009/11/13 02:37:57              880567

@@ -383,6 +383,14 @@ handle_lock(serf_request_t *request,

               return err;

             }

 

+          /* 405 == Method Not Allowed (Occurs when trying to lock a working

+             copy path which no longer exists at HEAD in the repository. */

+          if (sl.code == 405)

+            return svn_error_createf(SVN_ERR_FS_OUT_OF_DATE,

+                                     NULL,

+                                     _("Lock request failed: %d %s"),

+                                   ctx->status_code, ctx->reason);

+

 

/* in case of error code 405, returns the appropriate error message instead of keep going */

           headers = serf_bucket_response_get_headers(response);

 

           val = serf_bucket_headers_get(headers, SVN_DAV_LOCK_OWNER_HEADER);

Failure symptom category

Assertion failed! Crash (just send ABORT signal)

Is there any log message?

Yes

 

How can we automatically insert the log message?

Memory safety check.