[Pkg-owncloud-commits] [php-sabredav] 79/148: Simplified the locking plugin.

David Prévot taffit at moszumanska.debian.org
Wed Apr 15 01:37:18 UTC 2015


This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to branch master
in repository php-sabredav.

commit cc12f74e7016c7ed4331d1f75cff0efa9bf92b53
Author: Evert Pot <me at evertpot.com>
Date:   Fri Mar 20 23:03:40 2015 -0400

    Simplified the locking plugin.
    
    * Locks backend is no longer optional.
    * For the first time fully unittested.
    * Fixed some edge cases.
---
 CHANGELOG.md                           |   9 +-
 lib/DAV/Locks/Plugin.php               |  68 +++++-------
 lib/DAV/Xml/Property/SupportedLock.php |  20 ----
 tests/Sabre/DAV/Locks/PluginTest.php   | 188 +++++++++++++++++----------------
 4 files changed, 134 insertions(+), 151 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 9f89081..900812c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,12 +5,19 @@ ChangeLog
 -------------------------
 
 * Contains all the changes introduced between 2.1.2 and 2.1.3.
-* Upgraded to sabre/http 4.0.0alpha1 and now also using sabre/uri.
+* Complete rewrite of the XML system. We now use our own [sabre/xml][xml],
+  which has a much smarter XML Reader and Writer.
+* BC Break: It's no longer possible to instantiate the Locks plugin without
+  a locks backend. I'm not sure why this ever made sense.
+* Simplified the Locking system and fixed a bug related to if tokens checking
+  locks unrelated to the current request.
 * The zip release ships with [sabre/vobject 3.4.2][vobj],
   [sabre/http 4.0.0-alpha1][http], [sabre/event 2.0.1][evnt] and
   [sabre/uri 1.0.0][uri].
 
 
+
+
 2.2.0-alpha2 (2015-01-09)
 -------------------------
 
