Locking: Limit key length in Shared Storage #25376 (#25423)

5 files changed, 42 insertions, 2 deletions

diff --git a/apps/files_sharing/tests/sharedstorage.php b/apps/files_sharing/tests/sharedstorage.php
index 3361d2cbd12..6418d00ac1d 100644
--- a/apps/files_sharing/tests/sharedstorage.php
+++ b/apps/files_sharing/tests/sharedstorage.php
@@ -6,6 +6,7 @@
  * @author Robin Appelman <icewind@owncloud.com>
  * @author Thomas Müller <thomas.mueller@tmit.eu>
  * @author Vincent Petry <pvince81@owncloud.com>
+ * @author Markus Goetz <markus@woboq.com>
  *
  * @copyright Copyright (c) 2015, ownCloud, Inc.
  * @license AGPL-3.0
@@ -483,4 +484,30 @@ class Test_Files_Sharing_Storage extends OCA\Files_sharing\Tests\TestCase {
 		$source = $storage->getFile('');
 		$this->assertEquals(self::TEST_FILES_SHARING_API_USER1, $source['uid_owner']);
 	}
+
+	public function testLongLock() {
+		// https://github.com/owncloud/core/issues/25376
+		$fn = str_repeat("x",250); // maximum name length in oc_filecache
+		self::loginHelper(self::TEST_FILES_SHARING_API_USER1);
+		$view1 = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER1 . '/files');
+		$view1->mkdir($fn);
+		$view1->mkdir($fn.'/'.'vincent');
+		$view1->file_put_contents($fn . '/vincent/' . $fn, "dummy file data\n");
+
+		$fileinfo = $view1->getFileInfo($fn);
+		$result = \OCP\Share::shareItem('folder', $fileinfo['fileid'], \OCP\Share::SHARE_TYPE_USER, self::TEST_FILES_SHARING_API_USER2, \OCP\Constants::PERMISSION_ALL);
+		$this->assertTrue($result);
+
+		self::loginHelper(self::TEST_FILES_SHARING_API_USER2);
+		$user2View = new \OC\Files\View('/' . self::TEST_FILES_SHARING_API_USER2 . '/files');
+		$this->assertTrue($user2View->file_exists($fn . '/vincent/' . $fn));
+
+		list($sharedStorage,$bla) = $user2View->resolvePath($fn . '/vincent/' . $fn);
+		$cache = $sharedStorage->getCache();
+		$scanner = $sharedStorage->getScanner();
+		$scanner->scan('');
+		$this->assertTrue($cache->inCache('/vincent'));
+		$this->assertTrue($cache->inCache('/vincent/'.$fn));
+		$this->assertEquals(16, $cache->get('/vincent/'.$fn)['size']);
+	}
 }
diff --git a/lib/private/files/cache/scanner.php b/lib/private/files/cache/scanner.php
index 12b1b541d24..ba2c4931bd4 100644
--- a/lib/private/files/cache/scanner.php
+++ b/lib/private/files/cache/scanner.php
@@ -261,7 +261,7 @@ class Scanner extends BasicEmitter {
 			$reuse = ($recursive === self::SCAN_SHALLOW) ? self::REUSE_ETAG | self::REUSE_SIZE : self::REUSE_ETAG;
 		}
 		if ($lock) {
-			$this->lockingProvider->acquireLock('scanner::' . $this->storageId . '::' . $path, ILockingProvider::LOCK_EXCLUSIVE);
+			$this->lockingProvider->acquireLock('scanner::' . md5($this->storageId . '::' . $path), ILockingProvider::LOCK_EXCLUSIVE);
 			$this->storage->acquireLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
 		}
 		$data = $this->scanFile($path, $reuse, -1, null, $lock);
@@ -271,7 +271,7 @@ class Scanner extends BasicEmitter {
 		}
 		if ($lock) {
 			$this->storage->releaseLock($path, ILockingProvider::LOCK_SHARED, $this->lockingProvider);
-			$this->lockingProvider->releaseLock('scanner::' . $this->storageId . '::' . $path, ILockingProvider::LOCK_EXCLUSIVE);
+			$this->lockingProvider->releaseLock('scanner::' . md5($this->storageId . '::' . $path), ILockingProvider::LOCK_EXCLUSIVE);
 		}
 		return $data;
 	}
diff --git a/lib/private/lock/dblockingprovider.php b/lib/private/lock/dblockingprovider.php
index fb4c60a7640..e648a9b0850 100644
--- a/lib/private/lock/dblockingprovider.php
+++ b/lib/private/lock/dblockingprovider.php
@@ -150,6 +150,9 @@ class DBLockingProvider extends AbstractLockingProvider {
 	 * @throws \OCP\Lock\LockedException
 	 */
 	public function acquireLock($path, $type) {
+		if (strlen($path) > 64) { // max length in file_locks
+			throw new \InvalidArgumentException("Lock key length too long");
+		}
 		$expire = $this->getExpireTime();
 		if ($type === self::LOCK_SHARED) {
 			if (!$this->isLocallyLocked($path)) {
diff --git a/lib/private/lock/memcachelockingprovider.php b/lib/private/lock/memcachelockingprovider.php
index af95200d159..e63e5912982 100644
--- a/lib/private/lock/memcachelockingprovider.php
+++ b/lib/private/lock/memcachelockingprovider.php
@@ -66,6 +66,9 @@ class MemcacheLockingProvider extends AbstractLockingProvider {
 	 * @throws \OCP\Lock\LockedException
 	 */
 	public function acquireLock($path, $type) {
+		if (strlen($path) > 64) { // max length in file_locks
+			throw new \InvalidArgumentException("Lock key length too long");
+		}
 		if ($type === self::LOCK_SHARED) {
 			if (!$this->memcache->inc($path)) {
 				throw new LockedException($path);
diff --git a/tests/lib/lock/lockingprovider.php b/tests/lib/lock/lockingprovider.php
index ca72c1bb7f3..60f7edb3c42 100644
--- a/tests/lib/lock/lockingprovider.php
+++ b/tests/lib/lock/lockingprovider.php
@@ -241,4 +241,11 @@ abstract class LockingProvider extends TestCase {
 		$this->instance->acquireLock('foo', ILockingProvider::LOCK_SHARED);
 		$this->instance->changeLock('foo', ILockingProvider::LOCK_SHARED);
 	}
+
+	/**
+	 * @expectedException \InvalidArgumentException
+	*/
+	public function testTooLongLockName() {
+	   $this->instance->acquireLock(str_repeat("x",250), ILockingProvider::LOCK_EXCLUSIVE);
+	}
 }