When attempting to unlock a resource with a token that the

resource hasn't been locked with, we should return an error
instead of a 20X response.  See
http://lists.w3.org/Archives/Public/w3c-dist-auth/2001JanMar/0099.html
for rationale.

Prior to Zope 2.11, we returned a 204 under this circumstance.
We choose do what mod_dav does, which is return a '400 Bad
Request' error.

This was caught by litmus locks.notowner_lock test #10.

doc/CHANGES.txt

@@ -97,6 +97,15 @@ Zope Changes
 
     Bugs Fixed
 
+      - DAV: When a client attempted to unlock a resource with a token
+        that the resource hadn't been locked with, in the past we
+        returned a 204 response.  This was incorrect.  The "correct"
+        behavior is to do what mod_dav does, which is return a '400
+        Bad Request' error.  This was caught by litmus
+        locks.notowner_lock test #10.  See
+        http://lists.w3.org/Archives/Public/w3c-dist-auth/2001JanMar/0099.html
+        for further rationale.
+
       - When Zope properties were set via DAV in the "null" namespace
         (xmlns="") a subsequent PROPFIND for the property would cause the 
         XML representation for that property to show a namespace of 

lib/python/webdav/davcmds.py

@@ -437,14 +437,18 @@ class Unlock:
         islockable = IWriteLock.providedBy(obj) or \
                      WriteLockInterface.isImplementedBy(obj)
 
-        if islockable and obj.wl_hasLock(token):
-            method = getattr(obj, 'wl_delLock')
-            vld = getSecurityManager().validate(None,obj,'wl_delLock',method)
-            if vld:
-                obj.wl_delLock(token)
+        if islockable:
+            if obj.wl_hasLock(token):
+                method = getattr(obj, 'wl_delLock')
+                vld = getSecurityManager().validate(None,obj,
+                                                    'wl_delLock',method)
+                if vld:
+                    obj.wl_delLock(token)
+                else:
+                    errmsg = "403 Forbidden"
             else:
-                errmsg = "403 Forbidden"
-        elif not islockable:
+                errmsg = '400 Bad Request'
+        else:
             # Only set an error message if the command is being applied
             # to a top level object.  Otherwise, we're descending a tree
             # which may contain many objects that don't implement locking,
@@ -476,7 +480,7 @@ class Unlock:
                     uri = urljoin(url, absattr(ob.getId()))
                     self.apply(ob, token, uri, result, top=0)
         if not top:
-            return result
+            return result.getvalue()
         if result.getvalue():
             # One or more subitems probably failed, so close the multistatus
             # element and clear out all succesful unlocks

lib/python/webdav/tests/test_davcmds.py

+import unittest
+
+class TestUnlock(unittest.TestCase):
+
+    def _getTargetClass(self):
+        from webdav.davcmds import Unlock
+        return Unlock
+
+    def _makeOne(self):
+        klass = self._getTargetClass()
+        return klass()
+
+    def _makeLockable(self, locktoken):
+        from webdav.interfaces import IWriteLock
+        from zope.interface import implements
+        class Lockable:
+            implements(IWriteLock)
+            def __init__(self, token):
+                self.token = token
+            def wl_hasLock(self, token):
+                return self.token == token
+        return Lockable(locktoken)
+
+    def test_apply_nontop_resource_returns_string(self):
+        """ When top=0 in unlock constructor, prior to Zope 2.11, the
+        unlock.apply method would return a StringIO.  This was bogus
+        because a StringIO cannot be used as a response body via the
+        standard RESPONSE.setBody() command.  Only strings or objects
+        with an asHTML method may be passed into setBody()."""
+
+        inst = self._makeOne()
+        lockable = self._makeLockable(None)
+        result = inst.apply(lockable, 'bogus',
+                            url='http://example.com/foo/UNLOCK', top=0)
+        self.failUnless(isinstance(result, str))
+
+    def test_apply_bogus_lock(self):
+        """
+        When attempting to unlock a resource with a token that the
+        resource hasn't been locked with, we should return an error
+        instead of a 20X response.  See
+        http://lists.w3.org/Archives/Public/w3c-dist-auth/2001JanMar/0099.html
+        for rationale.
+
+        Prior to Zope 2.11, we returned a 204 under this circumstance.
+        We choose do what mod_dav does, which is return a '400 Bad
+        Request' error.
+        
+        This was caught by litmus locks.notowner_lock test #10.
+        """
+        inst = self._makeOne()
+        lockable = self._makeLockable(None)
+        result = inst.apply(lockable, 'bogus',
+                            url='http://example.com/foo/UNLOCK', top=0)
+        self.assertNotEqual(
+            result.find('<d:status>HTTP/1.1 400 Bad Request</d:status>'),
+            -1)
+
+def test_suite():
+    return unittest.TestSuite((
+        unittest.makeSuite(TestUnlock),
+        ))
+
+if __name__ == '__main__':
+    unittest.main(defaultTest='test_suite')
+