diff --git a/lib/DAV/Locks/Plugin.php b/lib/DAV/Locks/Plugin.php
index e538cc3..112c7cd 100644
--- a/lib/DAV/Locks/Plugin.php
+++ b/lib/DAV/Locks/Plugin.php
@@ -42,7 +42,7 @@ class Plugin extends DAV\ServerPlugin {
      *
      * @param Backend\BackendInterface $locksBackend
      */
-    function __construct(Backend\BackendInterface $locksBackend = null) {
+    function __construct(Backend\BackendInterface $locksBackend) {
 
         $this->locksBackend = $locksBackend;
 
@@ -95,7 +95,7 @@ class Plugin extends DAV\ServerPlugin {
     function propFind(DAV\PropFind $propFind, DAV\INode $node) {
 
         $propFind->handle('{DAV:}supportedlock', function() {
-            return new DAV\Xml\Property\SupportedLock(!!$this->locksBackend);
+            return new DAV\Xml\Property\SupportedLock();
         });
         $propFind->handle('{DAV:}lockdiscovery', function() use ($propFind) {
             return new DAV\Xml\Property\LockDiscovery(
@@ -117,10 +117,7 @@ class Plugin extends DAV\ServerPlugin {
      */
     function getHTTPMethods($uri) {
 
-        if ($this->locksBackend)
-            return ['LOCK','UNLOCK'];
-
-        return [];
+        return ['LOCK','UNLOCK'];
 
     }
 
@@ -153,12 +150,7 @@ class Plugin extends DAV\ServerPlugin {
      */
     function getLocks($uri, $returnChildLocks = false) {
 
-        $lockList = [];
-
-        if ($this->locksBackend)
-            $lockList = array_merge($lockList,$this->locksBackend->getLocks($uri, $returnChildLocks));
-
-        return $lockList;
+        return $this->locksBackend->getLocks($uri, $returnChildLocks);
 
     }
 
@@ -346,9 +338,7 @@ class Plugin extends DAV\ServerPlugin {
     function lockNode($uri,LockInfo $lockInfo) {
 
         if (!$this->server->emit('beforeLock', [$uri,$lockInfo])) return;
-
-        if ($this->locksBackend) return $this->locksBackend->lock($uri,$lockInfo);
-        throw new DAV\Exception\MethodNotAllowed('Locking support is not enabled for this resource. No Locking backend was found so if you didn\'t expect this error, please check your configuration.');
+        return $this->locksBackend->lock($uri,$lockInfo);
 
     }
 
@@ -364,7 +354,7 @@ class Plugin extends DAV\ServerPlugin {
     function unlockNode($uri, LockInfo $lockInfo) {
 
         if (!$this->server->emit('beforeUnlock', [$uri,$lockInfo])) return;
-        if ($this->locksBackend) return $this->locksBackend->unlock($uri,$lockInfo);
+        return $this->locksBackend->unlock($uri,$lockInfo);
 
     }
 
@@ -510,33 +500,33 @@ class Plugin extends DAV\ServerPlugin {
 
                     }
 
-                    // If we got here, it means that there was a
-                    // lock-token, but it was not in 'mustLocks'.
-                    //
-                    // This is an edge-case, as it could mean that token
-                    // was specified with a url that was not 'required' to
-                    // check. So we're doing one extra lookup to make sure
-                    // we really don't know this token.
-                    //
-                    // This also gets triggered when the user specified a
-                    // lock-token that was expired.
-                    $oddLocks = $this->getLocks($condition['uri']);
-                    foreach($oddLocks as $oddLock) {
-
-                        if ($oddLock->token === $checkToken) {
-
-                            // We have a hit!
-                            $conditions[$kk]['tokens'][$ii]['validToken'] = true;
-                            continue 2;
-
-                        }
-                    }
+                }
 
-                    // If we get all the way here, the lock-token was
-                    // really unknown.
+                // If we got here, it means that there was a
+                // lock-token, but it was not in 'mustLocks'.
+                //
+                // This is an edge-case, as it could mean that token
+                // was specified with a url that was not 'required' to
+                // check. So we're doing one extra lookup to make sure
+                // we really don't know this token.
+                //
+                // This also gets triggered when the user specified a
+                // lock-token that was expired.
+                $oddLocks = $this->getLocks($condition['uri']);
+                foreach($oddLocks as $oddLock) {
+
+                    if ($oddLock->token === $checkToken) {
+
+                        // We have a hit!
+                        $conditions[$kk]['tokens'][$ii]['validToken'] = true;
+                        continue 2;
 
+                    }
                 }
 
+                // If we get all the way here, the lock-token was
+                // really unknown.
+
 
             }
 
diff --git a/lib/DAV/Xml/Property/SupportedLock.php b/lib/DAV/Xml/Property/SupportedLock.php
index 4bd37ba..4d38104 100644
--- a/lib/DAV/Xml/Property/SupportedLock.php
+++ b/lib/DAV/Xml/Property/SupportedLock.php
@@ -22,24 +22,6 @@ use Sabre\Xml\XmlSerializable;
 class SupportedLock implements XmlSerializable {
 
     /**
-     * supportsLocks
-     *
-     * @var bool
-     */
-    public $supportsLocks = false;
-
-    /**
-     * __construct
-     *
-     * @param bool $supportsLocks
-     */
-    function __construct($supportsLocks) {
-
-        $this->supportsLocks = $supportsLocks;
-
-    }
-
-    /**
      * The xmlSerialize metod is called during xml writing.
      *
      * Use the $writer argument to write its own xml serialization.
@@ -60,8 +42,6 @@ class SupportedLock implements XmlSerializable {
      */
     function xmlSerialize(Writer $writer) {
 
-        if (!$this->supportsLocks) return null;
-
         $writer->writeElement('{DAV:}lockentry', [
             '{DAV:}lockscope' => ['{DAV:}exclusive' => null],
             '{DAV:}locktype' =>  ['{DAV:}write' => null],
diff --git a/tests/Sabre/DAV/Locks/PluginTest.php b/tests/Sabre/DAV/Locks/PluginTest.php
index 4753610..89ff25f 100644
--- a/tests/Sabre/DAV/Locks/PluginTest.php
+++ b/tests/Sabre/DAV/Locks/PluginTest.php
@@ -36,24 +36,10 @@ class PluginTest extends DAV\AbstractServer {
 
     }
 
-    function testGetHTTPMethodsNoBackend() {
-
-        $locksPlugin = new Plugin();
-        $this->server->addPlugin($locksPlugin);
-        $this->assertEquals(array(),$locksPlugin->getHTTPMethods(''));
-
-    }
-
     function testLockNoBody() {
 
-        $serverVars = array(
-            'REQUEST_URI'    => '/test.txt',
-            'REQUEST_METHOD' => 'LOCK',
-        );
-
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
-        $request->setBody('');
-        $this->server->httpRequest = ($request);
+        $request = new HTTP\Request('LOCK', '/test.txt');
+        $this->server->httpRequest = $request;
         $this->server->exec();
 
         $this->assertEquals(array(
@@ -69,12 +55,7 @@ class PluginTest extends DAV\AbstractServer {
 
     function testLock() {
 
-        $serverVars = array(
-            'REQUEST_URI'    => '/test.txt',
-            'REQUEST_METHOD' => 'LOCK',
-        );
-
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
+        $request = new HTTP\Request('LOCK', '/test.txt');
         $request->setBody('<?xml version="1.0"?>
 <D:lockinfo xmlns:D="DAV:">
     <D:lockscope><D:exclusive/></D:lockscope>
@@ -131,12 +112,7 @@ class PluginTest extends DAV\AbstractServer {
      */
     function testDoubleLock() {
 
-        $serverVars = array(
-            'REQUEST_URI'    => '/test.txt',
-            'REQUEST_METHOD' => 'LOCK',
-        );
-
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
+        $request = new HTTP\Request('LOCK', '/test.txt');
         $request->setBody('<?xml version="1.0"?>
 <D:lockinfo xmlns:D="DAV:">
     <D:lockscope><D:exclusive/></D:lockscope>
@@ -198,14 +174,44 @@ class PluginTest extends DAV\AbstractServer {
     /**
      * @depends testLock
      */
-    function testLockNoFile() {
+    function testLockRefreshBadToken() {
 
-        $serverVars = array(
-            'REQUEST_URI'    => '/notfound.txt',
-            'REQUEST_METHOD' => 'LOCK',
-        );
+        $request = new HTTP\Request('LOCK', '/test.txt');
+        $request->setBody('<?xml version="1.0"?>
+<D:lockinfo xmlns:D="DAV:">
+    <D:lockscope><D:exclusive/></D:lockscope>
+    <D:locktype><D:write/></D:locktype>
+    <D:owner>
+        <D:href>http://example.org/~ejw/contact.html</D:href>
+    </D:owner>
+</D:lockinfo>');
 
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
+        $this->server->httpRequest = $request;
+        $this->server->exec();
+
+        $lockToken = $this->response->getHeader('Lock-Token');
+
+        $this->response = new HTTP\ResponseMock();
+        $this->server->httpResponse = $this->response;
+
+        $request = new HTTP\Request('LOCK', '/test.txt', ['If' => '(' . $lockToken . 'foobar) (<opaquelocktoken:anotherbadtoken>)' ]);
+        $request->setBody('');
+
+        $this->server->httpRequest = $request;
+        $this->server->exec();
+
+        $this->assertEquals('application/xml; charset=utf-8',$this->response->getHeader('Content-Type'));
+
+        $this->assertEquals(423, $this->response->getStatus(),'We received an incorrect status code. Full response body: ' . $this->response->getBody());
+
+    }
+
+    /**
+     * @depends testLock
+     */
+    function testLockNoFile() {
+
+        $request = new HTTP\Request('LOCK', '/notfound.txt');
         $request->setBody('<?xml version="1.0"?>
 <D:lockinfo xmlns:D="DAV:">
     <D:lockscope><D:exclusive/></D:lockscope>
@@ -230,19 +236,14 @@ class PluginTest extends DAV\AbstractServer {
      */
     function testUnlockNoToken() {
 
-        $serverVars = array(
-            'REQUEST_URI'    => '/test.txt',
-            'REQUEST_METHOD' => 'UNLOCK',
-        );
-
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
-        $this->server->httpRequest = ($request);
+        $request = new HTTP\Request('UNLOCK', '/test.txt');
+        $this->server->httpRequest = $request;
         $this->server->exec();
 
-        $this->assertEquals(array(
+        $this->assertEquals([
             'X-Sabre-Version' => [DAV\Version::VERSION],
             'Content-Type' => ['application/xml; charset=utf-8'],
-            ),
+            ],
             $this->response->getHeaders()
          );
 
@@ -255,20 +256,14 @@ class PluginTest extends DAV\AbstractServer {
      */
     function testUnlockBadToken() {
 
-        $serverVars = array(
-            'REQUEST_URI'     => '/test.txt',
-            'REQUEST_METHOD'  => 'UNLOCK',
-            'HTTP_LOCK_TOKEN' => '<opaquelocktoken:blablabla>',
-        );
-
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
-        $this->server->httpRequest = ($request);
+        $request = new HTTP\Request('UNLOCK', '/test.txt', ['Lock-Token' => '<opaquelocktoken:blablabla>']);
+        $this->server->httpRequest = $request;
         $this->server->exec();
 
-        $this->assertEquals(array(
+        $this->assertEquals([
             'X-Sabre-Version' => [DAV\Version::VERSION],
             'Content-Type' => ['application/xml; charset=utf-8'],
-            ),
+            ],
             $this->response->getHeaders()
          );
 
@@ -281,12 +276,7 @@ class PluginTest extends DAV\AbstractServer {
      */
     function testLockPutNoToken() {
 
-        $serverVars = array(
-            'REQUEST_URI'    => '/test.txt',
-            'REQUEST_METHOD' => 'LOCK',
-        );
-
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
+        $request = new HTTP\Request('LOCK', '/test.txt');
         $request->setBody('<?xml version="1.0"?>
 <D:lockinfo xmlns:D="DAV:">
     <D:lockscope><D:exclusive/></D:lockscope>
@@ -304,12 +294,7 @@ class PluginTest extends DAV\AbstractServer {
 
         $this->assertEquals(200, $this->response->status);
 
-        $serverVars = array(
-            'REQUEST_URI'    => '/test.txt',
-            'REQUEST_METHOD' => 'PUT',
-        );
-
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
+        $request = new HTTP\Request('PUT', '/test.txt');
         $request->setBody('newbody');
         $this->server->httpRequest = $request;
         $this->server->exec();
@@ -326,10 +311,7 @@ class PluginTest extends DAV\AbstractServer {
      */
     function testUnlock() {
 
-        $request = HTTP\Sapi::createFromServerArray([
-            'REQUEST_URI' => '/test.txt',
-            'REQUEST_METHOD' => 'LOCK',
-        ]);
+        $request = new HTTP\Request('LOCK', '/test.txt');
         $this->server->httpRequest = $request;
 
         $request->setBody('<?xml version="1.0"?>
@@ -344,22 +326,16 @@ class PluginTest extends DAV\AbstractServer {
         $this->server->invokeMethod($request, $this->server->httpResponse);
         $lockToken = $this->server->httpResponse->getHeader('Lock-Token');
 
-        $serverVars = array(
-            'HTTP_LOCK_TOKEN' => $lockToken,
-            'REQUEST_URI' => '/test.txt',
-            'REQUEST_METHOD' => 'UNLOCK',
-        );
-
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
+        $request = new HTTP\Request('UNLOCK', '/test.txt', ['Lock-Token' => $lockToken]);
         $this->server->httpRequest = $request;
         $this->server->httpResponse = new HTTP\ResponseMock();
         $this->server->invokeMethod($request, $this->server->httpResponse);
 
         $this->assertEquals(204,$this->server->httpResponse->status,'Got an incorrect status code. Full response body: ' . $this->response->body);
-        $this->assertEquals(array(
+        $this->assertEquals([
             'X-Sabre-Version' => [DAV\Version::VERSION],
             'Content-Length' => ['0'],
-            ),
+            ],
             $this->server->httpResponse->getHeaders()
          );
 
@@ -371,10 +347,7 @@ class PluginTest extends DAV\AbstractServer {
      */
     function testUnlockWindowsBug() {
 
-        $request = HTTP\Sapi::createFromServerArray([
-            'REQUEST_URI' => '/test.txt',
-            'REQUEST_METHOD' => 'LOCK',
-        ]);
+        $request = new HTTP\Request('LOCK', '/test.txt');
         $this->server->httpRequest = $request;
 
         $request->setBody('<?xml version="1.0"?>
@@ -392,22 +365,16 @@ class PluginTest extends DAV\AbstractServer {
         // See Issue 123
         $lockToken = trim($lockToken,'<>');
 
-        $serverVars = array(
-            'HTTP_LOCK_TOKEN' => $lockToken,
-            'REQUEST_URI' => '/test.txt',
-            'REQUEST_METHOD' => 'UNLOCK',
-        );
-
-        $request = HTTP\Sapi::createFromServerArray($serverVars);
+        $request = new HTTP\Request('UNLOCK', '/test.txt', ['Lock-Token' => $lockToken]);
         $this->server->httpRequest = $request;
         $this->server->httpResponse = new HTTP\ResponseMock();
         $this->server->invokeMethod($request, $this->server->httpResponse);
 
         $this->assertEquals(204, $this->server->httpResponse->status,'Got an incorrect status code. Full response body: ' . $this->response->body);
-        $this->assertEquals(array(
+        $this->assertEquals([
             'X-Sabre-Version' => [DAV\Version::VERSION],
             'Content-Length' => ['0'],
-            ),
+            ],
             $this->server->httpResponse->getHeaders()
          );
 
@@ -877,6 +844,45 @@ class PluginTest extends DAV\AbstractServer {
 
     }
 
+    /**
+     * @depends testLock
+     */
+    function testLockPutUnrelatedToken() {
+
+        $request = new HTTP\Request('LOCK', '/unrelated.txt');
+        $request->setBody('<?xml version="1.0"?>
+<D:lockinfo xmlns:D="DAV:">
+    <D:lockscope><D:exclusive/></D:lockscope>
+    <D:locktype><D:write/></D:locktype>
+    <D:owner>
+        <D:href>http://example.org/~ejw/contact.html</D:href>
+    </D:owner>
+</D:lockinfo>');
+
+        $this->server->httpRequest = $request;
+        $this->server->exec();
+
+        $this->assertEquals('application/xml; charset=utf-8',$this->response->getHeader('Content-Type'));
+        $this->assertTrue(preg_match('/^<opaquelocktoken:(.*)>$/',$this->response->getHeader('Lock-Token'))===1,'We did not get a valid Locktoken back (' . $this->response->getHeader('Lock-Token') . ')');
+
+        $this->assertEquals(201, $this->response->getStatus());
+
+        $request = new HTTP\Request(
+            'PUT',
+            '/test.txt',
+            ['If' => '</unrelated.txt> ('.$this->response->getHeader('Lock-Token').')']
+        );
+        $request->setBody('newbody');
+        $this->server->httpRequest = $request;
+        $this->server->exec();
+
+        $this->assertEquals('application/xml; charset=utf-8',$this->response->getHeader('Content-Type'));
+        $this->assertTrue(preg_match('/^<opaquelocktoken:(.*)>$/',$this->response->getHeader('Lock-Token'))===1,'We did not get a valid Locktoken back (' . $this->response->getHeader('Lock-Token') . ')');
+
+        $this->assertEquals(204, $this->response->status);
+
+    }
+
     function testPutWithIncorrectETag() {
 
         $serverVars = array(

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-owncloud/php-sabredav.git



More information about the Pkg-owncloud-commits mailing list