From a87ec7f66f8be3474936616903331b95bfd5d1c8 Mon Sep 17 00:00:00 2001 From: Barry de Graaff Date: Wed, 23 Sep 2026 12:24:11 +0200 Subject: [PATCH 1/2] feat(files): add WebDAV BDELETE for bulk deletions Add a bounded BDELETE WebDAV method for deleting selected files in batches. Each target is deleted through the normal DAV unbind lifecycle without nested HTTP requests or mutation of the server request/response context. Keep the feature behind bulk_delete.enabled and advertise the batch limit through capabilities so Files can fall back to individual DELETE requests when the feature is disabled or unavailable. Support Ticket#96104279 See also: https://github.com/nextcloud/server/pull/64611 Assisted-by: ChatGPT:GPT-5.6-Sol --- .../composer/composer/autoload_classmap.php | 1 + .../dav/composer/composer/autoload_static.php | 1 + apps/dav/lib/BulkDelete/BulkDeletePlugin.php | 308 ++++++++++++++++++ apps/dav/lib/Capabilities.php | 9 +- apps/dav/lib/Server.php | 4 + .../unit/BulkDelete/BulkDeletePluginTest.php | 252 ++++++++++++++ apps/dav/tests/unit/CapabilitiesTest.php | 39 ++- apps/files/src/actions/deleteAction.ts | 6 + .../src/actions/deleteBatchUtils.spec.ts | 103 ++++++ apps/files/src/actions/deleteBatchUtils.ts | 83 +++++ apps/files/src/services/bulkDelete.spec.ts | 141 ++++++++ apps/files/src/services/bulkDelete.ts | 206 ++++++++++++ 12 files changed, 1143 insertions(+), 10 deletions(-) create mode 100644 apps/dav/lib/BulkDelete/BulkDeletePlugin.php create mode 100644 apps/dav/tests/unit/BulkDelete/BulkDeletePluginTest.php create mode 100644 apps/files/src/actions/deleteBatchUtils.spec.ts create mode 100644 apps/files/src/actions/deleteBatchUtils.ts create mode 100644 apps/files/src/services/bulkDelete.spec.ts create mode 100644 apps/files/src/services/bulkDelete.ts diff --git a/apps/dav/composer/composer/autoload_classmap.php b/apps/dav/composer/composer/autoload_classmap.php index 310f1ec83be3d..9d21b3baf7525 100644 --- a/apps/dav/composer/composer/autoload_classmap.php +++ b/apps/dav/composer/composer/autoload_classmap.php @@ -29,6 +29,7 @@ 'OCA\\DAV\\BackgroundJob\\UpdateCalendarResourcesRoomsBackgroundJob' => $baseDir . '/../lib/BackgroundJob/UpdateCalendarResourcesRoomsBackgroundJob.php', 'OCA\\DAV\\BackgroundJob\\UploadCleanup' => $baseDir . '/../lib/BackgroundJob/UploadCleanup.php', 'OCA\\DAV\\BackgroundJob\\UserStatusAutomation' => $baseDir . '/../lib/BackgroundJob/UserStatusAutomation.php', + 'OCA\\DAV\\BulkDelete\\BulkDeletePlugin' => $baseDir . '/../lib/BulkDelete/BulkDeletePlugin.php', 'OCA\\DAV\\BulkUpload\\BulkUploadPlugin' => $baseDir . '/../lib/BulkUpload/BulkUploadPlugin.php', 'OCA\\DAV\\BulkUpload\\MultipartRequestParser' => $baseDir . '/../lib/BulkUpload/MultipartRequestParser.php', 'OCA\\DAV\\CalDAV\\Activity\\Backend' => $baseDir . '/../lib/CalDAV/Activity/Backend.php', diff --git a/apps/dav/composer/composer/autoload_static.php b/apps/dav/composer/composer/autoload_static.php index 98a49d46284dd..70686320197bc 100644 --- a/apps/dav/composer/composer/autoload_static.php +++ b/apps/dav/composer/composer/autoload_static.php @@ -44,6 +44,7 @@ class ComposerStaticInitDAV 'OCA\\DAV\\BackgroundJob\\UpdateCalendarResourcesRoomsBackgroundJob' => __DIR__ . '/..' . '/../lib/BackgroundJob/UpdateCalendarResourcesRoomsBackgroundJob.php', 'OCA\\DAV\\BackgroundJob\\UploadCleanup' => __DIR__ . '/..' . '/../lib/BackgroundJob/UploadCleanup.php', 'OCA\\DAV\\BackgroundJob\\UserStatusAutomation' => __DIR__ . '/..' . '/../lib/BackgroundJob/UserStatusAutomation.php', + 'OCA\\DAV\\BulkDelete\\BulkDeletePlugin' => __DIR__ . '/..' . '/../lib/BulkDelete/BulkDeletePlugin.php', 'OCA\\DAV\\BulkUpload\\BulkUploadPlugin' => __DIR__ . '/..' . '/../lib/BulkUpload/BulkUploadPlugin.php', 'OCA\\DAV\\BulkUpload\\MultipartRequestParser' => __DIR__ . '/..' . '/../lib/BulkUpload/MultipartRequestParser.php', 'OCA\\DAV\\CalDAV\\Activity\\Backend' => __DIR__ . '/..' . '/../lib/CalDAV/Activity/Backend.php', diff --git a/apps/dav/lib/BulkDelete/BulkDeletePlugin.php b/apps/dav/lib/BulkDelete/BulkDeletePlugin.php new file mode 100644 index 0000000000000..48f2825f2ee16 --- /dev/null +++ b/apps/dav/lib/BulkDelete/BulkDeletePlugin.php @@ -0,0 +1,308 @@ + + * + * file1 + * folder/file2 + * + * + * + * Targets are relative to the collection addressed by the BDELETE request. + */ +class BulkDeletePlugin extends ServerPlugin { + public const MAX_FILES = 100; + public const MAX_BODY_BYTES = 1048576; + public const MAX_HREF_BYTES = 4096; + + private Server $server; + + public function __construct( + private string $userId, + private LoggerInterface $logger, + ) { + } + + #[\Override] + public function initialize(Server $server): void { + $this->server = $server; + $server->on('method:BDELETE', [$this, 'httpBulkDelete'], 10); + } + + #[\Override] + public function getPluginName(): string { + return 'bulk-delete'; + } + + /** + * @param string $path + * @return string[] + */ + #[\Override] + public function getHTTPMethods($path) { + return $this->isUserFilesPath($path) ? ['BDELETE'] : []; + } + + /** + * Delete each target through the standard DAV unbind lifecycle. + */ + public function httpBulkDelete(RequestInterface $request, ResponseInterface $response): bool { + $containerPath = trim($request->getPath(), '/'); + if (!$this->isUserFilesPath($containerPath)) { + return true; + } + + $container = $this->server->tree->getNodeForPath($containerPath); + if (!$container instanceof Directory) { + throw new Forbidden('BDELETE target must be a collection'); + } + + $contentType = strtolower(trim(explode(';', $request->getHeader('Content-Type') ?? '')[0])); + if (!in_array($contentType, ['application/xml', 'text/xml'], true)) { + throw new UnsupportedMediaType('BDELETE requires an XML request body'); + } + + $body = $request->getBody(); + $body = is_resource($body) ? stream_get_contents($body, self::MAX_BODY_BYTES + 1) : $body; + if (!is_string($body) || strlen($body) > self::MAX_BODY_BYTES) { + $response->setStatus(413); + $response->setHeader('Content-Length', '0'); + return false; + } + + $targets = $this->parseTargets($body, $containerPath); + $failures = []; + $stopped = false; + foreach ($targets as $target) { + if ($stopped) { + $failures[] = ['href' => $target['href'], 'status' => 424]; + continue; + } + + $status = $this->deleteTarget($target['path']); + if ($status !== 204) { + $failures[] = ['href' => $target['href'], 'status' => $status]; + $stopped = true; + } + } + + $response->setHeader('Cache-Control', 'no-store'); + if ($failures === []) { + $response->setStatus(204); + $response->setHeader('Content-Length', '0'); + return false; + } + + $response->setStatus(207); + $response->setHeader('Content-Type', 'application/xml; charset=utf-8'); + $response->setBody($this->buildMultiStatus($failures)); + return false; + } + + private function isUserFilesPath(string $path): bool { + $path = trim($path, '/'); + $root = 'files/' . $this->userId; + return $path === $root || str_starts_with($path, $root . '/'); + } + + /** + * @return list + */ + private function parseTargets(string $body, string $containerPath): array { + $previous = libxml_use_internal_errors(true); + try { + $document = new \DOMDocument(); + $loaded = $document->loadXML($body, LIBXML_NONET | LIBXML_NOBLANKS); + } finally { + libxml_clear_errors(); + libxml_use_internal_errors($previous); + } + + if (!$loaded || $document->doctype !== null) { + throw new BadRequest('Invalid BDELETE XML body'); + } + + $root = $document->documentElement; + if ($root === null || $root->namespaceURI !== 'DAV:' || $root->localName !== 'delete') { + throw new BadRequest('BDELETE body must have a DAV: delete element'); + } + + $hrefs = []; + foreach ($root->childNodes as $targetNode) { + if ($targetNode instanceof \DOMText && trim($targetNode->textContent) === '') { + continue; + } + if (!$targetNode instanceof \DOMElement || $targetNode->namespaceURI !== 'DAV:' || $targetNode->localName !== 'target') { + throw new BadRequest('BDELETE body may only contain DAV: target elements'); + } + + $targetHasHref = false; + foreach ($targetNode->childNodes as $hrefNode) { + if ($hrefNode instanceof \DOMText && trim($hrefNode->textContent) === '') { + continue; + } + if (!$hrefNode instanceof \DOMElement || $hrefNode->namespaceURI !== 'DAV:' || $hrefNode->localName !== 'href') { + throw new BadRequest('DAV: target may only contain DAV: href elements'); + } + foreach ($hrefNode->childNodes as $hrefChild) { + if ($hrefChild instanceof \DOMElement) { + throw new BadRequest('DAV: href must contain text only'); + } + } + $href = trim($hrefNode->textContent); + if ($href === '') { + throw new BadRequest('DAV: href must not be empty'); + } + $hrefs[] = $href; + $targetHasHref = true; + } + if (!$targetHasHref) { + throw new BadRequest('DAV: target must contain at least one DAV: href'); + } + } + + if (count($hrefs) < 1 || count($hrefs) > self::MAX_FILES) { + throw new BadRequest('BDELETE requires between 1 and 100 target hrefs'); + } + + $targets = []; + $paths = []; + foreach ($hrefs as $href) { + $path = $this->resolveHref($containerPath, $href); + if (isset($paths[$path])) { + throw new BadRequest('Duplicate BDELETE targets are not allowed'); + } + $paths[$path] = true; + $targets[] = ['href' => $href, 'path' => $path]; + } + return $targets; + } + + private function resolveHref(string $containerPath, string $href): string { + if (strlen($href) > self::MAX_HREF_BYTES + || str_starts_with($href, '/') + || str_contains($href, '?') + || str_contains($href, '#') + || preg_match('/[\x00-\x1f\x7f\\\\]/', $href) + ) { + throw new BadRequest('BDELETE href must be a relative resource path'); + } + + $decoded = []; + foreach (explode('/', $href) as $segment) { + if ($segment === '' || preg_match('/%(?![0-9A-Fa-f]{2})/', $segment)) { + throw new BadRequest('Invalid BDELETE href'); + } + $segment = rawurldecode($segment); + if ($segment === '' || $segment === '.' || $segment === '..' + || str_contains($segment, '/') || str_contains($segment, '\\') + || preg_match('/[\x00-\x1f\x7f]/', $segment) + || !mb_check_encoding($segment, 'UTF-8') + ) { + throw new BadRequest('Invalid BDELETE href'); + } + $decoded[] = $segment; + } + + $path = $containerPath . '/' . implode('/', $decoded); + if (strlen($path) > self::MAX_HREF_BYTES) { + throw new BadRequest('BDELETE target path is too long'); + } + return $path; + } + + private function deleteTarget(string $path): int { + try { + $node = $this->server->tree->getNodeForPath($path); + if (!$node instanceof File || $node->getInternalPath() === '') { + return 403; + } + + if (!$this->server->emit('beforeUnbind', [$path])) { + return 403; + } + $this->server->tree->delete($path); + $this->server->emit('afterUnbind', [$path]); + return 204; + } catch (Exception $exception) { + $status = $exception->getHTTPCode(); + return $status >= 400 && $status <= 599 ? $status : 500; + } catch (\Throwable $exception) { + $this->logger->error('BDELETE target failed', [ + 'path' => $path, + 'exception' => $exception, + ]); + return 500; + } + } + + /** + * @param list $failures + */ + private function buildMultiStatus(array $failures): string { + $document = new \DOMDocument('1.0', 'UTF-8'); + $document->formatOutput = true; + $multiStatus = $document->createElementNS('DAV:', 'd:multistatus'); + $document->appendChild($multiStatus); + + foreach ($failures as $failure) { + $item = $document->createElementNS('DAV:', 'd:response'); + $href = $document->createElementNS('DAV:', 'd:href'); + $href->appendChild($document->createTextNode($failure['href'])); + $status = $document->createElementNS('DAV:', 'd:status'); + $status->appendChild($document->createTextNode($this->statusLine($failure['status']))); + $item->appendChild($href); + $item->appendChild($status); + $multiStatus->appendChild($item); + } + + return $document->saveXML(); + } + + private function statusLine(int $status): string { + $reason = match ($status) { + 400 => 'Bad Request', + 401 => 'Unauthorized', + 403 => 'Forbidden', + 404 => 'Not Found', + 405 => 'Method Not Allowed', + 409 => 'Conflict', + 412 => 'Precondition Failed', + 413 => 'Content Too Large', + 415 => 'Unsupported Media Type', + 423 => 'Locked', + 424 => 'Failed Dependency', + 429 => 'Too Many Requests', + 500 => 'Internal Server Error', + 503 => 'Service Unavailable', + 507 => 'Insufficient Storage', + default => 'Error', + }; + return 'HTTP/1.1 ' . $status . ' ' . $reason; + } +} diff --git a/apps/dav/lib/Capabilities.php b/apps/dav/lib/Capabilities.php index e710ea3b3d1b1..fccc2683a9900 100644 --- a/apps/dav/lib/Capabilities.php +++ b/apps/dav/lib/Capabilities.php @@ -9,6 +9,7 @@ namespace OCA\DAV; +use OCA\DAV\BulkDelete\BulkDeletePlugin; use OCP\Capabilities\ICapability; use OCP\IConfig; use OCP\User\IAvailabilityCoordinator; @@ -21,7 +22,7 @@ public function __construct( } /** - * @return array{dav: array{chunking: string, public_shares_chunking: bool, search_supports_creation_time: bool, search_supports_upload_time: bool, search_supports_last_activity: bool, bulkupload?: string, absence-supported?: bool, absence-replacement?: bool}} + * @return array{dav: array{chunking: string, public_shares_chunking: bool, search_supports_creation_time: bool, search_supports_upload_time: bool, search_supports_last_activity: bool, bulkupload?: string, bulk_delete?: array{version: string, max_files: int}, absence-supported?: bool, absence-replacement?: bool}} */ #[\Override] public function getCapabilities() { @@ -37,6 +38,12 @@ public function getCapabilities() { if ($this->config->getSystemValueBool('bulkupload.enabled', true)) { $capabilities['dav']['bulkupload'] = '1.0'; } + if ($this->config->getSystemValueBool('bulk_delete.enabled', true)) { + $capabilities['dav']['bulk_delete'] = [ + 'version' => '1.0', + 'max_files' => BulkDeletePlugin::MAX_FILES, + ]; + } if ($this->coordinator->isEnabled()) { $capabilities['dav']['absence-supported'] = true; $capabilities['dav']['absence-replacement'] = true; diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index ce5964e1c10f7..e621bba4ab246 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -10,6 +10,7 @@ use OC\Files\Filesystem; use OCA\DAV\AppInfo\PluginManager; +use OCA\DAV\BulkDelete\BulkDeletePlugin; use OCA\DAV\BulkUpload\BulkUploadPlugin; use OCA\DAV\CalDAV\BirthdayCalendar\EnablePlugin; use OCA\DAV\CalDAV\BirthdayService; @@ -386,6 +387,9 @@ public function __construct( $view, \OCP\Server::get(IFilesMetadataManager::class) )); + if ($config->getSystemValueBool('bulk_delete.enabled', true)) { + $this->server->addPlugin(new BulkDeletePlugin($user->getUID(), $logger)); + } $this->server->addPlugin( new BulkUploadPlugin( $userFolder, diff --git a/apps/dav/tests/unit/BulkDelete/BulkDeletePluginTest.php b/apps/dav/tests/unit/BulkDelete/BulkDeletePluginTest.php new file mode 100644 index 0000000000000..a95b9f0f9da5c --- /dev/null +++ b/apps/dav/tests/unit/BulkDelete/BulkDeletePluginTest.php @@ -0,0 +1,252 @@ +tree = $this->createMock(Tree::class); + $this->tree->method('getNodeForPath')->willReturnCallback(function (string $path) { + if (!isset($this->nodes[$path])) { + throw new NotFound(); + } + return $this->nodes[$path]; + }); + $this->tree->method('delete')->willReturnCallback(function (string $path): void { + $this->nodes[$path]->delete(); + $this->deleted[] = $path; + unset($this->nodes[$path]); + }); + $this->nodes['files/alice'] = $this->createMock(Directory::class); + $this->server = new Server($this->tree); + $this->server->setBaseUri('/nextcloud/remote.php/dav/'); + $this->plugin = new BulkDeletePlugin('alice', new NullLogger()); + $this->server->addPlugin($this->plugin); + } + + private function addFile(string $path): File&MockObject { + $file = $this->createMock(File::class); + $file->method('getInternalPath')->willReturn('files/' . $path); + $this->nodes['files/alice/' . $path] = $file; + return $file; + } + + /** @param string[] $hrefs */ + private function request(array $hrefs, string $container = 'files/alice'): Response { + $body = ''; + foreach ($hrefs as $href) { + $body .= '' . htmlspecialchars($href, ENT_XML1 | ENT_QUOTES, 'UTF-8') . ''; + } + $body .= ''; + $request = new Request('BDELETE', '/nextcloud/remote.php/dav/' . $container . '/', [ + 'Content-Type' => 'application/xml; charset=utf-8', + ], $body); + $request->setBaseUrl($this->server->getBaseUri()); + $response = new Response(); + $this->server->httpRequest = $request; + $this->server->httpResponse = $response; + $this->server->invokeMethod($request, $response, false); + self::assertSame($request, $this->server->httpRequest); + self::assertSame($response, $this->server->httpResponse); + return $response; + } + + /** @return array */ + private function failures(Response $response): array { + self::assertSame(207, $response->getStatus()); + $document = new \DOMDocument(); + self::assertTrue($document->loadXML($response->getBodyAsString())); + $xpath = new \DOMXPath($document); + $xpath->registerNamespace('d', 'DAV:'); + $failures = []; + foreach ($xpath->query('/d:multistatus/d:response') as $item) { + $href = $xpath->evaluate('string(d:href)', $item); + $status = $xpath->evaluate('string(d:status)', $item); + self::assertMatchesRegularExpression('/^HTTP\/1\.1 \d{3} /', $status); + $failures[$href] = (int)substr($status, 9, 3); + } + return $failures; + } + + public function testUsesSingleRequestAndDavUnbindLifecycle(): void { + $this->addFile('one.txt'); + $this->addFile('two.txt'); + $before = []; + $after = []; + $methodCalls = 0; + $this->server->on('beforeMethod:BDELETE', function (Request $request) use (&$methodCalls): void { + self::assertSame($request, $this->server->httpRequest); + $methodCalls++; + }); + $this->server->on('beforeUnbind', function (string $path) use (&$before): void { + $before[] = $path; + }); + $this->server->on('afterUnbind', function (string $path) use (&$after): void { + $after[] = $path; + }); + + $response = $this->request(['one.txt', 'two.txt']); + self::assertSame(204, $response->getStatus()); + self::assertSame(1, $methodCalls); + self::assertSame(['files/alice/one.txt', 'files/alice/two.txt'], $before); + self::assertSame($before, $this->deleted); + self::assertSame($before, $after); + } + + public function testFailureStopsBatchAndReportsFailedDependency(): void { + $this->addFile('one'); + $this->addFile('two')->method('delete')->willThrowException(new Forbidden()); + $this->addFile('three')->expects(self::never())->method('delete'); + + $response = $this->request(['one', 'two', 'three']); + self::assertSame([ + 'two' => 403, + 'three' => 424, + ], $this->failures($response)); + self::assertSame(['files/alice/one'], $this->deleted); + } + + public function testMissingFileIsReportedInMultistatus(): void { + $response = $this->request(['missing']); + self::assertSame(['missing' => 404], $this->failures($response)); + } + + public function testFoldersAreRejected(): void { + $this->nodes['files/alice/folder'] = $this->createMock(Directory::class); + $response = $this->request(['folder']); + self::assertSame(['folder' => 403], $this->failures($response)); + } + + public function testMountRootFileIsRejected(): void { + $file = $this->createMock(File::class); + $file->method('getInternalPath')->willReturn(''); + $file->expects(self::never())->method('delete'); + $this->nodes['files/alice/shared-file'] = $file; + $response = $this->request(['shared-file']); + self::assertSame(['shared-file' => 403], $this->failures($response)); + } + + public function testSilentUnbindVetoIsNotReportedAsSuccess(): void { + $this->addFile('one')->expects(self::never())->method('delete'); + $this->server->on('beforeUnbind', static fn (): bool => false); + $response = $this->request(['one']); + self::assertSame(['one' => 403], $this->failures($response)); + } + + public function testThrowableDoesNotLeakDetails(): void { + $this->addFile('one')->method('delete')->willThrowException(new \RuntimeException('secret-storage-detail')); + $response = $this->request(['one']); + self::assertStringNotContainsString('secret-storage-detail', $response->getBodyAsString()); + self::assertSame(['one' => 500], $this->failures($response)); + } + + public function testUnicodeAndLiteralPercentAreDecodedExactlyOnce(): void { + $names = ['café + #.txt', '日本語.txt', 'literal%2Fname.txt', 'emoji-😀.txt']; + $hrefs = []; + foreach ($names as $name) { + $this->addFile('folder/' . $name); + $hrefs[] = 'folder/' . rawurlencode($name); + } + $response = $this->request($hrefs); + self::assertSame(204, $response->getStatus()); + self::assertSame(array_map(static fn (string $name): string => 'files/alice/folder/' . $name, $names), $this->deleted); + } + + public function testTargetIsAlwaysRelativeToRequestCollection(): void { + $this->nodes['files/alice/folder'] = $this->createMock(Directory::class); + $this->addFile('folder/files/bob/one'); + $response = $this->request(['files/bob/one'], 'files/alice/folder'); + self::assertSame(204, $response->getStatus()); + self::assertSame(['files/alice/folder/files/bob/one'], $this->deleted); + } + + #[DataProvider('invalidBodies')] + public function testInvalidBodyHasNoSideEffects(string $body): void { + $this->tree->expects(self::never())->method('delete'); + $request = new Request('BDELETE', '/nextcloud/remote.php/dav/files/alice/', [ + 'Content-Type' => 'application/xml', + ], $body); + $request->setBaseUrl($this->server->getBaseUri()); + $this->expectException(BadRequest::class); + $this->plugin->httpBulkDelete($request, new Response()); + } + + public static function invalidBodies(): array { + $href = static fn (string $value): string => '' . $value . ''; + $wrap = static fn (string $value): string => '' . $value . ''; + return [ + 'empty target' => [$wrap('')], + 'absolute path' => [$wrap($href('/one'))], + 'traversal' => [$wrap($href('../one'))], + 'encoded traversal' => [$wrap($href('%2e%2e/one'))], + 'double slash' => [$wrap($href('folder//one'))], + 'query' => [$wrap($href('one?x=1'))], + 'fragment' => [$wrap($href('one#x'))], + 'bad percent' => [$wrap($href('one%2'))], + 'encoded slash' => [$wrap($href('one%2Ftwo'))], + 'duplicate' => [$wrap($href('one') . $href('one'))], + 'wrong root' => ['' . $href('one') . ''], + 'unexpected element' => [$wrap('' . $href('one'))], + 'doctype' => [']>&x;'], + ]; + } + + public function testContentTypeIsRequired(): void { + $request = new Request('BDELETE', '/nextcloud/remote.php/dav/files/alice/', [], ''); + $request->setBaseUrl($this->server->getBaseUri()); + $this->expectException(UnsupportedMediaType::class); + $this->plugin->httpBulkDelete($request, new Response()); + } + + public function testOversizedStreamIsRejectedWithoutDeletion(): void { + $body = fopen('php://temp', 'w+'); + fwrite($body, str_repeat('x', BulkDeletePlugin::MAX_BODY_BYTES + 1)); + rewind($body); + $request = new Request('BDELETE', '/nextcloud/remote.php/dav/files/alice/', ['Content-Type' => 'application/xml'], $body); + $request->setBaseUrl($this->server->getBaseUri()); + $response = new Response(); + try { + self::assertFalse($this->plugin->httpBulkDelete($request, $response)); + self::assertSame(413, $response->getStatus()); + self::assertSame([], $this->deleted); + } finally { + fclose($body); + } + } + + public function testOptionsAdvertisesBDeleteOnUserFilesTree(): void { + $request = new Request('OPTIONS', '/nextcloud/remote.php/dav/files/alice/'); + $request->setBaseUrl($this->server->getBaseUri()); + $response = new Response(); + $this->server->invokeMethod($request, $response, false); + self::assertStringContainsString('BDELETE', $response->getHeader('Allow') ?? ''); + } +} diff --git a/apps/dav/tests/unit/CapabilitiesTest.php b/apps/dav/tests/unit/CapabilitiesTest.php index 1c8edd250ca14..9d1ab52b4faf6 100644 --- a/apps/dav/tests/unit/CapabilitiesTest.php +++ b/apps/dav/tests/unit/CapabilitiesTest.php @@ -19,10 +19,12 @@ class CapabilitiesTest extends TestCase { public function testGetCapabilities(): void { $config = $this->createMock(IConfig::class); - $config->expects($this->once()) + $config->expects($this->exactly(2)) ->method('getSystemValueBool') - ->with('bulkupload.enabled', $this->isType('bool')) - ->willReturn(false); + ->willReturnMap([ + ['bulkupload.enabled', true, false], + ['bulk_delete.enabled', true, false], + ]); $coordinator = $this->createMock(IAvailabilityCoordinator::class); $coordinator->expects($this->once()) ->method('isEnabled') @@ -42,10 +44,12 @@ public function testGetCapabilities(): void { public function testGetCapabilitiesWithBulkUpload(): void { $config = $this->createMock(IConfig::class); - $config->expects($this->once()) + $config->expects($this->exactly(2)) ->method('getSystemValueBool') - ->with('bulkupload.enabled', $this->isType('bool')) - ->willReturn(true); + ->willReturnMap([ + ['bulkupload.enabled', true, true], + ['bulk_delete.enabled', true, false], + ]); $coordinator = $this->createMock(IAvailabilityCoordinator::class); $coordinator->expects($this->once()) ->method('isEnabled') @@ -66,10 +70,12 @@ public function testGetCapabilitiesWithBulkUpload(): void { public function testGetCapabilitiesWithAbsence(): void { $config = $this->createMock(IConfig::class); - $config->expects($this->once()) + $config->expects($this->exactly(2)) ->method('getSystemValueBool') - ->with('bulkupload.enabled', $this->isType('bool')) - ->willReturn(false); + ->willReturnMap([ + ['bulkupload.enabled', true, false], + ['bulk_delete.enabled', true, false], + ]); $coordinator = $this->createMock(IAvailabilityCoordinator::class); $coordinator->expects($this->once()) ->method('isEnabled') @@ -88,4 +94,19 @@ public function testGetCapabilitiesWithAbsence(): void { ]; $this->assertSame($expected, $capabilities->getCapabilities()); } + + public function testGetCapabilitiesWithBulkDelete(): void { + $config = $this->createMock(IConfig::class); + $config->method('getSystemValueBool')->willReturnMap([ + ['bulkupload.enabled', true, false], + ['bulk_delete.enabled', true, true], + ]); + $coordinator = $this->createMock(IAvailabilityCoordinator::class); + $coordinator->method('isEnabled')->willReturn(false); + $capabilities = new Capabilities($config, $coordinator); + $this->assertSame([ + 'version' => '1.0', + 'max_files' => 100, + ], $capabilities->getCapabilities()['dav']['bulk_delete']); + } } diff --git a/apps/files/src/actions/deleteAction.ts b/apps/files/src/actions/deleteAction.ts index b5e06f85c7f15..c916e56d2b54c 100644 --- a/apps/files/src/actions/deleteAction.ts +++ b/apps/files/src/actions/deleteAction.ts @@ -14,6 +14,7 @@ import { loadState } from '@nextcloud/initial-state' import { t } from '@nextcloud/l10n' import PQueue from 'p-queue' import { logger } from '../utils/logger.ts' +import { deleteNodesInBatches } from './deleteBatchUtils.ts' import { askConfirmation, canDisconnectOnly, canUnshareOnly, deleteNode, displayName, shouldAskForConfirmation } from './deleteUtils.ts' // TODO: once the files app is migrated to the new frontend use the import instead: @@ -89,6 +90,11 @@ export const action: IFileAction = { return Promise.all(nodes.map(() => null)) } + var batchResult = deleteNodesInBatches(nodes, view, queue) + if (batchResult !== null) { + return batchResult + } + // Map each node to a promise that resolves with the result of exec(node) const promises = nodes.map((node) => { // Create a promise that resolves with the result of exec(node) diff --git a/apps/files/src/actions/deleteBatchUtils.spec.ts b/apps/files/src/actions/deleteBatchUtils.spec.ts new file mode 100644 index 0000000000000..075a6a72ccd77 --- /dev/null +++ b/apps/files/src/actions/deleteBatchUtils.spec.ts @@ -0,0 +1,103 @@ +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { INode, IView } from '@nextcloud/files' + +import { getCurrentUser } from '@nextcloud/auth' +import axios from '@nextcloud/axios' +import { getCapabilities } from '@nextcloud/capabilities' +import { emit } from '@nextcloud/event-bus' +import { File, Folder, Permission } from '@nextcloud/files' +import PQueue from 'p-queue' +import { beforeEach, expect, test, vi } from 'vitest' +import { deleteNodesInBatches } from './deleteBatchUtils.ts' + +vi.mock('@nextcloud/auth') +vi.mock('@nextcloud/axios') +vi.mock('@nextcloud/capabilities') +vi.mock('@nextcloud/event-bus') +vi.mock('@nextcloud/router', () => ({ generateRemoteUrl: () => 'http://nextcloud.local/remote.php/dav' })) + +var view = { id: 'files', name: 'Files' } as IView +var queue: PQueue + +function file(id: number, name = `file-${id}.txt`): File { + return new File({ + id, + source: `http://nextcloud.local/remote.php/dav/files/alice/${name}`, + owner: 'alice', + root: '/files/alice', + mime: 'text/plain', + permissions: Permission.ALL, + }) +} + +beforeEach(() => { + vi.resetAllMocks() + queue = new PQueue({ concurrency: 5 }) + vi.mocked(getCurrentUser).mockReturnValue({ uid: 'alice' } as ReturnType) + vi.mocked(getCapabilities).mockReturnValue({ files: { undelete: true }, dav: { bulk_delete: { version: '1.0', max_files: 100 } } }) + vi.mocked(axios.request).mockResolvedValue({ status: 204, data: '' }) +}) + +test('uses one BDELETE request and only confirmed deletion events', async () => { + var nodes = [file(1), file(2)] + expect(await deleteNodesInBatches(nodes, view, queue)).toEqual([true, true]) + expect(axios.request).toHaveBeenCalledTimes(1) + expect(axios.delete).not.toHaveBeenCalled() + expect(axios.request).toHaveBeenCalledWith({ + method: 'BDELETE', + url: 'http://nextcloud.local/remote.php/dav/files/alice/', + data: 'file-1.txtfile-2.txt', + headers: { 'Content-Type': 'application/xml; charset=utf-8' }, + }) + expect(emit).toHaveBeenCalledWith('files:node:deleted', nodes[0]) + expect(emit).toHaveBeenCalledWith('files:node:deleted', nodes[1]) +}) + +test('does not use BDELETE without the advertised capability', () => { + vi.mocked(getCapabilities).mockReturnValue({ files: { undelete: true } }) + expect(deleteNodesInBatches([file(1), file(2)], view, queue)).toBeNull() + expect(axios.request).not.toHaveBeenCalled() +}) + +test('keeps permanent deletion on the existing path', () => { + expect(deleteNodesInBatches([file(1), file(2)], { ...view, id: 'trashbin' }, queue)).toBeNull() + vi.mocked(getCapabilities).mockReturnValue({ files: { undelete: false }, dav: { bulk_delete: { version: '1.0', max_files: 100 } } }) + expect(deleteNodesInBatches([file(1), file(2)], view, queue)).toBeNull() +}) + +test('keeps mixed file-folder selections on the existing path', () => { + var folder = new Folder({ id: 3, source: 'http://nextcloud.local/remote.php/dav/files/alice/folder', root: '/files/alice', owner: 'alice', permissions: Permission.ALL }) + expect(deleteNodesInBatches([file(1), folder], view, queue)).toBeNull() +}) + +test('keeps shared and external mount roots on the existing path', () => { + for (var mountType of ['shared', 'external']) { + var mounted = file(1) + mounted.attributes['is-mount-root'] = true + mounted.attributes['mount-type'] = mountType + expect(deleteNodesInBatches([mounted, file(2)], view, queue)).toBeNull() + } +}) + +test('rejects foreign origins, user roots and public share paths', () => { + for (var source of [ + 'https://other.invalid/remote.php/dav/files/alice/one', + 'http://nextcloud.local/remote.php/dav/files/bob/one', + 'http://nextcloud.local/remote.php/dav/public-files/token/one', + ]) { + var node = { ...file(1), type: file(1).type, permissions: Permission.ALL, fileid: 1, attributes: {}, encodedSource: source } as INode + expect(deleteNodesInBatches([node, file(2)], view, queue)).toBeNull() + } +}) + +test('does not retry or fall back after a BDELETE timeout', async () => { + vi.mocked(axios.request).mockRejectedValue(new Error('timeout')) + expect(await deleteNodesInBatches([file(1), file(2)], view, queue)).toEqual([false, false]) + expect(axios.request).toHaveBeenCalledTimes(1) + expect(axios.delete).not.toHaveBeenCalled() + expect(emit).not.toHaveBeenCalled() +}) diff --git a/apps/files/src/actions/deleteBatchUtils.ts b/apps/files/src/actions/deleteBatchUtils.ts new file mode 100644 index 0000000000000..3ae817245aa6f --- /dev/null +++ b/apps/files/src/actions/deleteBatchUtils.ts @@ -0,0 +1,83 @@ +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { INode, IView } from '@nextcloud/files' +import type PQueue from 'p-queue' +import type { BulkDeleteItem } from '../services/bulkDelete.ts' + +import { getCurrentUser } from '@nextcloud/auth' +import axios from '@nextcloud/axios' +import { getCapabilities } from '@nextcloud/capabilities' +import { emit } from '@nextcloud/event-bus' +import { FileType, Permission } from '@nextcloud/files' +import { generateRemoteUrl } from '@nextcloud/router' +import { createBDeleteBody, isBulkDeleteItem, parseBDeleteResponse, runBulkDelete } from '../services/bulkDelete.ts' +import { logger } from '../utils/logger.ts' + +interface BulkDeleteCapabilities { + files?: { undelete?: boolean } + dav?: { bulk_delete?: { version?: string, max_files?: number } } +} + +/** + * Return null before sending any request when the existing action must be used. + * Mixed selections, folders, trash entries, public shares and mount roots keep + * their existing semantics. node.source must belong to this user's DAV root. + */ +export function deleteNodesInBatches(nodes: INode[], view: IView, queue: PQueue): Promise | null { + var capabilities = getCapabilities() as BulkDeleteCapabilities + var support = capabilities?.dav?.bulk_delete + var user = getCurrentUser() + if (nodes.length < 2 || view.id === 'trashbin' || capabilities?.files?.undelete !== true + || support?.version !== '1.0' || !Number.isInteger(support.max_files) || support.max_files! < 1 || !user) { + return null + } + + var davRoot = generateRemoteUrl('dav').replace(/\/$/, '') + var userRoot = new URL(`${davRoot}/files/${encodeURIComponent(user.uid)}/`, window.location.href) + var files: BulkDeleteItem[] = [] + try { + for (var node of nodes) { + if (node.type !== FileType.File || node.attributes['is-mount-root'] === true + || !(node.permissions & Permission.DELETE) || typeof node.fileid !== 'number') { + return null + } + var source = new URL(node.encodedSource, window.location.href) + if (source.origin !== userRoot.origin || !source.pathname.startsWith(userRoot.pathname) + || source.search !== '' || source.hash !== '') { + return null + } + var file = { path: '/' + decodeURIComponent(source.pathname.slice(userRoot.pathname.length)), fileId: node.fileid } + if (!isBulkDeleteItem(file)) { + return null + } + files.push(file) + } + } catch { + return null + } + + return runBulkDelete(files, { + batchSize: Math.min(100, support.max_files!), + concurrency: 5, + async request(batch) { + return queue.add(async () => { + var response = await axios.request({ + method: 'BDELETE', + url: userRoot.toString(), + data: createBDeleteBody(batch), + headers: { 'Content-Type': 'application/xml; charset=utf-8' }, + }) + return parseBDeleteResponse(response.status, response.data, batch) + }) + }, + onDeleted(index) { + emit('files:node:deleted', nodes[index]!) + }, + onError(error) { + logger.error('BDELETE failed; refresh the file list before retrying', { error }) + }, + }) +} diff --git a/apps/files/src/services/bulkDelete.spec.ts b/apps/files/src/services/bulkDelete.spec.ts new file mode 100644 index 0000000000000..a2769a48dd236 --- /dev/null +++ b/apps/files/src/services/bulkDelete.spec.ts @@ -0,0 +1,141 @@ +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import type { BulkDeleteItem } from './bulkDelete.ts' + +import { expect, test, vi } from 'vitest' +import { createBDeleteBody, getBDeleteHref, isBulkDeleteItem, parseBDeleteResponse, runBulkDelete, validateBulkDeleteResponse } from './bulkDelete.ts' + +function files(count: number): BulkDeleteItem[] { + return Array.from({ length: count }, (_, index) => ({ path: `/file-${index}.txt`, fileId: index + 1 })) +} + +function success(batch: BulkDeleteItem[]) { + return { results: batch.map((file) => ({ ...file, status: 204, attempted: true })), stopped: false } +} + +function multiStatus(entries: Array<[string, number]>): string { + var responses = entries.map(([href, status]) => `${href}HTTP/1.1 ${status} Error`).join('') + return `${responses}` +} + +test.each([[385, 4], [970, 10], [1100, 11]])('%i files use %i BDELETE requests', async (count, requests) => { + var request = vi.fn(async (batch: BulkDeleteItem[]) => success(batch)) + var onDeleted = vi.fn() + var result = await runBulkDelete(files(count), { batchSize: 100, request, onDeleted, onError: vi.fn() }) + expect(request).toHaveBeenCalledTimes(requests) + expect(onDeleted).toHaveBeenCalledTimes(count) + expect(result).toEqual(Array(count).fill(true)) + expect(request.mock.calls.every(([batch]) => batch.length <= 100)).toBe(true) +}) + +test('never has more than five requests in flight', async () => { + var active = 0 + var maximum = 0 + await runBulkDelete(files(1100), { + batchSize: 100, + async request(batch) { + active++ + maximum = Math.max(maximum, active) + await new Promise((resolve) => setTimeout(resolve, 1)) + active-- + return success(batch) + }, + onDeleted: vi.fn(), + onError: vi.fn(), + }) + expect(maximum).toBe(5) +}) + +test('creates an Exchange-style DAV delete body', () => { + var batch = [ + { path: '/folder/café + #.txt', fileId: 1 }, + { path: '/literal%2Fname.txt', fileId: 2 }, + ] + var body = createBDeleteBody(batch) + expect(body).toContain('') + expect(body).toContain('folder/caf%C3%A9%20%2B%20%23.txt') + expect(body).toContain('literal%252Fname.txt') +}) + +test('204 means every target was deleted', () => { + var batch = files(2) + expect(parseBDeleteResponse(204, '', batch)).toEqual(success(batch)) +}) + +test('207 maps the failed target and unattempted suffix', () => { + var batch = files(3) + var response = parseBDeleteResponse(207, multiStatus([ + [getBDeleteHref(batch[1]!), 403], + [getBDeleteHref(batch[2]!), 424], + ]), batch) + expect(response).toEqual({ + results: [ + { ...batch[0]!, status: 204, attempted: true }, + { ...batch[1]!, status: 403, attempted: true }, + { ...batch[2]!, status: 424, attempted: false }, + ], + stopped: true, + }) +}) + +test('malformed or mismatched multistatus cannot remove visible nodes', () => { + var batch = files(2) + expect(() => parseBDeleteResponse(207, '', batch)).toThrow() + expect(() => parseBDeleteResponse(207, multiStatus([['other.txt', 403]]), batch)).toThrow() + expect(() => parseBDeleteResponse(200, '', batch)).toThrow() +}) + +test('partial failure emits success only for confirmed items and stops unsent batches', async () => { + var request = vi.fn(async (batch: BulkDeleteItem[]) => ({ + results: batch.map((file, index) => ({ ...file, status: [204, 403, 424][index], attempted: index < 2 })), + stopped: true, + })) + var onDeleted = vi.fn() + var onError = vi.fn() + var result = await runBulkDelete(files(6), { batchSize: 3, concurrency: 1, request, onDeleted, onError }) + expect(result).toEqual([true, false, false, false, false, false]) + expect(request).toHaveBeenCalledTimes(1) + expect(onDeleted).toHaveBeenCalledTimes(1) + expect(onDeleted).toHaveBeenCalledWith(0) + expect(onError).toHaveBeenCalledTimes(1) +}) + +test('a lost response is never retried', async () => { + var request = vi.fn(async () => { throw new Error('network timeout') }) + var onDeleted = vi.fn() + var result = await runBulkDelete(files(200), { batchSize: 100, concurrency: 1, request, onDeleted, onError: vi.fn() }) + expect(result.every((value) => !value)).toBe(true) + expect(request).toHaveBeenCalledTimes(1) + expect(onDeleted).not.toHaveBeenCalled() +}) + +test('response identities and the stopped suffix are validated', () => { + var batch = files(3) + var response = success(batch) + response.results[1]!.fileId = 999 + expect(() => validateBulkDeleteResponse(response, batch)).toThrow() + response = success(batch) + response.results[1]!.status = 403 + response.stopped = true + expect(() => validateBulkDeleteResponse(response, batch)).toThrow() +}) + +test.each(['/', '../file', '/a/../b', '/a//b', '/a/./b', '/a\\b', '/a\u0000b'])('rejects unsafe path %s', (path) => { + expect(isBulkDeleteItem({ path, fileId: 1 })).toBe(false) +}) + +test.each(['café + #.txt', '日本語.txt', 'literal%2Fname.txt', 'emoji-😀.txt'])('preserves UTF-8 path %s', (name) => { + var file = { path: `/folder/${name}`, fileId: 1 } + expect(isBulkDeleteItem(file)).toBe(true) + expect(getBDeleteHref(file)).not.toContain(' ') +}) + +test('rejects duplicate selections before any request', async () => { + var request = vi.fn() + var batch = files(1) + await runBulkDelete([batch[0]!, batch[0]!], { batchSize: 100, request, onDeleted: vi.fn(), onError: vi.fn() }) + expect(request).not.toHaveBeenCalled() +}) diff --git a/apps/files/src/services/bulkDelete.ts b/apps/files/src/services/bulkDelete.ts new file mode 100644 index 0000000000000..06f9ed3fe428f --- /dev/null +++ b/apps/files/src/services/bulkDelete.ts @@ -0,0 +1,206 @@ +/*! + * SPDX-FileCopyrightText: 2026 Nextcloud contributors + * SPDX-License-Identifier: AGPL-3.0-or-later + */ + +import { encodePath } from '@nextcloud/paths' + +export interface BulkDeleteItem { + path: string + fileId: number +} + +export interface BulkDeleteResult extends BulkDeleteItem { + status: number + attempted: boolean +} + +export interface BulkDeleteResponse { + results: BulkDeleteResult[] + stopped: boolean +} + +export interface BulkDeleteOptions { + batchSize: number + concurrency?: number + request: (files: BulkDeleteItem[]) => Promise + onDeleted: (index: number) => void + onError: (error: unknown) => void +} + +/** Both sides validate the same raw UTF-8, user-relative paths. */ +export function isBulkDeleteItem(value: BulkDeleteItem): boolean { + return Number.isSafeInteger(value.fileId) && value.fileId > 0 + && typeof value.path === 'string' && value.path.startsWith('/') + && new TextEncoder().encode(value.path).length <= 4096 + && !/[\u0000-\u001f\u007f\\]/.test(value.path) + && value.path.slice(1).split('/').every((part) => part !== '' && part !== '.' && part !== '..') +} + +export function getBDeleteHref(file: BulkDeleteItem): string { + return encodePath(file.path).replace(/^\/+/, '') +} + +function escapeXml(value: string): string { + return value + .replaceAll('&', '&') + .replaceAll('<', '<') + .replaceAll('>', '>') + .replaceAll('"', '"') + .replaceAll("'", ''') +} + +export function createBDeleteBody(files: BulkDeleteItem[]): string { + var hrefs = files.map((file) => `${escapeXml(getBDeleteHref(file))}`).join('') + return `${hrefs}` +} + +/** + * Convert the Exchange-style BDELETE 204/207 response into the existing local + * result shape used by the batching scheduler. + */ +export function parseBDeleteResponse(status: number, body: unknown, files: BulkDeleteItem[]): BulkDeleteResponse { + if (status === 204) { + return { + results: files.map((file) => ({ ...file, status: 204, attempted: true })), + stopped: false, + } + } + if (status !== 207 || typeof body !== 'string') { + throw new Error('Invalid BDELETE response; refresh the file list before retrying') + } + + var document = new DOMParser().parseFromString(body, 'application/xml') + if (document.querySelector('parsererror') !== null + || document.documentElement.namespaceURI !== 'DAV:' + || document.documentElement.localName !== 'multistatus') { + throw new Error('Invalid BDELETE multistatus response') + } + + var hrefToIndex = new Map(files.map((file, index) => [getBDeleteHref(file), index])) + var failures = new Map() + for (var response of Array.from(document.documentElement.children)) { + if (response.namespaceURI !== 'DAV:' || response.localName !== 'response') { + throw new Error('Invalid BDELETE multistatus child') + } + var hrefElements = Array.from(response.children).filter((element) => element.namespaceURI === 'DAV:' && element.localName === 'href') + var statusElements = Array.from(response.children).filter((element) => element.namespaceURI === 'DAV:' && element.localName === 'status') + if (hrefElements.length !== 1 || statusElements.length !== 1) { + throw new Error('Invalid BDELETE multistatus item') + } + var href = hrefElements[0]!.textContent ?? '' + var match = /^HTTP\/1\.[01]\s+(\d{3})(?:\s|$)/.exec(statusElements[0]!.textContent ?? '') + var index = hrefToIndex.get(href) + var itemStatus = match ? Number.parseInt(match[1]!, 10) : 0 + if (index === undefined || failures.has(index) || itemStatus < 400 || itemStatus > 599) { + throw new Error('BDELETE response does not match the requested files') + } + failures.set(index, itemStatus) + } + if (failures.size === 0) { + throw new Error('BDELETE 207 response did not contain any failures') + } + + var firstFailure = Math.min(...failures.keys()) + var results: BulkDeleteResult[] = files.map((file, index) => { + if (index < firstFailure) { + if (failures.has(index)) { + throw new Error('Invalid BDELETE failure ordering') + } + return { ...file, status: 204, attempted: true } + } + if (index === firstFailure) { + var itemStatus = failures.get(index) + if (itemStatus === undefined || itemStatus === 424) { + throw new Error('Invalid first BDELETE failure') + } + return { ...file, status: itemStatus, attempted: true } + } + var itemStatus = failures.get(index) + if (itemStatus !== 424) { + throw new Error('Invalid stopped BDELETE response') + } + return { ...file, status: 424, attempted: false } + }) + + return { results, stopped: true } +} + +/** Reject malformed or mismatched local results before emitting success events. */ +export function validateBulkDeleteResponse(value: unknown, files: BulkDeleteItem[]): BulkDeleteResponse { + if (!value || typeof value !== 'object' || !('results' in value) || !Array.isArray(value.results) + || !('stopped' in value) || typeof value.stopped !== 'boolean' || value.results.length !== files.length) { + throw new Error('Invalid bulk delete response; refresh the file list before retrying') + } + var stopped = false + for (var index = 0; index < files.length; index++) { + var result = value.results[index] + var file = files[index]! + if (!result || result.path !== file.path || result.fileId !== file.fileId + || !Number.isInteger(result.status) || typeof result.attempted !== 'boolean') { + throw new Error('Bulk delete response does not match the requested files') + } + if (stopped) { + if (result.attempted !== false || result.status !== 424) { + throw new Error('Invalid bulk delete stopped-batch response') + } + } else { + if (result.attempted !== true || (result.status !== 204 && (result.status < 400 || result.status > 599))) { + throw new Error('Invalid bulk delete item status') + } + stopped = result.status !== 204 + } + } + if (value.stopped !== stopped) { + throw new Error('Inconsistent bulk delete response') + } + return value as BulkDeleteResponse +} + +/** + * Bounded workers preserve input-order results even when requests finish out of + * order. Never retry a BDELETE or fall back to DELETE after dispatching a batch. + */ +export async function runBulkDelete(files: BulkDeleteItem[], options: BulkDeleteOptions): Promise { + var results = files.map(() => false) + var batchSize = options.batchSize + var concurrency = options.concurrency ?? 5 + if (!Number.isInteger(batchSize) || batchSize < 1 || batchSize > 100 + || !Number.isInteger(concurrency) || concurrency < 1 || concurrency > 5 + || !files.every(isBulkDeleteItem) + || new Set(files.map((file) => file.path)).size !== files.length + || new Set(files.map((file) => file.fileId)).size !== files.length) { + options.onError(new Error('Invalid bulk delete selection or limits')) + return results + } + + var nextIndex = 0 + var stopped = false + async function worker(): Promise { + while (!stopped && nextIndex < files.length) { + var start = nextIndex + nextIndex += batchSize + var batch = files.slice(start, start + batchSize) + try { + var response = validateBulkDeleteResponse(await options.request(batch), batch) + if (response.stopped) { + stopped = true + } + for (var offset = 0; offset < response.results.length; offset++) { + if (response.results[offset]!.status === 204) { + results[start + offset] = true + options.onDeleted(start + offset) + } + } + if (response.stopped) { + options.onError(new Error('Bulk deletion stopped after a failed item; refresh before retrying')) + } + } catch (error) { + stopped = true + options.onError(error) + } + } + } + await Promise.all(Array.from({ length: Math.min(concurrency, Math.ceil(files.length / batchSize)) }, () => worker())) + return results +} From a9db2c5303f234251212aaa236a8590781c4c78c Mon Sep 17 00:00:00 2001 From: Barry de Graaff Date: Wed, 23 Sep 2026 17:28:50 +0200 Subject: [PATCH 2/2] Assisted-by: ChatGPT:GPT-5.6-Sol Signed-off-by: Barry de Graaff Signed-off-by: Barry de Graaff --- apps/dav/lib/BulkDelete/BulkDeletePlugin.php | 79 ++++++++----------- .../unit/BulkDelete/BulkDeletePluginTest.php | 45 ++++++++--- apps/files/src/actions/deleteAction.ts | 2 +- .../src/actions/deleteBatchUtils.spec.ts | 16 ++-- apps/files/src/actions/deleteBatchUtils.ts | 20 ++--- apps/files/src/services/bulkDelete.spec.ts | 48 +++++------ apps/files/src/services/bulkDelete.ts | 56 ++++++------- 7 files changed, 140 insertions(+), 126 deletions(-) diff --git a/apps/dav/lib/BulkDelete/BulkDeletePlugin.php b/apps/dav/lib/BulkDelete/BulkDeletePlugin.php index 48f2825f2ee16..0b11c70571284 100644 --- a/apps/dav/lib/BulkDelete/BulkDeletePlugin.php +++ b/apps/dav/lib/BulkDelete/BulkDeletePlugin.php @@ -20,6 +20,7 @@ use Sabre\DAV\ServerPlugin; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; +use Sabre\Xml\ParseException; /** * Implements the non-standard BDELETE WebDAV method for bounded file batches. @@ -134,55 +135,45 @@ private function isUserFilesPath(string $path): bool { * @return list */ private function parseTargets(string $body, string $containerPath): array { - $previous = libxml_use_internal_errors(true); - try { - $document = new \DOMDocument(); - $loaded = $document->loadXML($body, LIBXML_NONET | LIBXML_NOBLANKS); - } finally { - libxml_clear_errors(); - libxml_use_internal_errors($previous); + if (stripos($body, 'doctype !== null) { + try { + $elements = $this->server->xml->expect('{DAV:}delete', $body); + } catch (ParseException) { throw new BadRequest('Invalid BDELETE XML body'); } - $root = $document->documentElement; - if ($root === null || $root->namespaceURI !== 'DAV:' || $root->localName !== 'delete') { - throw new BadRequest('BDELETE body must have a DAV: delete element'); + if ($elements === null) { + $elements = []; + } + if (!is_array($elements)) { + throw new BadRequest('BDELETE body must contain DAV: target elements'); } $hrefs = []; - foreach ($root->childNodes as $targetNode) { - if ($targetNode instanceof \DOMText && trim($targetNode->textContent) === '') { - continue; - } - if (!$targetNode instanceof \DOMElement || $targetNode->namespaceURI !== 'DAV:' || $targetNode->localName !== 'target') { + foreach ($elements as $target) { + if (!is_array($target) || ($target['name'] ?? null) !== '{DAV:}target') { throw new BadRequest('BDELETE body may only contain DAV: target elements'); } - $targetHasHref = false; - foreach ($targetNode->childNodes as $hrefNode) { - if ($hrefNode instanceof \DOMText && trim($hrefNode->textContent) === '') { - continue; - } - if (!$hrefNode instanceof \DOMElement || $hrefNode->namespaceURI !== 'DAV:' || $hrefNode->localName !== 'href') { + $targetValue = $target['value'] ?? null; + if (!is_array($targetValue) || $targetValue === []) { + throw new BadRequest('DAV: target must contain at least one DAV: href'); + } + + foreach ($targetValue as $hrefElement) { + if (!is_array($hrefElement) || ($hrefElement['name'] ?? null) !== '{DAV:}href' + || !is_string($hrefElement['value'] ?? null)) { throw new BadRequest('DAV: target may only contain DAV: href elements'); } - foreach ($hrefNode->childNodes as $hrefChild) { - if ($hrefChild instanceof \DOMElement) { - throw new BadRequest('DAV: href must contain text only'); - } - } - $href = trim($hrefNode->textContent); + + $href = trim($hrefElement['value']); if ($href === '') { throw new BadRequest('DAV: href must not be empty'); } $hrefs[] = $href; - $targetHasHref = true; - } - if (!$targetHasHref) { - throw new BadRequest('DAV: target must contain at least one DAV: href'); } } @@ -265,23 +256,21 @@ private function deleteTarget(string $path): int { * @param list $failures */ private function buildMultiStatus(array $failures): string { - $document = new \DOMDocument('1.0', 'UTF-8'); - $document->formatOutput = true; - $multiStatus = $document->createElementNS('DAV:', 'd:multistatus'); - $document->appendChild($multiStatus); + $writer = $this->server->xml->getWriter(); + $writer->openMemory(); + $writer->setIndent(true); + $writer->startDocument(); + $writer->startElement('{DAV:}multistatus'); foreach ($failures as $failure) { - $item = $document->createElementNS('DAV:', 'd:response'); - $href = $document->createElementNS('DAV:', 'd:href'); - $href->appendChild($document->createTextNode($failure['href'])); - $status = $document->createElementNS('DAV:', 'd:status'); - $status->appendChild($document->createTextNode($this->statusLine($failure['status']))); - $item->appendChild($href); - $item->appendChild($status); - $multiStatus->appendChild($item); + $writer->startElement('{DAV:}response'); + $writer->writeElement('{DAV:}href', $failure['href']); + $writer->writeElement('{DAV:}status', $this->statusLine($failure['status'])); + $writer->endElement(); } - return $document->saveXML(); + $writer->endElement(); + return $writer->outputMemory(); } private function statusLine(int $status): string { diff --git a/apps/dav/tests/unit/BulkDelete/BulkDeletePluginTest.php b/apps/dav/tests/unit/BulkDelete/BulkDeletePluginTest.php index a95b9f0f9da5c..ebab6d6dc6a17 100644 --- a/apps/dav/tests/unit/BulkDelete/BulkDeletePluginTest.php +++ b/apps/dav/tests/unit/BulkDelete/BulkDeletePluginTest.php @@ -24,6 +24,7 @@ use Sabre\DAV\Tree; use Sabre\HTTP\Request; use Sabre\HTTP\Response; +use Sabre\Xml\Service; class BulkDeletePluginTest extends TestCase { private Server $server; @@ -61,11 +62,18 @@ private function addFile(string $path): File&MockObject { /** @param string[] $hrefs */ private function request(array $hrefs, string $container = 'files/alice'): Response { - $body = ''; + $writer = (new Service())->getWriter(); + $writer->openMemory(); + $writer->startDocument(); + $writer->startElement('{DAV:}delete'); + $writer->startElement('{DAV:}target'); foreach ($hrefs as $href) { - $body .= '' . htmlspecialchars($href, ENT_XML1 | ENT_QUOTES, 'UTF-8') . ''; + $writer->writeElement('{DAV:}href', $href); } - $body .= ''; + $writer->endElement(); + $writer->endElement(); + $body = $writer->outputMemory(); + $request = new Request('BDELETE', '/nextcloud/remote.php/dav/' . $container . '/', [ 'Content-Type' => 'application/xml; charset=utf-8', ], $body); @@ -82,14 +90,23 @@ private function request(array $hrefs, string $container = 'files/alice'): Respo /** @return array */ private function failures(Response $response): array { self::assertSame(207, $response->getStatus()); - $document = new \DOMDocument(); - self::assertTrue($document->loadXML($response->getBodyAsString())); - $xpath = new \DOMXPath($document); - $xpath->registerNamespace('d', 'DAV:'); + $elements = (new Service())->expect('{DAV:}multistatus', $response->getBodyAsString()); + self::assertIsArray($elements); $failures = []; - foreach ($xpath->query('/d:multistatus/d:response') as $item) { - $href = $xpath->evaluate('string(d:href)', $item); - $status = $xpath->evaluate('string(d:status)', $item); + foreach ($elements as $item) { + self::assertSame('{DAV:}response', $item['name']); + self::assertIsArray($item['value']); + $href = null; + $status = null; + foreach ($item['value'] as $child) { + if ($child['name'] === '{DAV:}href') { + $href = $child['value']; + } elseif ($child['name'] === '{DAV:}status') { + $status = $child['value']; + } + } + self::assertIsString($href); + self::assertIsString($status); self::assertMatchesRegularExpression('/^HTTP\/1\.1 \d{3} /', $status); $failures[$href] = (int)substr($status, 9, 3); } @@ -180,6 +197,13 @@ public function testUnicodeAndLiteralPercentAreDecodedExactlyOnce(): void { self::assertSame(array_map(static fn (string $name): string => 'files/alice/folder/' . $name, $names), $this->deleted); } + public function testFailureResponsePreservesEncodedHref(): void { + $this->addFile('café + #.txt')->method('delete')->willThrowException(new Forbidden()); + $href = 'caf%C3%A9%20%2B%20%23.txt'; + $response = $this->request([$href]); + self::assertSame([$href => 403], $this->failures($response)); + } + public function testTargetIsAlwaysRelativeToRequestCollection(): void { $this->nodes['files/alice/folder'] = $this->createMock(Directory::class); $this->addFile('folder/files/bob/one'); @@ -215,6 +239,7 @@ public static function invalidBodies(): array { 'duplicate' => [$wrap($href('one') . $href('one'))], 'wrong root' => ['' . $href('one') . ''], 'unexpected element' => [$wrap('' . $href('one'))], + 'nested href element' => [$wrap('')], 'doctype' => [']>&x;'], ]; } diff --git a/apps/files/src/actions/deleteAction.ts b/apps/files/src/actions/deleteAction.ts index c916e56d2b54c..2fcb256acdcf9 100644 --- a/apps/files/src/actions/deleteAction.ts +++ b/apps/files/src/actions/deleteAction.ts @@ -90,7 +90,7 @@ export const action: IFileAction = { return Promise.all(nodes.map(() => null)) } - var batchResult = deleteNodesInBatches(nodes, view, queue) + const batchResult = deleteNodesInBatches(nodes, view, queue) if (batchResult !== null) { return batchResult } diff --git a/apps/files/src/actions/deleteBatchUtils.spec.ts b/apps/files/src/actions/deleteBatchUtils.spec.ts index 075a6a72ccd77..4506b36dd0164 100644 --- a/apps/files/src/actions/deleteBatchUtils.spec.ts +++ b/apps/files/src/actions/deleteBatchUtils.spec.ts @@ -20,8 +20,8 @@ vi.mock('@nextcloud/capabilities') vi.mock('@nextcloud/event-bus') vi.mock('@nextcloud/router', () => ({ generateRemoteUrl: () => 'http://nextcloud.local/remote.php/dav' })) -var view = { id: 'files', name: 'Files' } as IView -var queue: PQueue +const view = { id: 'files', name: 'Files' } as IView +let queue: PQueue function file(id: number, name = `file-${id}.txt`): File { return new File({ @@ -43,7 +43,7 @@ beforeEach(() => { }) test('uses one BDELETE request and only confirmed deletion events', async () => { - var nodes = [file(1), file(2)] + const nodes = [file(1), file(2)] expect(await deleteNodesInBatches(nodes, view, queue)).toEqual([true, true]) expect(axios.request).toHaveBeenCalledTimes(1) expect(axios.delete).not.toHaveBeenCalled() @@ -70,13 +70,13 @@ test('keeps permanent deletion on the existing path', () => { }) test('keeps mixed file-folder selections on the existing path', () => { - var folder = new Folder({ id: 3, source: 'http://nextcloud.local/remote.php/dav/files/alice/folder', root: '/files/alice', owner: 'alice', permissions: Permission.ALL }) + const folder = new Folder({ id: 3, source: 'http://nextcloud.local/remote.php/dav/files/alice/folder', root: '/files/alice', owner: 'alice', permissions: Permission.ALL }) expect(deleteNodesInBatches([file(1), folder], view, queue)).toBeNull() }) test('keeps shared and external mount roots on the existing path', () => { - for (var mountType of ['shared', 'external']) { - var mounted = file(1) + for (const mountType of ['shared', 'external']) { + const mounted = file(1) mounted.attributes['is-mount-root'] = true mounted.attributes['mount-type'] = mountType expect(deleteNodesInBatches([mounted, file(2)], view, queue)).toBeNull() @@ -84,12 +84,12 @@ test('keeps shared and external mount roots on the existing path', () => { }) test('rejects foreign origins, user roots and public share paths', () => { - for (var source of [ + for (const source of [ 'https://other.invalid/remote.php/dav/files/alice/one', 'http://nextcloud.local/remote.php/dav/files/bob/one', 'http://nextcloud.local/remote.php/dav/public-files/token/one', ]) { - var node = { ...file(1), type: file(1).type, permissions: Permission.ALL, fileid: 1, attributes: {}, encodedSource: source } as INode + const node = { ...file(1), type: file(1).type, permissions: Permission.ALL, fileid: 1, attributes: {}, encodedSource: source } as INode expect(deleteNodesInBatches([node, file(2)], view, queue)).toBeNull() } }) diff --git a/apps/files/src/actions/deleteBatchUtils.ts b/apps/files/src/actions/deleteBatchUtils.ts index 3ae817245aa6f..eb85a2e798317 100644 --- a/apps/files/src/actions/deleteBatchUtils.ts +++ b/apps/files/src/actions/deleteBatchUtils.ts @@ -27,29 +27,29 @@ interface BulkDeleteCapabilities { * their existing semantics. node.source must belong to this user's DAV root. */ export function deleteNodesInBatches(nodes: INode[], view: IView, queue: PQueue): Promise | null { - var capabilities = getCapabilities() as BulkDeleteCapabilities - var support = capabilities?.dav?.bulk_delete - var user = getCurrentUser() + const capabilities = getCapabilities() as BulkDeleteCapabilities + const support = capabilities?.dav?.bulk_delete + const user = getCurrentUser() if (nodes.length < 2 || view.id === 'trashbin' || capabilities?.files?.undelete !== true || support?.version !== '1.0' || !Number.isInteger(support.max_files) || support.max_files! < 1 || !user) { return null } - var davRoot = generateRemoteUrl('dav').replace(/\/$/, '') - var userRoot = new URL(`${davRoot}/files/${encodeURIComponent(user.uid)}/`, window.location.href) - var files: BulkDeleteItem[] = [] + const davRoot = generateRemoteUrl('dav').replace(/\/$/, '') + const userRoot = new URL(`${davRoot}/files/${encodeURIComponent(user.uid)}/`, window.location.href) + const files: BulkDeleteItem[] = [] try { - for (var node of nodes) { + for (const node of nodes) { if (node.type !== FileType.File || node.attributes['is-mount-root'] === true || !(node.permissions & Permission.DELETE) || typeof node.fileid !== 'number') { return null } - var source = new URL(node.encodedSource, window.location.href) + const source = new URL(node.encodedSource, window.location.href) if (source.origin !== userRoot.origin || !source.pathname.startsWith(userRoot.pathname) || source.search !== '' || source.hash !== '') { return null } - var file = { path: '/' + decodeURIComponent(source.pathname.slice(userRoot.pathname.length)), fileId: node.fileid } + const file = { path: '/' + decodeURIComponent(source.pathname.slice(userRoot.pathname.length)), fileId: node.fileid } if (!isBulkDeleteItem(file)) { return null } @@ -64,7 +64,7 @@ export function deleteNodesInBatches(nodes: INode[], view: IView, queue: PQueue) concurrency: 5, async request(batch) { return queue.add(async () => { - var response = await axios.request({ + const response = await axios.request({ method: 'BDELETE', url: userRoot.toString(), data: createBDeleteBody(batch), diff --git a/apps/files/src/services/bulkDelete.spec.ts b/apps/files/src/services/bulkDelete.spec.ts index a2769a48dd236..164c6026931cc 100644 --- a/apps/files/src/services/bulkDelete.spec.ts +++ b/apps/files/src/services/bulkDelete.spec.ts @@ -17,14 +17,14 @@ function success(batch: BulkDeleteItem[]) { } function multiStatus(entries: Array<[string, number]>): string { - var responses = entries.map(([href, status]) => `${href}HTTP/1.1 ${status} Error`).join('') + const responses = entries.map(([href, status]) => `${href}HTTP/1.1 ${status} Error`).join('') return `${responses}` } test.each([[385, 4], [970, 10], [1100, 11]])('%i files use %i BDELETE requests', async (count, requests) => { - var request = vi.fn(async (batch: BulkDeleteItem[]) => success(batch)) - var onDeleted = vi.fn() - var result = await runBulkDelete(files(count), { batchSize: 100, request, onDeleted, onError: vi.fn() }) + const request = vi.fn(async (batch: BulkDeleteItem[]) => success(batch)) + const onDeleted = vi.fn() + const result = await runBulkDelete(files(count), { batchSize: 100, request, onDeleted, onError: vi.fn() }) expect(request).toHaveBeenCalledTimes(requests) expect(onDeleted).toHaveBeenCalledTimes(count) expect(result).toEqual(Array(count).fill(true)) @@ -32,8 +32,8 @@ test.each([[385, 4], [970, 10], [1100, 11]])('%i files use %i BDELETE requests', }) test('never has more than five requests in flight', async () => { - var active = 0 - var maximum = 0 + let active = 0 + let maximum = 0 await runBulkDelete(files(1100), { batchSize: 100, async request(batch) { @@ -50,24 +50,24 @@ test('never has more than five requests in flight', async () => { }) test('creates an Exchange-style DAV delete body', () => { - var batch = [ + const batch = [ { path: '/folder/café + #.txt', fileId: 1 }, { path: '/literal%2Fname.txt', fileId: 2 }, ] - var body = createBDeleteBody(batch) + const body = createBDeleteBody(batch) expect(body).toContain('') expect(body).toContain('folder/caf%C3%A9%20%2B%20%23.txt') expect(body).toContain('literal%252Fname.txt') }) test('204 means every target was deleted', () => { - var batch = files(2) + const batch = files(2) expect(parseBDeleteResponse(204, '', batch)).toEqual(success(batch)) }) test('207 maps the failed target and unattempted suffix', () => { - var batch = files(3) - var response = parseBDeleteResponse(207, multiStatus([ + const batch = files(3) + const response = parseBDeleteResponse(207, multiStatus([ [getBDeleteHref(batch[1]!), 403], [getBDeleteHref(batch[2]!), 424], ]), batch) @@ -82,20 +82,20 @@ test('207 maps the failed target and unattempted suffix', () => { }) test('malformed or mismatched multistatus cannot remove visible nodes', () => { - var batch = files(2) + const batch = files(2) expect(() => parseBDeleteResponse(207, '', batch)).toThrow() expect(() => parseBDeleteResponse(207, multiStatus([['other.txt', 403]]), batch)).toThrow() expect(() => parseBDeleteResponse(200, '', batch)).toThrow() }) test('partial failure emits success only for confirmed items and stops unsent batches', async () => { - var request = vi.fn(async (batch: BulkDeleteItem[]) => ({ + const request = vi.fn(async (batch: BulkDeleteItem[]) => ({ results: batch.map((file, index) => ({ ...file, status: [204, 403, 424][index], attempted: index < 2 })), stopped: true, })) - var onDeleted = vi.fn() - var onError = vi.fn() - var result = await runBulkDelete(files(6), { batchSize: 3, concurrency: 1, request, onDeleted, onError }) + const onDeleted = vi.fn() + const onError = vi.fn() + const result = await runBulkDelete(files(6), { batchSize: 3, concurrency: 1, request, onDeleted, onError }) expect(result).toEqual([true, false, false, false, false, false]) expect(request).toHaveBeenCalledTimes(1) expect(onDeleted).toHaveBeenCalledTimes(1) @@ -104,17 +104,17 @@ test('partial failure emits success only for confirmed items and stops unsent ba }) test('a lost response is never retried', async () => { - var request = vi.fn(async () => { throw new Error('network timeout') }) - var onDeleted = vi.fn() - var result = await runBulkDelete(files(200), { batchSize: 100, concurrency: 1, request, onDeleted, onError: vi.fn() }) + const request = vi.fn(async () => { throw new Error('network timeout') }) + const onDeleted = vi.fn() + const result = await runBulkDelete(files(200), { batchSize: 100, concurrency: 1, request, onDeleted, onError: vi.fn() }) expect(result.every((value) => !value)).toBe(true) expect(request).toHaveBeenCalledTimes(1) expect(onDeleted).not.toHaveBeenCalled() }) test('response identities and the stopped suffix are validated', () => { - var batch = files(3) - var response = success(batch) + const batch = files(3) + let response = success(batch) response.results[1]!.fileId = 999 expect(() => validateBulkDeleteResponse(response, batch)).toThrow() response = success(batch) @@ -128,14 +128,14 @@ test.each(['/', '../file', '/a/../b', '/a//b', '/a/./b', '/a\\b', '/a\u0000b'])( }) test.each(['café + #.txt', '日本語.txt', 'literal%2Fname.txt', 'emoji-😀.txt'])('preserves UTF-8 path %s', (name) => { - var file = { path: `/folder/${name}`, fileId: 1 } + const file = { path: `/folder/${name}`, fileId: 1 } expect(isBulkDeleteItem(file)).toBe(true) expect(getBDeleteHref(file)).not.toContain(' ') }) test('rejects duplicate selections before any request', async () => { - var request = vi.fn() - var batch = files(1) + const request = vi.fn() + const batch = files(1) await runBulkDelete([batch[0]!, batch[0]!], { batchSize: 100, request, onDeleted: vi.fn(), onError: vi.fn() }) expect(request).not.toHaveBeenCalled() }) diff --git a/apps/files/src/services/bulkDelete.ts b/apps/files/src/services/bulkDelete.ts index 06f9ed3fe428f..9c8c082cced30 100644 --- a/apps/files/src/services/bulkDelete.ts +++ b/apps/files/src/services/bulkDelete.ts @@ -51,7 +51,7 @@ function escapeXml(value: string): string { } export function createBDeleteBody(files: BulkDeleteItem[]): string { - var hrefs = files.map((file) => `${escapeXml(getBDeleteHref(file))}`).join('') + const hrefs = files.map((file) => `${escapeXml(getBDeleteHref(file))}`).join('') return `${hrefs}` } @@ -70,28 +70,28 @@ export function parseBDeleteResponse(status: number, body: unknown, files: BulkD throw new Error('Invalid BDELETE response; refresh the file list before retrying') } - var document = new DOMParser().parseFromString(body, 'application/xml') + const document = new DOMParser().parseFromString(body, 'application/xml') if (document.querySelector('parsererror') !== null || document.documentElement.namespaceURI !== 'DAV:' || document.documentElement.localName !== 'multistatus') { throw new Error('Invalid BDELETE multistatus response') } - var hrefToIndex = new Map(files.map((file, index) => [getBDeleteHref(file), index])) - var failures = new Map() - for (var response of Array.from(document.documentElement.children)) { + const hrefToIndex = new Map(files.map((file, index) => [getBDeleteHref(file), index])) + const failures = new Map() + for (const response of Array.from(document.documentElement.children)) { if (response.namespaceURI !== 'DAV:' || response.localName !== 'response') { throw new Error('Invalid BDELETE multistatus child') } - var hrefElements = Array.from(response.children).filter((element) => element.namespaceURI === 'DAV:' && element.localName === 'href') - var statusElements = Array.from(response.children).filter((element) => element.namespaceURI === 'DAV:' && element.localName === 'status') + const hrefElements = Array.from(response.children).filter((element) => element.namespaceURI === 'DAV:' && element.localName === 'href') + const statusElements = Array.from(response.children).filter((element) => element.namespaceURI === 'DAV:' && element.localName === 'status') if (hrefElements.length !== 1 || statusElements.length !== 1) { throw new Error('Invalid BDELETE multistatus item') } - var href = hrefElements[0]!.textContent ?? '' - var match = /^HTTP\/1\.[01]\s+(\d{3})(?:\s|$)/.exec(statusElements[0]!.textContent ?? '') - var index = hrefToIndex.get(href) - var itemStatus = match ? Number.parseInt(match[1]!, 10) : 0 + const href = hrefElements[0]!.textContent ?? '' + const match = /^HTTP\/1\.[01]\s+(\d{3})(?:\s|$)/.exec(statusElements[0]!.textContent ?? '') + const index = hrefToIndex.get(href) + const itemStatus = match ? Number.parseInt(match[1]!, 10) : 0 if (index === undefined || failures.has(index) || itemStatus < 400 || itemStatus > 599) { throw new Error('BDELETE response does not match the requested files') } @@ -101,8 +101,8 @@ export function parseBDeleteResponse(status: number, body: unknown, files: BulkD throw new Error('BDELETE 207 response did not contain any failures') } - var firstFailure = Math.min(...failures.keys()) - var results: BulkDeleteResult[] = files.map((file, index) => { + const firstFailure = Math.min(...failures.keys()) + const results: BulkDeleteResult[] = files.map((file, index) => { if (index < firstFailure) { if (failures.has(index)) { throw new Error('Invalid BDELETE failure ordering') @@ -110,13 +110,13 @@ export function parseBDeleteResponse(status: number, body: unknown, files: BulkD return { ...file, status: 204, attempted: true } } if (index === firstFailure) { - var itemStatus = failures.get(index) + const itemStatus = failures.get(index) if (itemStatus === undefined || itemStatus === 424) { throw new Error('Invalid first BDELETE failure') } return { ...file, status: itemStatus, attempted: true } } - var itemStatus = failures.get(index) + const itemStatus = failures.get(index) if (itemStatus !== 424) { throw new Error('Invalid stopped BDELETE response') } @@ -132,10 +132,10 @@ export function validateBulkDeleteResponse(value: unknown, files: BulkDeleteItem || !('stopped' in value) || typeof value.stopped !== 'boolean' || value.results.length !== files.length) { throw new Error('Invalid bulk delete response; refresh the file list before retrying') } - var stopped = false - for (var index = 0; index < files.length; index++) { - var result = value.results[index] - var file = files[index]! + let stopped = false + for (let index = 0; index < files.length; index++) { + const result = value.results[index] + const file = files[index]! if (!result || result.path !== file.path || result.fileId !== file.fileId || !Number.isInteger(result.status) || typeof result.attempted !== 'boolean') { throw new Error('Bulk delete response does not match the requested files') @@ -162,9 +162,9 @@ export function validateBulkDeleteResponse(value: unknown, files: BulkDeleteItem * order. Never retry a BDELETE or fall back to DELETE after dispatching a batch. */ export async function runBulkDelete(files: BulkDeleteItem[], options: BulkDeleteOptions): Promise { - var results = files.map(() => false) - var batchSize = options.batchSize - var concurrency = options.concurrency ?? 5 + const results = files.map(() => false) + const batchSize = options.batchSize + const concurrency = options.concurrency ?? 5 if (!Number.isInteger(batchSize) || batchSize < 1 || batchSize > 100 || !Number.isInteger(concurrency) || concurrency < 1 || concurrency > 5 || !files.every(isBulkDeleteItem) @@ -174,19 +174,19 @@ export async function runBulkDelete(files: BulkDeleteItem[], options: BulkDelete return results } - var nextIndex = 0 - var stopped = false + let nextIndex = 0 + let stopped = false async function worker(): Promise { while (!stopped && nextIndex < files.length) { - var start = nextIndex + const start = nextIndex nextIndex += batchSize - var batch = files.slice(start, start + batchSize) + const batch = files.slice(start, start + batchSize) try { - var response = validateBulkDeleteResponse(await options.request(batch), batch) + const response = validateBulkDeleteResponse(await options.request(batch), batch) if (response.stopped) { stopped = true } - for (var offset = 0; offset < response.results.length; offset++) { + for (let offset = 0; offset < response.results.length; offset++) { if (response.results[offset]!.status === 204) { results[start + offset] = true options.onDeleted(start + offset)