Merge pull request #36774 from nextcloud/bugfix/noid/sabre-nodes

fix: always use proper path on node api when calling the view
nextcloud · Apr 18, 2023 · 855e7a2 · 855e7a2
2 parents e10e509 + d9c81f5
commit 855e7a2
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 135 deletions.
diff --git a/apps/dav/lib/Connector/Sabre/Node.php b/apps/dav/lib/Connector/Sabre/Node.php
@@ -91,11 +91,13 @@ public function __construct(View $view, FileInfo $info, IManager $shareManager =
 		if ($info instanceof Folder || $info instanceof File) {
 			$this->node = $info;
 		} else {
+			// The Node API assumes that the view passed doesn't have a fake root
+			$rootView = \OC::$server->get(View::class);
 			$root = \OC::$server->get(IRootFolder::class);
 			if ($info->getType() === FileInfo::TYPE_FOLDER) {
-				$this->node = new Folder($root, $view, $this->path, $info);
+				$this->node = new Folder($root, $rootView, $this->fileView->getAbsolutePath($this->path), $info);
 			} else {
-				$this->node = new File($root, $view, $this->path, $info);
+				$this->node = new File($root, $rootView, $this->fileView->getAbsolutePath($this->path), $info);
 			}
 		}
 	}
@@ -107,10 +109,11 @@ protected function refreshInfo(): void {
 		}
 		$this->info = $info;
 		$root = \OC::$server->get(IRootFolder::class);
+		$rootView = \OC::$server->get(View::class);
 		if ($this->info->getType() === FileInfo::TYPE_FOLDER) {
-			$this->node = new Folder($root, $this->fileView, $this->path, $this->info);
+			$this->node = new Folder($root, $rootView, $this->path, $this->info);
 		} else {
-			$this->node = new File($root, $this->fileView, $this->path, $this->info);
+			$this->node = new File($root, $rootView, $this->path, $this->info);
 		}
 	}
 

diff --git a/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php b/apps/dav/tests/unit/Connector/Sabre/DirectoryTest.php
@@ -29,9 +29,12 @@
 namespace OCA\DAV\Tests\Unit\Connector\Sabre;
 
 use OC\Files\FileInfo;
+use OC\Files\Filesystem;
 use OC\Files\Node\Node;
 use OC\Files\Storage\Wrapper\Quota;
+use OC\Files\View;
 use OCA\DAV\Connector\Sabre\Directory;
+use OCP\Constants;
 use OCP\Files\ForbiddenException;
 use OCP\Files\Mount\IMountPoint;
 use Test\Traits\UserTrait;
@@ -91,6 +94,10 @@ protected function setUp(): void {
 			->willReturn(Node::TYPE_FOLDER);
 		$this->info->method('getName')
 			->willReturn("folder");
+		$this->info->method('getPath')
+			->willReturn("/admin/files/folder");
+		$this->info->method('getPermissions')
+			->willReturn(Constants::PERMISSION_READ);
 	}
 
 	private function getDir($path = '/') {
@@ -207,12 +214,21 @@ public function testGetChildren(): void {
 
 		$this->view->expects($this->once())
 			->method('getDirectoryContent')
-			->with('')
 			->willReturn([$info1, $info2]);
 
 		$this->view->expects($this->any())
 			->method('getRelativePath')
-			->willReturn('');
+			->willReturnCallback(function ($path) {
+				return str_replace('/admin/files/', '', $path);
+			});
+
+		$this->view->expects($this->any())
+			->method('getAbsolutePath')
+			->willReturnCallback(function ($path) {
+				return Filesystem::normalizePath('/admin/files' . $path);
+			});
+
+		$this->overwriteService(View::class, $this->view);
 
 		$dir = new Directory($this->view, $this->info);
 		$nodes = $dir->getChildren();

diff --git a/apps/dav/tests/unit/Files/FileSearchBackendTest.php b/apps/dav/tests/unit/Files/FileSearchBackendTest.php
@@ -86,9 +86,11 @@ protected function setUp(): void {
 			->disableOriginalConstructor()
 			->getMock();
 
-		$this->view = $this->getMockBuilder(View::class)
-			->disableOriginalConstructor()
-			->getMock();
+		$this->view = $this->createMock(View::class);
+
+		$this->view->expects($this->any())
+			->method('getRoot')
+			->willReturn('');
 
 		$this->view->expects($this->any())
 			->method('getRelativePath')

diff --git a/lib/private/Files/Node/Node.php b/lib/private/Files/Node/Node.php
@@ -37,6 +37,7 @@
 use OCP\Files\NotFoundException;
 use OCP\Files\NotPermittedException;
 use OCP\Lock\LockedException;
+use OCP\PreConditionNotMetException;
 use Symfony\Component\EventDispatcher\GenericEvent;
 
 // FIXME: this class really should be abstract
@@ -52,7 +53,7 @@ class Node implements \OCP\Files\Node {
 	protected $root;
 
 	/**
-	 * @var string $path
+	 * @var string $path Absolute path to the node (e.g. /admin/files/folder/file)
 	 */
 	protected $path;
 
@@ -72,6 +73,9 @@ class Node implements \OCP\Files\Node {
 	 * @param FileInfo $fileInfo
 	 */
 	public function __construct($root, $view, $path, $fileInfo = null, ?Node $parent = null, bool $infoHasSubMountsIncluded = true) {
+		if (Filesystem::normalizePath($view->getRoot()) !== '/') {
+			throw new PreConditionNotMetException('The view passed to the node should not have any fake root set');
+		}
 		$this->view = $view;
 		$this->root = $root;
 		$this->path = $path;

diff --git a/tests/lib/Files/Node/FileTest.php b/tests/lib/Files/Node/FileTest.php
@@ -185,7 +185,7 @@ public function testFOpenWrite() {
 
 		$root = new \OC\Files\Node\Root(
 			$this->manager,
-			new $this->view,
+			$this->view,
 			$this->user,
 			$this->userMountCache,
 			$this->logger,
@@ -277,7 +277,7 @@ public function testFOpenReadWriteNoWritePermissions() {
 
 		$root = new \OC\Files\Node\Root(
 			$this->manager,
-			new $this->view,
+			$this->view,
 			$this->user,
 			$this->userMountCache,
 			$this->logger,