From 865a9f549dc51fdac699841306e7bae1c1f310c1 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Tue, 28 Jul 2026 15:29:29 +0200 Subject: [PATCH] fix(dav): properly finalize with MOVE only Signed-off-by: Ferdinand Thiessen --- apps/dav/lib/Upload/ChunkingV2Plugin.php | 11 +++- .../unit/Upload/ChunkingV2PluginTest.php | 8 +-- .../dav_features/dav-v2-public.feature | 34 ++++++++++ .../integration/features/bootstrap/WebDav.php | 63 +++++++++++++++++++ 4 files changed, 109 insertions(+), 7 deletions(-) diff --git a/apps/dav/lib/Upload/ChunkingV2Plugin.php b/apps/dav/lib/Upload/ChunkingV2Plugin.php index 38ba3b1f8f1b7..83ced7b8933ab 100644 --- a/apps/dav/lib/Upload/ChunkingV2Plugin.php +++ b/apps/dav/lib/Upload/ChunkingV2Plugin.php @@ -70,7 +70,8 @@ public function __construct(ICacheFactory $cacheFactory) { */ #[\Override] public function initialize(Server $server) { - $server->on('beforeMethod:GET', $this->beforeGet(...)); + $server->on('beforeMethod:GET', $this->forbiddenMethod(...)); + $server->on('beforeMethod:COPY', $this->forbiddenMethod(...)); $server->on('beforeMethod:PUT', [$this, 'beforePut']); $server->on('beforeMethod:DELETE', [$this, 'beforeDelete']); $server->on('beforeMove', [$this, 'beforeMove'], 90); @@ -82,12 +83,16 @@ public function initialize(Server $server) { /** * @throws MethodNotAllowed */ - public function beforeGet(RequestInterface $request) { + public function forbiddenMethod(RequestInterface $request) { try { $sourceNode = $this->server->tree->getNodeForPath($request->getPath()); if ($sourceNode instanceof FutureFile || $sourceNode instanceof UploadFile) { - throw new MethodNotAllowed('Reading intermediate uploads is not allowed'); + if ($request->getMethod() === 'GET') { + throw new MethodNotAllowed('Reading intermediate uploads is not allowed'); + } else { + throw new MethodNotAllowed('Intermediate uploads must be finalized using MOVE'); + } } } catch (NotFound) { // The node could not be resolved (yet), e.g. because the targeted diff --git a/apps/dav/tests/unit/Upload/ChunkingV2PluginTest.php b/apps/dav/tests/unit/Upload/ChunkingV2PluginTest.php index e850fac9a0c08..25a032b433182 100644 --- a/apps/dav/tests/unit/Upload/ChunkingV2PluginTest.php +++ b/apps/dav/tests/unit/Upload/ChunkingV2PluginTest.php @@ -75,7 +75,7 @@ public function testBeforeGetIgnoresUnresolvablePath(): void { ->with('versions/admin/versions/74/1782831952') ->willThrowException(new NotFound("File not found: versions in 'root'")); - $this->assertTrue($this->plugin->beforeGet($this->request)); + $this->assertTrue($this->plugin->forbiddenMethod($this->request)); } public function testBeforeGetBlocksFutureFile(): void { @@ -84,7 +84,7 @@ public function testBeforeGetBlocksFutureFile(): void { $this->request->method('getPath')->willReturn('uploads/admin/web-file-upload-id/1'); $this->tree->method('getNodeForPath')->willReturn($this->createMock(FutureFile::class)); - $this->plugin->beforeGet($this->request); + $this->plugin->forbiddenMethod($this->request); } public function testBeforeGetBlocksUploadFile(): void { @@ -93,13 +93,13 @@ public function testBeforeGetBlocksUploadFile(): void { $this->request->method('getPath')->willReturn('uploads/admin/web-file-upload-id/.target'); $this->tree->method('getNodeForPath')->willReturn($this->createMock(UploadFile::class)); - $this->plugin->beforeGet($this->request); + $this->plugin->forbiddenMethod($this->request); } public function testBeforeGetAllowsRegularNode(): void { $this->request->method('getPath')->willReturn('files/admin/foo.txt'); $this->tree->method('getNodeForPath')->willReturn($this->createMock(Directory::class)); - $this->assertTrue($this->plugin->beforeGet($this->request)); + $this->assertTrue($this->plugin->forbiddenMethod($this->request)); } } diff --git a/build/integration/dav_features/dav-v2-public.feature b/build/integration/dav_features/dav-v2-public.feature index a1ff85dc77bbf..9b36ef4ff20f9 100644 --- a/build/integration/dav_features/dav-v2-public.feature +++ b/build/integration/dav_features/dav-v2-public.feature @@ -57,6 +57,40 @@ Feature: dav-v2-public When Downloading public file "/image.png" without ajax header Then the downloaded file has the content of "/testshare/image.png" from "user1" data + Scenario: Finalizing a public chunked upload with COPY is not allowed + Given using new dav path + And user "user0" exists + And As an "user0" + And user "user0" created a folder "/public-upload" + And User "user0" uploads file with content "original content" to "/public-upload/target.txt" + And as "user0" creating a share with + | path | public-upload | + | shareType | 3 | + | publicUpload | true | + And creating a new public chunking upload with id "chunking-public-copy" + And uploading new public chunk file "1" with "AAAAA" to id "chunking-public-copy" + When copying new public chunk file with id "chunking-public-copy" to "/target.txt" + Then the HTTP status code should be "405" + And Downloading file "/public-upload/target.txt" as "user0" + Then Downloaded content should be "original content" + + Scenario: Finalizing a public chunked upload with MOVE overwrites the target + Given using new dav path + And user "user0" exists + And As an "user0" + And user "user0" created a folder "/public-upload" + And User "user0" uploads file with content "original content" to "/public-upload/target.txt" + And as "user0" creating a share with + | path | public-upload | + | shareType | 3 | + | publicUpload | true | + And creating a new public chunking upload with id "chunking-public-move" + And uploading new public chunk file "1" with "AAAAA" to id "chunking-public-move" + When moving new public chunk file with id "chunking-public-move" to "/target.txt" + Then the HTTP status code should be "204" + And Downloading file "/public-upload/target.txt" as "user0" + Then Downloaded content should be "AAAAA" + Scenario: Download a folder Given using new dav path And As an "admin" diff --git a/build/integration/features/bootstrap/WebDav.php b/build/integration/features/bootstrap/WebDav.php index 78a3e85ebbc6f..76f658384edb2 100644 --- a/build/integration/features/bootstrap/WebDav.php +++ b/build/integration/features/bootstrap/WebDav.php @@ -915,6 +915,69 @@ public function userMovesNewChunkFileWithIdToMychunkedfileWithSize($user, $id, $ } } + /** + * @Given creating a new public chunking upload with id :id + */ + public function creatingANewPublicChunkingUploadWithId(string $id): void { + $this->makePublicUploadsDavRequest('MKCOL', '/' . $id); + } + + /** + * @Given uploading new public chunk file :num with :data to id :id + */ + public function uploadingNewPublicChunkFileWithToId(string $num, string $data, string $id): void { + $this->makePublicUploadsDavRequest('PUT', '/' . $id . '/' . $num, [], \GuzzleHttp\Psr7\Utils::streamFor($data)); + } + + /** + * @When moving new public chunk file with id :id to :dest + */ + public function movingNewPublicChunkFileWithIdTo(string $id, string $dest): void { + $this->makePublicUploadsDavRequest('MOVE', '/' . $id . '/.file', [ + 'Destination' => $this->getPublicDavFilesUrl() . $dest, + ]); + } + + /** + * @When copying new public chunk file with id :id to :dest + */ + public function copyingNewPublicChunkFileWithIdTo(string $id, string $dest): void { + $this->makePublicUploadsDavRequest('COPY', '/' . $id . '/.file', [ + 'Destination' => $this->getPublicDavFilesUrl() . $dest, + ]); + } + + private function getLastShareToken(): string { + if (count($this->lastShareData->data->element) > 0) { + return (string)$this->lastShareData->data[0]->token; + } + return (string)$this->lastShareData->data->token; + } + + private function getPublicDavFilesUrl(): string { + return substr($this->baseUrl, 0, -4) . 'public.php/dav/files/' . $this->getLastShareToken(); + } + + /** + * Performs a request on the public chunked upload endpoint of the last created share + */ + private function makePublicUploadsDavRequest(string $method, string $path, array $headers = [], $body = null): void { + $fullUrl = substr($this->baseUrl, 0, -4) . 'public.php/dav/uploads/' . $this->getLastShareToken() . $path; + // Non GET requests on the public DAV endpoint require the AJAX header + $headers['X-Requested-With'] = 'XMLHttpRequest'; + + $client = new GClient(); + try { + $this->response = $client->request($method, $fullUrl, [ + 'headers' => $headers, + 'body' => $body, + ]); + } catch (\GuzzleHttp\Exception\BadResponseException $e) { + // 4xx and 5xx responses cause an exception + $this->response = $e->getResponse(); + } + } + /** * @Given user :user creates a new chunking v2 upload with id :id and destination :targetDestination */