Disable REPORT without filter

Because we need to use Node::search($pattern) to find all matching nodes in all the subfolder recursively, but the result nodes contain incomplete information like owner.
owncloud · Mar 17, 2017 · a7ed90f · a7ed90f
1 parent 099a9f9
commit a7ed90f
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 66 deletions.
diff --git a/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php b/apps/dav/lib/Connector/Sabre/FilesReportPlugin.php
@@ -180,10 +180,34 @@ public function onReport($reportName, $report, $uri) {
 		$filterRules = $report->filters;
 
 		if (empty($filterRules['systemtag']) && is_null($filterRules['favorite'])) {
-			// load all
-			$results = $reportTargetNode->getChildren();
-			$results = $this->slice($results, $report);
+			// FIXME: search currently not possible because results are missing properties!
+			throw new BadRequest('No filter criteria specified');
+			/*
+			// search all
+			$folder = $this->userFolder;
+			if (trim($reportTargetNode->getPath(), '/') !== '') {
+				$folder = $folder->get($rootNode->getPath());
+			}
+
+			if (!isset($report->search['pattern']) || trim($report->search['pattern']) === '') {
+				// refuse to return ALL files from the system if no pattern is specified
+				throw new BadRequest('No search criteria specified');
+			}
+
+			// TODO: if depth one, could use a different search algo based on getChildren() ...
+			// TODO: Depth header filtering?
+			$results = $folder->search($report->search['pattern']);
+			$results = $this->slice($results);
+			$results = array_map(function ($result) {
+				return $this->makeSabreNode($result);
+			}, $results);
+			*/
 		} else {
+			if (isset($report->search['pattern'])) {
+				// TODO: implement this at some point...
+				throw new BadRequest('Search pattern cannot be combined with filter');
+			}
+
 			// gather all file ids matching filter
 			try {
 				$resultFileIds = $this->processFilterRules($filterRules);
@@ -366,17 +390,25 @@ public function findNodesByFileIds($rootNode, $fileIds) {
 			$entry = $folder->getById($fileId);
 			if ($entry) {
 				$entry = current($entry);
-				if ($entry instanceof \OCP\Files\File) {
-					$results[] = new File($this->fileView, $entry);
-				} else if ($entry instanceof \OCP\Files\Folder) {
-					$results[] = new Directory($this->fileView, $entry);
+				$node = $this->makeSabreNode($entry);
+				if ($node) {
+					$results[] = $node;
 				}
 			}
 		}
 
 		return $results;
 	}
 
+	private function makeSabreNode(\OCP\Files\Node $filesNode) {
+		if ($filesNode instanceof \OCP\Files\File) {
+			return new File($this->fileView, $filesNode);
+		} else if ($filesNode instanceof \OCP\Files\Folder) {
+			return new Directory($this->fileView, $filesNode);
+		}
+		return null;
+	}
+
 	/**
 	 * Returns whether the currently logged in user is an administrator
 	 */

diff --git a/apps/dav/lib/Files/Xml/FilterRequest.php b/apps/dav/lib/Files/Xml/FilterRequest.php
@@ -87,6 +87,9 @@ static function xmlDeserialize(Reader $reader) {
 					break;
 				case '{http://owncloud.org/ns}search' :
 					$value = $elem['value'];
+					if (isset($value['{http://owncloud.org/ns}pattern'])) {
+						$newProps['search']['pattern'] = $value['{http://owncloud.org/ns}pattern'];
+					}
 					if (isset($value['{http://owncloud.org/ns}limit'])) {
 						$newProps['search']['limit'] = (int)$value['{http://owncloud.org/ns}limit'];
 					}

diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php
@@ -267,65 +267,6 @@ public function testOnReport() {
 		$this->assertCount(0, $props2[200]['{DAV:}resourcetype']->getValue());
 	}
 
-	public function testOnReportPaginationAll() {
-		$path = 'test';
-
-		$parameters = new FilterRequest();
-		$parameters->properties = [
-			'{DAV:}getcontentlength',
-		];
-		$parameters->search = [
-			'offset' => 2,
-			'limit' => 3,
-		];
-
-		$nodes = [];
-		for ($i = 0; $i < 20; $i++) {
-			$fileInfo = $this->createMock(FileInfo::class);
-			$fileInfo->method('isReadable')->willReturn(true);
-			$node = $this->createMock(\OCA\DAV\Connector\Sabre\File::class);
-			$node->method('getId')->willReturn(1000 + $i);
-			$node->method('getPath')->willReturn('/nodes/node' . $i);
-			$node->method('getFileInfo')->willReturn($fileInfo);
-			$nodes[$node->getId()] = $node;
-		}
-
-		$reportTargetNode = $this->createMock(\OCA\DAV\Connector\Sabre\Directory::class);
-		$reportTargetNode->expects($this->once())
-			->method('getChildren')
-			->willReturn(array_values($nodes));
-
-		$this->tree->expects($this->any())
-			->method('getNodeForPath')
-			->with('/' . $path)
-			->will($this->returnValue($reportTargetNode));
-
-		// getById must only be called for the required nodes
-		$this->userFolder->expects($this->never())
-			->method('getById');
-
-		$this->server->expects($this->any())
-			->method('getRequestUri')
-			->will($this->returnValue($path));
-
-		$this->plugin->initialize($this->server);
-
-		$responses = null;
-		$this->server->expects($this->once())
-			->method('generateMultiStatus')
-			->will($this->returnCallback(function($responsesArg) use (&$responses) {
-				$responses = $responsesArg;
-			})
-		);
-
-		$this->assertFalse($this->plugin->onReport(FilesReportPluginImplementation::REPORT_NAME, $parameters, '/' . $path));
-
-		$this->assertCount(3, $responses);
-
-		$this->assertEquals('/test/nodes/node2', $responses[0]['href']);
-		$this->assertEquals('/test/nodes/node3', $responses[1]['href']);
-		$this->assertEquals('/test/nodes/node4', $responses[2]['href']);
-	}
 	public function testOnReportPaginationFiltered() {
 		$path = 'test';