From 192b0d33b7a551184dbb07b5b403e44169cb2190 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Mr=C3=B3wczy=C5=84ski?= Date: Mon, 25 Apr 2022 21:47:10 +0200 Subject: [PATCH 1/2] default for propfind depth infinity should be false as in the past --- apps/dav/lib/Capabilities.php | 2 +- apps/dav/lib/Connector/Sabre/ServerFactory.php | 2 +- apps/dav/lib/Server.php | 2 +- changelog/unreleased/40016 | 7 +++++++ config/config.sample.php | 7 ++++--- 5 files changed, 14 insertions(+), 6 deletions(-) create mode 100644 changelog/unreleased/40016 diff --git a/apps/dav/lib/Capabilities.php b/apps/dav/lib/Capabilities.php index 08fcae3fd060..51518cb44d33 100644 --- a/apps/dav/lib/Capabilities.php +++ b/apps/dav/lib/Capabilities.php @@ -45,7 +45,7 @@ public function getCapabilities() { 'search-files', ], 'propfind' => [ - 'depth_infinity' => $this->config->getSystemValue('dav.propfind.depth_infinity', true), + 'depth_infinity' => $this->config->getSystemValue('dav.propfind.depth_infinity', false), ] ] ]; diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index d0229205f98e..a892565f22ef 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -140,7 +140,7 @@ public function createServer( $server->on('beforeMethod:PROPFIND', function (Request $request) use ($config) { $depthHeader = strtolower($request->getHeader('depth')); - if ($depthHeader === 'infinity' && !$config->getSystemValue('dav.propfind.depth_infinity', true)) { + if ($depthHeader === 'infinity' && !$config->getSystemValue('dav.propfind.depth_infinity', false)) { throw new PreconditionFailed('Depth infinity not supported'); } }, 0); diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 29fd27959907..684e1b800915 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -244,7 +244,7 @@ public function __construct(IRequest $request, $baseUri) { $this->server->on('beforeMethod:PROPFIND', function (Request $request) use ($config) { $depthHeader = strtolower($request->getHeader('depth')); - if ($depthHeader === 'infinity' && !$config->getSystemValue('dav.propfind.depth_infinity', true)) { + if ($depthHeader === 'infinity' && !$config->getSystemValue('dav.propfind.depth_infinity', false)) { throw new Exception\PreconditionFailed('Depth infinity not supported'); } }, 0); diff --git a/changelog/unreleased/40016 b/changelog/unreleased/40016 new file mode 100644 index 000000000000..ae6dbd60e98e --- /dev/null +++ b/changelog/unreleased/40016 @@ -0,0 +1,7 @@ +Bugfix: default for propfind depth infinity adjusted + +Fixed potential cause for performance issues under certain conditions +with infinite propfind being enabled by default. + +https://github.com/owncloud/core/pull/40016 +https://github.com/owncloud/enterprise/issues/5154 diff --git a/config/config.sample.php b/config/config.sample.php index d107aeca8764..324e4da7e94e 100644 --- a/config/config.sample.php +++ b/config/config.sample.php @@ -1717,11 +1717,12 @@ /** * Allow propfind depth infinity * - * With this setting that defaults to true, propfind requests will now be streamed to reduce memory usage - * with large responses. It tells the clients whether `depth=infinity` is allowed for propfind requests. + * It tells the clients whether `depth=infinity` is allowed for propfind requests. + * Streamed infinite depth propfind requests can reduce memory usage + * with large responses. * For details see: https://datatracker.ietf.org/doc/html/rfc4918#section-10.2 */ -'dav.propfind.depth_infinity' => true, +'dav.propfind.depth_infinity' => false, /** * Show the grace period popup From 47a651ca5ab617f17d5103415d0fc3ebe92df653 Mon Sep 17 00:00:00 2001 From: Phil Davis Date: Tue, 26 Apr 2022 10:07:35 +0545 Subject: [PATCH 2/2] Adjust tests for infinite depth changes --- .../apiWebdavOperations/listFiles.feature | 16 +- .../features/bootstrap/CommentsContext.php | 4 +- .../features/bootstrap/OccContext.php | 12 +- .../features/bootstrap/TrashbinContext.php | 165 ++++++++++++------ .../acceptance/features/bootstrap/WebDav.php | 117 ++++++++++--- .../bootstrap/WebDavPropertiesContext.php | 18 +- .../propfindOnLocalStorage.feature | 2 + ...opfindOnLocalStorageOc10Issue39470.feature | 2 + 8 files changed, 239 insertions(+), 97 deletions(-) diff --git a/tests/acceptance/features/apiWebdavOperations/listFiles.feature b/tests/acceptance/features/apiWebdavOperations/listFiles.feature index 857d413ce7f6..04b60f737dac 100644 --- a/tests/acceptance/features/apiWebdavOperations/listFiles.feature +++ b/tests/acceptance/features/apiWebdavOperations/listFiles.feature @@ -76,6 +76,7 @@ Feature: list files Scenario Outline: Get the list of resources in the root folder with depth infinity Given using DAV path + And the administrator has set depth_infinity_allowed to 1 When user "Alice" lists the resources in "/" with depth "infinity" using the WebDAV API Then the HTTP status code should be "207" And the last DAV response for user "Alice" should contain these nodes @@ -157,6 +158,7 @@ Feature: list files Scenario Outline: Get the list of resources in a folder with depth infinity Given using DAV path + And the administrator has set depth_infinity_allowed to 1 When user "Alice" lists the resources in "/simple-folder" with depth "infinity" using the WebDAV API Then the HTTP status code should be "207" And the last DAV response for user "Alice" should contain these nodes @@ -256,6 +258,7 @@ Feature: list files Scenario Outline: Get the list of resources in a folder shared through public link with depth infinity Given using DAV path + And the administrator has set depth_infinity_allowed to 1 And user "Alice" has created the following folders | path | | /simple-folder/simple-folder1/simple-folder2/simple-folder3 | @@ -291,14 +294,14 @@ Feature: list files | spaces | - Scenario Outline: Get the list of files in a folder in the trashbin with depth 0 + Scenario Outline: Get the list of files in the trashbin with depth 0 Given using DAV path And user "Alice" has deleted the following resources | path | | textfile0.txt | | welcome.txt | | simple-folder/ | - When user "Alice" lists the resources in the trashbin path "/" with depth "0" using the WebDAV API + When user "Alice" lists the resources in the trashbin with depth "0" using the WebDAV API Then the HTTP status code should be "207" And the trashbin DAV response should not contain these nodes | name | @@ -322,14 +325,14 @@ Feature: list files | spaces | - Scenario Outline: Get the list of files in a folder in the trashbin with depth 1 + Scenario Outline: Get the list of files in the trashbin with depth 1 Given using DAV path And user "Alice" has deleted the following resources | path | | textfile0.txt | | welcome.txt | | simple-folder/ | - When user "Alice" lists the resources in the trashbin path "/" with depth "1" using the WebDAV API + When user "Alice" lists the resources in the trashbin with depth "1" using the WebDAV API Then the HTTP status code should be "207" And the trashbin DAV response should contain these nodes | name | @@ -355,14 +358,15 @@ Feature: list files | spaces | - Scenario Outline: Get the list of files in a folder in the trashbin with depth infinity + Scenario Outline: Get the list of files in the trashbin with depth infinity Given using DAV path + And the administrator has set depth_infinity_allowed to 1 And user "Alice" has deleted the following resources | path | | textfile0.txt | | welcome.txt | | simple-folder/ | - When user "Alice" lists the resources in the trashbin path "/" with depth "infinity" using the WebDAV API + When user "Alice" lists the resources in the trashbin with depth "infinity" using the WebDAV API Then the HTTP status code should be "207" And the trashbin DAV response should contain these nodes | name | diff --git a/tests/acceptance/features/bootstrap/CommentsContext.php b/tests/acceptance/features/bootstrap/CommentsContext.php index d4dae50a576a..784949c9c3c4 100644 --- a/tests/acceptance/features/bootstrap/CommentsContext.php +++ b/tests/acceptance/features/bootstrap/CommentsContext.php @@ -529,6 +529,7 @@ public function followingPropertiesShouldBeListed(string $user, TableNode $expec $responseXmlObject = $this->featureContext->getResponseXmlObject(); $responses = $responseXmlObject->xpath("//d:response"); $found = false; + $savedResponseXml = "UNKNOWN"; foreach ($responses as $response) { foreach ($expectedProperties as $expectedProperty) { $expectedProperty['propertyValue'] = $this->featureContext->substituteInLineCodes( @@ -548,8 +549,9 @@ public function followingPropertiesShouldBeListed(string $user, TableNode $expec if ($found) { break; } + $savedResponseXml = $response->asXML(); } - Assert::assertTrue($found, "Could not find expected properties in" . $response->asXML()); + Assert::assertTrue($found, "Could not find expected properties in response " . $savedResponseXml); } /** diff --git a/tests/acceptance/features/bootstrap/OccContext.php b/tests/acceptance/features/bootstrap/OccContext.php index 9f4ff7188e16..e77cac80b652 100644 --- a/tests/acceptance/features/bootstrap/OccContext.php +++ b/tests/acceptance/features/bootstrap/OccContext.php @@ -1040,16 +1040,22 @@ public function theAdministratorHasSetTheDefaultFolderForReceivedSharesTo(string /** * @When the administrator has set depth_infinity_allowed to :depth_infinity_allowed * - * @param int $depth_infinity_allowed + * @param int $depthInfinityAllowed * * @return void * @throws Exception */ - public function theAdministratorHasSetDepthInfinityAllowedTo($depth_infinity_allowed) { + public function theAdministratorHasSetDepthInfinityAllowedTo($depthInfinityAllowed) { + $depthInfinityAllowedString = (string) $depthInfinityAllowed; $this->addSystemConfigKeyUsingTheOccCommand( "dav.propfind.depth_infinity", - (string) $depth_infinity_allowed + $depthInfinityAllowedString ); + if ($depthInfinityAllowedString === "0") { + $this->featureContext->davPropfindDepthInfinityDisabled(); + } else { + $this->featureContext->davPropfindDepthInfinityEnabled(); + } } /** diff --git a/tests/acceptance/features/bootstrap/TrashbinContext.php b/tests/acceptance/features/bootstrap/TrashbinContext.php index b3f34073ca3b..8b70ffe03e77 100644 --- a/tests/acceptance/features/bootstrap/TrashbinContext.php +++ b/tests/acceptance/features/bootstrap/TrashbinContext.php @@ -116,10 +116,17 @@ static function (SimpleXMLElement $propStat) { $name = $successPropStat->xpath('./d:prop/oc:trashbin-original-filename'); $mtime = $successPropStat->xpath('./d:prop/oc:trashbin-delete-timestamp'); + $resourcetype = $successPropStat->xpath('./d:prop/d:resourcetype'); + if (\array_key_exists(0, $resourcetype) && ($resourcetype[0]->asXML() === "")) { + $collection[0] = true; + } else { + $collection[0] = false; + } $originalLocation = $successPropStat->xpath('./d:prop/oc:trashbin-original-location'); } else { $name = []; $mtime = []; + $collection = []; $originalLocation = []; } @@ -127,6 +134,7 @@ static function (SimpleXMLElement $propStat) { 'href' => (string) $href, 'name' => isset($name[0]) ? (string) $name[0] : null, 'mtime' => isset($mtime[0]) ? (string) $mtime[0] : null, + 'collection' => isset($collection[0]) ? $collection[0] : false, 'original-location' => isset($originalLocation[0]) ? (string) $originalLocation[0] : null ]; }, @@ -137,28 +145,22 @@ static function (SimpleXMLElement $propStat) { } /** - * List trashbin folder + * List the top of the trashbin folder for a user * * @param string|null $user user - * @param string|null $path path - * @param string|null $asUser - To send request as another user - * @param string|null $password * @param string $depth * * @return array response * @throws Exception */ - public function listTrashbinFolder(?string $user, ?string $path, ?string $asUser = null, ?string $password = null, string $depth = "infinity"):array { - $asUser = $asUser ?? $user; - $path = $path ?? '/'; - $password = $password ?? $this->featureContext->getPasswordForUser($asUser); - $depth = (string) $depth; + public function listTopOfTrashbinFolder(?string $user, string $depth = "infinity"):array { + $password = $this->featureContext->getPasswordForUser($user); $davPathVersion = $this->featureContext->getDavPathVersion(); $response = WebDavHelper::listFolder( $this->featureContext->getBaseUrl(), - $asUser, + $user, $password, - $path, + "", $depth, $this->featureContext->getStepLineRef(), [ @@ -181,59 +183,116 @@ public function listTrashbinFolder(?string $user, ?string $path, ?string $asUser // filter root element $files = \array_filter( $files, - static function ($element) use ($user, $path) { - $path = \ltrim($path, '/'); - if ($path !== '') { - $path .= '/'; - } - return ($element['href'] !== "/remote.php/dav/trash-bin/$user/$path"); + static function ($element) use ($user) { + return ($element['href'] !== "/remote.php/dav/trash-bin/$user/"); + } + ); + return $files; + } + + /** + * List trashbin folder + * + * @param string|null $user user + * @param string $depth + * + * @return array of all the items in the trashbin of the user + * @throws Exception + */ + public function listTrashbinFolder(?string $user, string $depth = "1"):array { + return $this->listTrashbinFolderCollection( + $user, + "", + $depth + ); + } + + /** + * List a collection in the trashbin + * + * @param string|null $user user + * @param string|null $collectionPath the string of ids of the folder and sub-folders + * @param string $depth + * + * @return array response + * @throws Exception + */ + public function listTrashbinFolderCollection(?string $user, ?string $collectionPath = "", string $depth = "1"):array { + // $collectionPath should be some list of file-ids like 2147497661/2147497662 + // or the empty string, which will list the whole trashbin from the top. + $collectionPath = \trim($collectionPath, "/"); + $password = $this->featureContext->getPasswordForUser($user); + $davPathVersion = $this->featureContext->getDavPathVersion(); + $response = WebDavHelper::listFolder( + $this->featureContext->getBaseUrl(), + $user, + $password, + $collectionPath, + $depth, + $this->featureContext->getStepLineRef(), + [ + 'oc:trashbin-original-filename', + 'oc:trashbin-original-location', + 'oc:trashbin-delete-timestamp', + 'd:resourcetype', + 'd:getlastmodified' + ], + 'trash-bin', + $davPathVersion + ); + $responseXml = HttpRequestHelper::getResponseXml( + $response, + __METHOD__ . " $collectionPath" + ); + + $files = $this->getTrashbinContentFromResponseXml($responseXml); + // filter out the collection itself, we only want to return the members + $files = \array_filter( + $files, + static function ($element) use ($user, $collectionPath) { + return ($element['href'] !== "/remote.php/dav/trash-bin/$user/$collectionPath/"); } ); + + foreach ($files as $file) { + if ($file["collection"]) { + $trashbinRef = $file["href"]; + $trimmedHref = \trim($trashbinRef, "/"); + $explodedHref = \explode("/", $trimmedHref); + $trashbinId = $collectionPath . "/" . end($explodedHref); + $nextFiles = $this->listTrashbinFolderCollection( + $user, + $trashbinId, + $depth + ); + // filter the collection element. We only want the members. + $nextFiles = \array_filter( + $nextFiles, + static function ($element) use ($user, $trashbinRef) { + return ($element['href'] !== $trashbinRef); + } + ); + \array_push($files, ...$nextFiles); + } + } return $files; } /** - * @When user :user lists the resources in the trashbin path :path with depth :depth using the WebDAV API + * @When user :user lists the resources in the trashbin with depth :depth using the WebDAV API * * @param string $user - * @param string $path * @param string $depth * * @return void * @throws Exception */ - public function userGetsFilesInTheTrashbinWithDepthUsingTheWebdavApi(string $user, string $path, string $depth):void { - if ($path === "/" || $path === "") { - $this->listTrashbinFolder($user, $path, null, null, $depth); - return; - } + public function userGetsFilesInTheTrashbinWithDepthUsingTheWebdavApi(string $user, string $depth):void { $techPreviewHadToBeEnabled = $this->occContext->enableDAVTechPreview(); - $listing = $this->listTrashbinFolder($user, null); + $this->listTopOfTrashbinFolder($user, $depth); if ($techPreviewHadToBeEnabled) { $this->occContext->disableDAVTechPreview(); } - $originalPath = \trim($path, '/'); - if ($originalPath == "") { - $originalPath = null; - } - - $trashEntry = null; - foreach ($listing as $entry) { - if ($entry['original-location'] === $originalPath) { - $trashEntry = $entry; - break; - } - } - - Assert::assertNotNull( - $trashEntry, - "The first trash entry was not found while looking for trashbin entry '$path' of user '$user'" - ); - $topPath = "/" . $this->featureContext->getBasePath() . "/remote.php/dav/trash-bin/$user"; - $topPath = WebDavHelper::sanitizeUrl($topPath); - $path = str_replace($topPath, "", $trashEntry["href"]); - - $this->listTrashbinFolder($user, $path, null, null, $depth); } /** @@ -516,7 +575,7 @@ private function convertTrashbinHref(string $href):string { public function tryToDeleteFileFromTrashbin(?string $user, ?string $originalPath, ?string $asUser = null, ?string $password = null):int { $user = $this->featureContext->getActualUsername($user); $asUser = $asUser ?? $user; - $listing = $this->listTrashbinFolder($user, null); + $listing = $this->listTrashbinFolder($user); $originalPath = \trim($originalPath, '/'); $numItemsDeleted = 0; @@ -610,7 +669,7 @@ public function asFileOrFolderExistsInTrash(?string $user, ?string $path):void { if (\count($sections) !== 1) { // TODO: handle deeper structures - $listing = $this->listTrashbinFolder($user, \basename(\rtrim($firstEntry['href'], '/'))); + $listing = $this->listTrashbinFolderCollection($user, \basename(\rtrim($firstEntry['href'], '/'))); } if ($techPreviewHadToBeEnabled) { @@ -652,7 +711,7 @@ public function asFileOrFolderExistsInTrash(?string $user, ?string $path):void { private function isInTrash(?string $user, ?string $originalPath):bool { $techPreviewHadToBeEnabled = $this->occContext->enableDAVTechPreview(); $res = $this->featureContext->getResponse(); - $listing = $this->listTrashbinFolder($user, null); + $listing = $this->listTrashbinFolder($user); $this->featureContext->setResponse($res); if ($techPreviewHadToBeEnabled) { @@ -716,7 +775,7 @@ private function sendUndeleteRequest(string $user, string $trashItemHRef, string */ private function restoreElement(string $user, string $originalPath, ?string $destinationPath = null, bool $throwExceptionIfNotFound = true, ?string $asUser = null, ?string $password = null):?ResponseInterface { $asUser = $asUser ?? $user; - $listing = $this->listTrashbinFolder($user, null); + $listing = $this->listTrashbinFolder($user); $originalPath = \trim($originalPath, '/'); if ($destinationPath === null) { $destinationPath = $originalPath; @@ -755,7 +814,7 @@ private function restoreElement(string $user, string $originalPath, ?string $des */ public function restoreFileWithoutDestination(string $user, string $originalPath):ResponseInterface { $asUser = $asUser ?? $user; - $listing = $this->listTrashbinFolder($user, null); + $listing = $this->listTrashbinFolder($user); $originalPath = \trim($originalPath, '/'); foreach ($listing as $entry) { @@ -997,7 +1056,7 @@ public function followingElementsAreInTrashCheckingOriginalPath( * @throws Exception */ private function findFirstTrashedEntry(string $user, string $name):?array { - $listing = $this->listTrashbinFolder($user, '/'); + $listing = $this->listTrashbinFolder($user); foreach ($listing as $entry) { if ($entry['name'] === $name) { diff --git a/tests/acceptance/features/bootstrap/WebDav.php b/tests/acceptance/features/bootstrap/WebDav.php index d59c5a450c9c..f32523e9747a 100644 --- a/tests/acceptance/features/bootstrap/WebDav.php +++ b/tests/acceptance/features/bootstrap/WebDav.php @@ -103,6 +103,36 @@ trait WebDav { private $chunkingToUse = null; + /** + * The ability to do requests with depth infinity is disabled by default. + * This remembers when the setting dav.propfind.depth_infinity has been + * enabled, so that test code can make use of it as appropriate. + * + * @var bool + */ + private $davPropfindDepthInfinityEnabled = false; + + /** + * @return void + */ + public function davPropfindDepthInfinityEnabled():void { + $this->davPropfindDepthInfinityEnabled = true; + } + + /** + * @return void + */ + public function davPropfindDepthInfinityDisabled():void { + $this->davPropfindDepthInfinityEnabled = false; + } + + /** + * @return bool + */ + public function davPropfindDepthInfinityIsEnabled():bool { + return $this->davPropfindDepthInfinityEnabled; + } + /** * @param int $lastUploadDeleteTime * @@ -1996,7 +2026,7 @@ public function userShouldNotSeeTheElementsIfUpperAndLowerCaseUsernameDifferent( } /** - * asserts that a the user can or cannot see a list of files/folders by propfind + * asserts that the user can or cannot see a list of files/folders by propfind * * @param string $user * @param TableNode $elements @@ -2014,34 +2044,73 @@ public function checkElementList( ):void { $user = $this->getActualUsername($user); $this->verifyTableNodeColumnsCount($elements, 1); - $responseXmlObject = $this->listFolderAndReturnResponseXml( - $user, - "/", - "infinity" - ); $elementRows = $elements->getRows(); $elementsSimplified = $this->simplifyArray($elementRows); - foreach ($elementsSimplified as $expectedElement) { - // Allow the table of expected elements to have entries that do - // not have to specify the "implied" leading slash, or have multiple - // leading slashes, to make scenario outlines more flexible - $expectedElement = $this->encodePath($expectedElement); - $expectedElement = "/" . \ltrim($expectedElement, "/"); - $webdavPath = "/" . $this->getFullDavFilesPath($user) . $expectedElement; - $element = $responseXmlObject->xpath( - "//d:response/d:href[text() = \"$webdavPath\"]" + if ($this->davPropfindDepthInfinityIsEnabled()) { + // get a full "infinite" list of the user's root folder in one request + // and process that to check the elements (resources) + $responseXmlObject = $this->listFolderAndReturnResponseXml( + $user, + "/", + "infinity" ); - if ($expectedToBeListed - && (!isset($element[0]) || $element[0]->__toString() !== $webdavPath) - ) { - Assert::fail( - "$webdavPath is not in propfind answer but should be" + foreach ($elementsSimplified as $expectedElement) { + // Allow the table of expected elements to have entries that do + // not have to specify the "implied" leading slash, or have multiple + // leading slashes, to make scenario outlines more flexible + $expectedElement = $this->encodePath($expectedElement); + $expectedElement = "/" . \ltrim($expectedElement, "/"); + $webdavPath = "/" . $this->getFullDavFilesPath($user) . $expectedElement; + $element = $responseXmlObject->xpath( + "//d:response/d:href[text() = \"$webdavPath\"]" ); - } elseif (!$expectedToBeListed && isset($element[0]) - ) { - Assert::fail( - "$webdavPath is in propfind answer but should not be" + if ($expectedToBeListed + && (!isset($element[0]) || $element[0]->__toString() !== $webdavPath) + ) { + Assert::fail( + "$webdavPath is not in propfind answer but should be" + ); + } elseif (!$expectedToBeListed && isset($element[0]) + ) { + Assert::fail( + "$webdavPath is in propfind answer but should not be" + ); + } + } + } else { + // do a PROPFIND for each element + foreach ($elementsSimplified as $elementToRequest) { + // Allow the table of expected elements to have entries that do + // not have to specify the "implied" leading slash, or have multiple + // leading slashes, to make scenario outlines more flexible + $elementToRequest = "/" . \ltrim($elementToRequest, "/"); + // Note: in the request we ask to do a PROPFIND on a resource like: + // /some-folder with spaces/sub-folder + // but the response has encoded values for the special characters like: + // /some-folder%20with%20spaces/sub-folder + // So we need both $elementToRequest and $expectedElement + $expectedElement = $this->encodePath($elementToRequest); + $responseXmlObject = $this->listFolderAndReturnResponseXml( + $user, + $elementToRequest, + "1" ); + $webdavPath = "/" . $this->getFullDavFilesPath($user) . $expectedElement; + $element = $responseXmlObject->xpath( + "//d:response/d:href[text() = \"$webdavPath\"]" + ); + if ($expectedToBeListed + && (!isset($element[0]) || $element[0]->__toString() !== $webdavPath) + ) { + Assert::fail( + "$webdavPath is not in propfind answer but should be" + ); + } elseif (!$expectedToBeListed && isset($element[0]) + ) { + Assert::fail( + "$webdavPath is in propfind answer but should not be" + ); + } } } } diff --git a/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php b/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php index 2117e97a774a..ba42e1d042b1 100644 --- a/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php +++ b/tests/acceptance/features/bootstrap/WebDavPropertiesContext.php @@ -117,10 +117,7 @@ public function userGetsPropertiesOfFolder( $properties[] = $row["propertyName"]; } } - $depth = '0'; - if (\count($properties) > 1) { - $depth = 'infinity'; - } + $depth = "1"; $this->featureContext->setResponseXmlObject( $this->featureContext->listFolderAndReturnResponseXml( $user, @@ -136,11 +133,17 @@ public function userGetsPropertiesOfFolder( * @param string $user * @param string $path * @param TableNode $propertiesTable + * @param string $depth * * @return void * @throws Exception */ - public function getFollowingCommentPropertiesOfFileUsingWebDAVPropfindApi(string $user, string $path, TableNode $propertiesTable):void { + public function getFollowingCommentPropertiesOfFileUsingWebDAVPropfindApi( + string $user, + string $path, + TableNode $propertiesTable, + string $depth = "1" + ):void { $properties = null; $this->featureContext->verifyTableNodeColumns($propertiesTable, ["propertyName"]); $this->featureContext->verifyTableNodeColumnsCount($propertiesTable, 1); @@ -149,15 +152,10 @@ public function getFollowingCommentPropertiesOfFileUsingWebDAVPropfindApi(string $properties[] = $row["propertyName"]; } } - $depth = 0; $user = $this->featureContext->getActualUsername($user); $fileId = $this->featureContext->getFileIdForPath($user, $path); $commentsPath = "/comments/files/$fileId/"; - if (\count($properties) > 1) { - $depth = 'infinity'; - $this->featureContext->usingNewDavPath(); - } $this->featureContext->setResponseXmlObject( $this->featureContext->listFolderAndReturnResponseXml( $user, diff --git a/tests/acceptance/features/cliLocalStorage/propfindOnLocalStorage.feature b/tests/acceptance/features/cliLocalStorage/propfindOnLocalStorage.feature index 16dbd6f00446..29e4c1e16330 100644 --- a/tests/acceptance/features/cliLocalStorage/propfindOnLocalStorage.feature +++ b/tests/acceptance/features/cliLocalStorage/propfindOnLocalStorage.feature @@ -34,6 +34,7 @@ Feature: get file info using PROPFIND @skipOnEncryptionType:user-keys @issue-encryption-320 Scenario Outline: list files on root folder with external storage using depth infinity Given using DAV path + And the administrator has set depth_infinity_allowed to 1 When user "Alice" lists the resources in "/" with depth "infinity" using the WebDAV API Then the HTTP status code should be "207" And the propfind result of user "Alice" should contain only these entries: @@ -69,6 +70,7 @@ Feature: get file info using PROPFIND @skipOnEncryptionType:user-keys @issue-encryption-320 Scenario Outline: list files on external storage with depth infinity Given using DAV path + And the administrator has set depth_infinity_allowed to 1 When user "Alice" lists the resources in "/local_storage2" with depth "infinity" using the WebDAV API Then the HTTP status code should be "207" And the propfind result of user "Alice" should contain only these entries: diff --git a/tests/acceptance/features/cliLocalStorage/propfindOnLocalStorageOc10Issue39470.feature b/tests/acceptance/features/cliLocalStorage/propfindOnLocalStorageOc10Issue39470.feature index 7e4fed70b806..34420a0cfe51 100644 --- a/tests/acceptance/features/cliLocalStorage/propfindOnLocalStorageOc10Issue39470.feature +++ b/tests/acceptance/features/cliLocalStorage/propfindOnLocalStorageOc10Issue39470.feature @@ -18,6 +18,7 @@ Feature: get file info using PROPFIND @skipOnEncryptionType:user-keys @issue-encryption-320 Scenario Outline: list files on external storage that is currently unavailable Given using DAV path + And the administrator has set depth_infinity_allowed to 1 When the local storage mount for "/local_storage2" is renamed to "/new_local_storage" And user "Alice" lists the resources in "/local_storage2" with depth "infinity" using the WebDAV API Then the HTTP status code should be "207" @@ -34,6 +35,7 @@ Feature: get file info using PROPFIND @skipOnEncryptionType:user-keys @issue-encryption-320 Scenario Outline: list files on root folder with depth infinity when the external storage folder is unavailable Given using DAV path + And the administrator has set depth_infinity_allowed to 1 When the local storage mount for "/local_storage2" is renamed to "/new_local_storage" And user "Alice" lists the resources in "/" with depth "infinity" using the WebDAV API Then the HTTP status code should be "207"