You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by rh...@apache.org on 2014/03/20 11:59:21 UTC

svn commit: r1579588 - in /subversion/trunk/subversion: libsvn_ra_serf/get_lock.c libsvn_ra_serf/lock.c tests/cmdline/lock_tests.py

Author: rhuijben
Date: Thu Mar 20 10:59:21 2014
New Revision: 1579588

URL: http://svn.apache.org/r1579588
Log:
Make ra serf properly parse lock timeouts installed by generic dav clients.

* subversion/libsvn_ra_serf/get_lock.c
  (locks_closed): Properly parse timout format. This is the actual fix.

* subversion/libsvn_ra_serf/lock.c
  (locks_closed): Properly parse timeout format from the server response of
    the LOCK requests.

* subversion/tests/cmdline/lock_tests.py
  (dav_lock_timeout): New test, creating timeout lock via http request.
  (test_list): Add test.

This issue was
Found by: Sergey Raevskiy <sergey.raevskiy{_AT_}visualsvn.com>
(but the patch from his mail doesn't fix the issue on trunk.
 It might have worked around the problem on 1.7.x and 1.8.x)

Modified:
    subversion/trunk/subversion/libsvn_ra_serf/get_lock.c
    subversion/trunk/subversion/libsvn_ra_serf/lock.c
    subversion/trunk/subversion/tests/cmdline/lock_tests.py

Modified: subversion/trunk/subversion/libsvn_ra_serf/get_lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/get_lock.c?rev=1579588&r1=1579587&r2=1579588&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/get_lock.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/get_lock.c Thu Mar 20 10:59:21 2014
@@ -155,11 +155,20 @@ locks_closed(svn_ra_serf__xml_estate_t *
 
   if (leaving_state == TIMEOUT)
     {
-      if (strcmp(cdata->data, "Infinite") == 0)
+      if (strcasecmp(cdata->data, "Infinite") == 0)
         lock_ctx->lock->expiration_date = 0;
+      else if (strncasecmp(cdata->data, "Second-", 7) == 0)
+        {
+          unsigned n;
+          SVN_ERR(svn_cstring_atoui(&n, cdata->data+7));
+
+          lock_ctx->lock->expiration_date = apr_time_now() +
+                                            apr_time_from_sec(n);
+        }
       else
-        SVN_ERR(svn_time_from_cstring(&lock_ctx->lock->creation_date,
-                                      cdata->data, lock_ctx->pool));
+         return svn_error_createf(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
+                                  _("Invalid LOCK timeout value '%s'"),
+                                  cdata->data);
     }
   else if (leaving_state == HREF)
     {

Modified: subversion/trunk/subversion/libsvn_ra_serf/lock.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/lock.c?rev=1579588&r1=1579587&r2=1579588&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_ra_serf/lock.c (original)
+++ subversion/trunk/subversion/libsvn_ra_serf/lock.c Thu Mar 20 10:59:21 2014
@@ -148,11 +148,22 @@ locks_closed(svn_ra_serf__xml_estate_t *
 
   if (leaving_state == TIMEOUT)
     {
-      if (strcmp(cdata->data, "Infinite") == 0)
+      /* This function just parses the result of our own lock request,
+         so on a normal server we will only encounter 'Infinite' here. */
+      if (strcasecmp(cdata->data, "Infinite") == 0)
         lock_ctx->lock->expiration_date = 0;
+      else if (strncasecmp(cdata->data, "Second-", 7) == 0)
+        {
+          unsigned n;
+          SVN_ERR(svn_cstring_atoui(&n, cdata->data+7));
+
+          lock_ctx->lock->expiration_date = apr_time_now() +
+                                            apr_time_from_sec(n);
+        }
       else
-        SVN_ERR(svn_time_from_cstring(&lock_ctx->lock->creation_date,
-                                      cdata->data, lock_ctx->pool));
+         return svn_error_createf(SVN_ERR_RA_DAV_MALFORMED_DATA, NULL,
+                                  _("Invalid LOCK timeout value '%s'"),
+                                  cdata->data);
     }
   else if (leaving_state == HREF)
     {

Modified: subversion/trunk/subversion/tests/cmdline/lock_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/lock_tests.py?rev=1579588&r1=1579587&r2=1579588&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/lock_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/lock_tests.py Thu Mar 20 10:59:21 2014
@@ -2028,6 +2028,56 @@ def break_delete_add(sbox):
   expected_status.tweak('A/mu', status='  ', wc_rev=3)
   svntest.actions.run_and_verify_status(wc_dir, expected_status)
 
+@SkipUnless(svntest.main.is_ra_type_dav)
+def dav_lock_timeout(sbox):
+  "unlock a lock with timeout"
+
+  import httplib
+  from urlparse import urlparse
+  import base64
+
+  sbox.build()
+  loc = urlparse(sbox.repo_url)
+
+  if loc.scheme == 'http':
+    h = httplib.HTTPConnection(loc.hostname, loc.port)
+  else:
+    h = httplib.HTTPSConnection(loc.hostname, loc.port)
+
+  lock_body = '<?xml version="1.0" encoding="utf-8" ?>' \
+              '<D:lockinfo xmlns:D="DAV:">' \
+              '  <D:lockscope><D:exclusive/></D:lockscope>' \
+              '  <D:locktype><D:write/></D:locktype>' \
+              '  <D:owner>' \
+              '       <D:href>http://a/test</D:href>' \
+              '  </D:owner>' \
+              '</D:lockinfo>'
+
+  lock_headers = {
+    'Authorization': 'Basic ' + base64.b64encode('jconstant:rayjandom'),
+    'Timeout': 'Second-86400'
+  }
+
+  # Enabling the following line makes this test easier to debug
+  h.set_debuglevel(9)
+
+  h.request('LOCK', sbox.repo_url + '/iota', lock_body, lock_headers)
+
+  r = h.getresponse()
+
+  # Verify that there is a lock, by trying to obtain one
+  svntest.actions.run_and_verify_svn2(None, None, ".*locked by user", 0,
+                                      'lock', '-m', '', sbox.ospath('iota'))
+
+  # Before this patch this used to fail with a parse error of the timeout
+  svntest.actions.run_and_verify_svn2(None, None, ".*Unlock.*iota' failed", 0,
+                                     'unlock', sbox.repo_url + '/iota')
+
+  svntest.actions.run_and_verify_svn(None, None, [],
+                                     'unlock', sbox.ospath('iota'), '--force')
+
+
+
 ########################################################################
 # Run the tests
 
@@ -2085,6 +2135,7 @@ test_list = [ None,
               lock_hook_messages,
               failing_post_hooks,
               break_delete_add,
+              dav_lock_timeout,
             ]
 
 if __name__ == '__main__':