handle long property paths to hasing paths >250 chars

Signed-off-by: Robin Appelman <robin@icewind.nl>

2 files changed, 77 insertions, 32 deletions

diff --git a/apps/dav/lib/DAV/CustomPropertiesBackend.php b/apps/dav/lib/DAV/CustomPropertiesBackend.php
index c6357869a0c..5a0128e52ca 100644
--- a/apps/dav/lib/DAV/CustomPropertiesBackend.php
+++ b/apps/dav/lib/DAV/CustomPropertiesBackend.php
@@ -44,7 +44,7 @@ class CustomPropertiesBackend implements BackendInterface {
 	 *
 	 * @var array
 	 */
-	private $ignoredProperties = array(
+	private $ignoredProperties = [
 		'{DAV:}getcontentlength',
 		'{DAV:}getcontenttype',
 		'{DAV:}getetag',
@@ -55,7 +55,7 @@ class CustomPropertiesBackend implements BackendInterface {
 		'{http://owncloud.org/ns}dDC',
 		'{http://owncloud.org/ns}size',
 		'{http://nextcloud.org/ns}is-encrypted',
-	);
+	];
 
 	/**
 	 * @var Tree
@@ -115,7 +115,7 @@ class CustomPropertiesBackend implements BackendInterface {
 			// (soft fail)
 			\OC::$server->getLogger()->warning(
 				'Could not get node for path: \"' . $path . '\" : ' . $e->getMessage(),
-				array('app' => 'files')
+				['app' => 'files']
 			);
 			return;
 		}
@@ -172,7 +172,7 @@ class CustomPropertiesBackend implements BackendInterface {
 			return;
 		}
 
-		$propPatch->handleRemaining(function($changedProps) use ($node) {
+		$propPatch->handleRemaining(function ($changedProps) use ($node) {
 			return $this->updateProperties($node, $changedProps);
 		});
 	}
@@ -186,7 +186,7 @@ class CustomPropertiesBackend implements BackendInterface {
 		$statement = $this->connection->prepare(
 			'DELETE FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?'
 		);
-		$statement->execute(array($this->user->getUID(), $path));
+		$statement->execute([$this->user->getUID(), $this->formatPath($path)]);
 		$statement->closeCursor();
 
 		unset($this->cache[$path]);
@@ -205,12 +205,13 @@ class CustomPropertiesBackend implements BackendInterface {
 			'UPDATE `*PREFIX*properties` SET `propertypath` = ?' .
 			' WHERE `userid` = ? AND `propertypath` = ?'
 		);
-		$statement->execute(array($destination, $this->user->getUID(), $source));
+		$statement->execute([$this->formatPath($destination), $this->user->getUID(), $this->formatPath($source)]);
 		$statement->closeCursor();
 	}
 
 	/**
 	 * Returns a list of properties for this nodes.;
+	 *
 	 * @param Node $node
 	 * @param array $requestedProperties requested properties or empty array for "all"
 	 * @return array
@@ -228,8 +229,8 @@ class CustomPropertiesBackend implements BackendInterface {
 		// TODO: chunking if more than 1000 properties
 		$sql = 'SELECT * FROM `*PREFIX*properties` WHERE `userid` = ? AND `propertypath` = ?';
 
-		$whereValues = array($this->user->getUID(), $path);
-		$whereTypes = array(null, null);
+		$whereValues = [$this->user->getUID(), $this->formatPath($path)];
+		$whereTypes = [null, null];
 
 		if (!empty($requestedProperties)) {
 			// request only a subset
@@ -276,38 +277,38 @@ class CustomPropertiesBackend implements BackendInterface {
 			' WHERE `userid` = ? AND `propertypath` = ? AND `propertyname` = ?';
 
 		// TODO: use "insert or update" strategy ?
-		$existing = $this->getProperties($node, array());
+		$existing = $this->getProperties($node, []);
 		$this->connection->beginTransaction();
 		foreach ($properties as $propertyName => $propertyValue) {
 			// If it was null, we need to delete the property
 			if (is_null($propertyValue)) {
 				if (array_key_exists($propertyName, $existing)) {
 					$this->connection->executeUpdate($deleteStatement,
-						array(
+						[
 							$this->user->getUID(),
-							$path,
-							$propertyName
-						)
+							$this->formatPath($path),
+							$propertyName,
+						]
 					);
 				}
 			} else {
 				if (!array_key_exists($propertyName, $existing)) {
 					$this->connection->executeUpdate($insertStatement,
-						array(
+						[
 							$this->user->getUID(),
-							$path,
+							$this->formatPath($path),
 							$propertyName,
-							$propertyValue
-						)
+							$propertyValue,
+						]
 					);
 				} else {
 					$this->connection->executeUpdate($updateStatement,
-						array(
+						[
 							$propertyValue,
 							$this->user->getUID(),
-							$path,
-							$propertyName
-						)
+							$this->formatPath($path),
+							$propertyName,
+						]
 					);
 				}
 			}
@@ -318,4 +319,18 @@ class CustomPropertiesBackend implements BackendInterface {
 
 		return true;
 	}
+
+	/**
+	 * long paths are hashed to ensure they fit in the database
+	 *
+	 * @param string $path
+	 * @return string
+	 */
+	private function formatPath(string $path): string {
+		if (strlen($path) > 250) {
+			return sha1($path);
+		} else {
+			return $path;
+		}
+	}
 }
diff --git a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
index 45be59151ec..3250361ed17 100644
--- a/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
+++ b/apps/dav/tests/unit/DAV/CustomPropertiesBackendTest.php
@@ -100,6 +100,14 @@ class CustomPropertiesBackendTest extends TestCase {
 		parent::tearDown();
 	}
 
+	private function formatPath(string $path): string {
+		if (strlen($path) > 250) {
+			return sha1($path);
+		} else {
+			return $path;
+		}
+	}
+
 	protected function insertProps(string $user, string $path, array $props) {
 		foreach ($props as $name => $value) {
 			$this->insertProp($user, $path, $name, $value);
@@ -111,7 +119,7 @@ class CustomPropertiesBackendTest extends TestCase {
 		$query->insert('properties')
 			->values([
 				'userid' => $query->createNamedParameter($user),
-				'propertypath' => $query->createNamedParameter($path),
+				'propertypath' => $query->createNamedParameter($this->formatPath($path)),
 				'propertyname' => $query->createNamedParameter($name),
 				'propertyvalue' => $query->createNamedParameter($value),
 			]);
@@ -123,7 +131,7 @@ class CustomPropertiesBackendTest extends TestCase {
 		$query->select('propertyname', 'propertyvalue')
 			->from('properties')
 			->where($query->expr()->eq('userid', $query->createNamedParameter($user)))
-			->where($query->expr()->eq('propertypath', $query->createNamedParameter($path)));
+			->where($query->expr()->eq('propertypath', $query->createNamedParameter($this->formatPath($path))));
 		return $query->execute()->fetchAll(\PDO::FETCH_KEY_PAIR);
 	}
 
@@ -213,23 +221,45 @@ class CustomPropertiesBackendTest extends TestCase {
 	}
 
 	public function propPatchProvider() {
+		$longPath = str_repeat('long_path', 100);
 		return [
 			['foo_bar_path_1337', [], ['{DAV:}displayname' => 'anything'], ['{DAV:}displayname' => 'anything']],
 			['foo_bar_path_1337', ['{DAV:}displayname' => 'foo'], ['{DAV:}displayname' => 'anything'], ['{DAV:}displayname' => 'anything']],
 			['foo_bar_path_1337', ['{DAV:}displayname' => 'foo'], ['{DAV:}displayname' => null], []],
+			[$longPath, [], ['{DAV:}displayname' => 'anything'], ['{DAV:}displayname' => 'anything']],
 		];
 	}
 
-	public function testDelete() {
-		$this->insertProps('dummy_user_42', 'foo_bar_path_1337', ['foo' => 'bar']);
-		$this->backend->delete('foo_bar_path_1337');
-		$this->assertEquals([], $this->getProps('dummy_user_42', 'foo_bar_path_1337'));
+	/**
+	 * @dataProvider deleteProvider
+	 */
+	public function testDelete(string $path) {
+		$this->insertProps('dummy_user_42', $path, ['foo' => 'bar']);
+		$this->backend->delete($path);
+		$this->assertEquals([], $this->getProps('dummy_user_42', $path));
 	}
 
-	public function testMove() {
-		$this->insertProps('dummy_user_42', 'foo_bar_path_1337', ['foo' => 'bar']);
-		$this->backend->move('foo_bar_path_1337', 'bar_foo_path_7331');
-		$this->assertEquals([], $this->getProps('dummy_user_42', 'foo_bar_path_1337'));
-		$this->assertEquals(['foo' => 'bar'], $this->getProps('dummy_user_42', 'bar_foo_path_7331'));
+	public function deleteProvider() {
+		return [
+			['foo_bar_path_1337'],
+			[str_repeat('long_path', 100)]
+		];
+	}
+
+	/**
+	 * @dataProvider moveProvider
+	 */
+	public function testMove(string $source, string $target) {
+		$this->insertProps('dummy_user_42', $source, ['foo' => 'bar']);
+		$this->backend->move($source, $target);
+		$this->assertEquals([], $this->getProps('dummy_user_42', $source));
+		$this->assertEquals(['foo' => 'bar'], $this->getProps('dummy_user_42', $target));
+	}
+
+	public function moveProvider() {
+		return [
+			['foo_bar_path_1337', 'foo_bar_path_7333'],
+			[str_repeat('long_path1', 100), str_repeat('long_path2', 100)]
+		];
 	}
 }