From 55450eee5030768454bf812d688543a350688143 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Alfonso=20Pym=20Mej=C3=ADa?= Date: Wed, 28 Oct 2026 08:26:57 -0600 Subject: [PATCH] fix(files_sharing): encode path in public share download redirect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Assisted-by: ClaudeCode:claude-opus-5-5 Signed-off-by: Mario Alfonso Pym Mejía --- .../lib/Controller/ShareController.php | 3 +- .../tests/Controller/ShareControllerTest.php | 42 +++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/apps/files_sharing/lib/Controller/ShareController.php b/apps/files_sharing/lib/Controller/ShareController.php index 38a355bda5d39..11231cf777147 100644 --- a/apps/files_sharing/lib/Controller/ShareController.php +++ b/apps/files_sharing/lib/Controller/ShareController.php @@ -49,6 +49,7 @@ use OCP\Share\IManager as ShareManager; use OCP\Share\IPublicShareTemplateFactory; use OCP\Share\IShare; +use OCP\Util; /** * @package OCA\Files_Sharing\Controllers @@ -391,7 +392,7 @@ public function downloadShare(string $token, ?string $files = null, string $path $davPath = ''; if ($node !== $share->getNode()) { - $davPath = substr($node->getPath(), strlen($share->getNode()->getPath())); + $davPath = Util::encodePath(substr($node->getPath(), strlen($share->getNode()->getPath()))); } $params = []; diff --git a/apps/files_sharing/tests/Controller/ShareControllerTest.php b/apps/files_sharing/tests/Controller/ShareControllerTest.php index f3e247cda2141..91fb4cabdaac1 100644 --- a/apps/files_sharing/tests/Controller/ShareControllerTest.php +++ b/apps/files_sharing/tests/Controller/ShareControllerTest.php @@ -21,6 +21,7 @@ use OCP\AppFramework\Http; use OCP\AppFramework\Http\ContentSecurityPolicy; use OCP\AppFramework\Http\DataResponse; +use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\Template\ExternalShareMenuAction; use OCP\AppFramework\Http\Template\LinkMenuAction; use OCP\AppFramework\Http\Template\PublicTemplateResponse; @@ -752,6 +753,47 @@ public function testDownloadShareWithoutDownloadPermission(): void { $this->assertEquals($expectedResponse, $response); } + public static function dataDownloadShareRedirectEncodesPath(): array { + return [ + 'plain folder' => ['folder', null, 'folder', '/public.php/dav/files/validtoken/folder?accept=zip'], + 'hash in folder' => ['a#b', null, 'a#b', '/public.php/dav/files/validtoken/a%23b?accept=zip'], + 'nested special chars' => ['sub dir/50% ?x', null, 'sub dir/50% ?x', '/public.php/dav/files/validtoken/sub%20dir/50%25%20%3Fx?accept=zip'], + 'legacy file name' => ['', 'notes #1.txt', 'notes #1.txt', '/public.php/dav/files/validtoken/notes%20%231.txt'], + ]; + } + + #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'dataDownloadShareRedirectEncodesPath')] + public function testDownloadShareRedirectEncodesPath(string $path, ?string $files, string $relativePath, string $expectedUrl): void { + $target = $files === null ? $this->createMock(Folder::class) : $this->createMock(File::class); + $target->method('getPath')->willReturn('/owner/files/shared/' . $relativePath); + + $shareNode = $this->createMock(Folder::class); + $shareNode->method('isReadable')->willReturn(true); + $shareNode->method('isShareable')->willReturn(true); + $shareNode->method('getPath')->willReturn('/owner/files/shared'); + $shareNode->method('get') + ->with($files ?? $path) + ->willReturn($target); + + $share = $this->createMock(IShare::class); + $share->method('getPermissions')->willReturn(Constants::PERMISSION_READ); + $share->method('getNode')->willReturn($shareNode); + + $this->shareManager + ->method('getShareByToken') + ->with('validtoken') + ->willReturn($share); + + $this->urlGenerator->expects(self::once()) + ->method('getAbsoluteURL') + ->with($expectedUrl) + ->willReturn('https://cloud.example.com' . $expectedUrl); + + $response = $this->shareController->downloadShare('validtoken', $files, $path); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('https://cloud.example.com' . $expectedUrl, $response->getRedirectURL()); + } + public function testDisabledOwner(): void { $this->shareController->setToken('token');