diff --git a/apps/dav/lib/CardDAV/ImageExportPlugin.php b/apps/dav/lib/CardDAV/ImageExportPlugin.php index 74a8b032e42fd..e4b1047173de7 100644 --- a/apps/dav/lib/CardDAV/ImageExportPlugin.php +++ b/apps/dav/lib/CardDAV/ImageExportPlugin.php @@ -14,6 +14,7 @@ use Sabre\DAV\ServerPlugin; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; +use Symfony\Component\HttpFoundation\HeaderUtils; class ImageExportPlugin extends ServerPlugin { @@ -86,7 +87,11 @@ public function httpGet(RequestInterface $request, ResponseInterface $response) $file = $this->cache->get($addressbook->getResourceId(), $node->getName(), $size, $node); $response->setHeader('Content-Type', $file->getMimeType()); $fileName = $node->getName() . '.' . PhotoCache::ALLOWED_CONTENT_TYPES[$file->getMimeType()]; - $response->setHeader('Content-Disposition', "attachment; filename=$fileName"); + $sanitized = str_replace(['/', '\\'], '-', $fileName); + $fallback = @iconv('UTF-8', 'ASCII//TRANSLIT', $sanitized) ?: $sanitized; + $fallback = preg_replace('/[^\x20-\x7e]/', '', $fallback); + $fallback = str_replace('%', '', $fallback); + $response->setHeader('Content-Disposition', HeaderUtils::makeDisposition(HeaderUtils::DISPOSITION_ATTACHMENT, $sanitized, $fallback)); $response->setStatus(Http::STATUS_OK); $response->setBody($file->getContent()); diff --git a/apps/dav/lib/Provisioning/Apple/AppleProvisioningPlugin.php b/apps/dav/lib/Provisioning/Apple/AppleProvisioningPlugin.php index 258138caa42be..8796db84f5836 100644 --- a/apps/dav/lib/Provisioning/Apple/AppleProvisioningPlugin.php +++ b/apps/dav/lib/Provisioning/Apple/AppleProvisioningPlugin.php @@ -15,6 +15,7 @@ use Sabre\DAV\ServerPlugin; use Sabre\HTTP\RequestInterface; use Sabre\HTTP\ResponseInterface; +use Symfony\Component\HttpFoundation\HeaderUtils; class AppleProvisioningPlugin extends ServerPlugin { /** @@ -127,7 +128,11 @@ public function httpGet(RequestInterface $request, ResponseInterface $response): )); $response->setStatus(Http::STATUS_OK); - $response->setHeader('Content-Disposition', 'attachment; filename="' . $filename . '"'); + $sanitized = str_replace(['/', '\\'], '-', $filename); + $fallback = @iconv('UTF-8', 'ASCII//TRANSLIT', $sanitized) ?: $sanitized; + $fallback = preg_replace('/[^\x20-\x7e]/', '', $fallback); + $fallback = str_replace('%', '', $fallback); + $response->setHeader('Content-Disposition', HeaderUtils::makeDisposition(HeaderUtils::DISPOSITION_ATTACHMENT, $sanitized, $fallback)); $response->setHeader('Content-Type', 'application/xml; charset=utf-8'); $response->setBody($body); diff --git a/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php b/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php index 2a766f1327b51..d996845a61ef7 100644 --- a/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php +++ b/apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php @@ -171,4 +171,64 @@ public function testCard(?int $size, bool $photo): void { $result = $this->plugin->httpGet($this->request, $this->response); $this->assertFalse($result); } + + public function testCardWithSpecialCharactersInName(): void { + $this->request->method('getQueryParameters') + ->willReturn(['photo' => null]); + $this->request->method('getPath') + ->willReturn('user/book/card'); + + $card = $this->createMock(Card::class); + $card->method('getETag') + ->willReturn('"myEtag"'); + $card->method('getName') + ->willReturn('contact "with" special;chars'); + $book = $this->createMock(AddressBook::class); + $book->method('getResourceId') + ->willReturn(1); + + $this->tree->method('getNodeForPath') + ->willReturnCallback(function ($path) use ($card, $book) { + if ($path === 'user/book/card') { + return $card; + } elseif ($path === 'user/book') { + return $book; + } + $this->fail(); + }); + + $file = $this->createMock(ISimpleFile::class); + $file->method('getMimeType') + ->willReturn('image/png'); + $file->method('getContent') + ->willReturn('imgdata'); + + $this->cache->method('get') + ->with(1, 'contact "with" special;chars', -1, $card) + ->willReturn($file); + + // When special characters are present, they should be properly quoted in the filename parameter + $setHeaderCalls = [ + ['Cache-Control', 'private, max-age=3600, must-revalidate'], + ['Etag', '"myEtag"'], + ['Content-Type', 'image/png'], + ['Content-Disposition', 'attachment; filename="contact \"with\" special;chars.png"'], + ]; + $this->response->expects($this->exactly(count($setHeaderCalls))) + ->method('setHeader') + ->willReturnCallback(function () use (&$setHeaderCalls): void { + $expected = array_shift($setHeaderCalls); + $this->assertEquals($expected, func_get_args()); + }); + + $this->response->expects($this->once()) + ->method('setStatus') + ->with(200); + $this->response->expects($this->once()) + ->method('setBody') + ->with('imgdata'); + + $result = $this->plugin->httpGet($this->request, $this->response); + $this->assertFalse($result); + } } diff --git a/apps/dav/tests/unit/Provisioning/Apple/AppleProvisioningPluginTest.php b/apps/dav/tests/unit/Provisioning/Apple/AppleProvisioningPluginTest.php index 58e588aa68d65..9c0b2cefa39bc 100644 --- a/apps/dav/tests/unit/Provisioning/Apple/AppleProvisioningPluginTest.php +++ b/apps/dav/tests/unit/Provisioning/Apple/AppleProvisioningPluginTest.php @@ -148,7 +148,7 @@ public function testHttpGetOnHttps(): void { ->with(200); $calls = [ - ['Content-Disposition', 'attachment; filename="userName-apple-provisioning.mobileconfig"'], + ['Content-Disposition', 'attachment; filename=userName-apple-provisioning.mobileconfig'], ['Content-Type', 'application/xml; charset=utf-8'], ]; $this->sabreResponse->expects($this->exactly(2)) diff --git a/lib/public/AppFramework/Http/DownloadResponse.php b/lib/public/AppFramework/Http/DownloadResponse.php index 190de022d368e..6f3a3660ce11a 100644 --- a/lib/public/AppFramework/Http/DownloadResponse.php +++ b/lib/public/AppFramework/Http/DownloadResponse.php @@ -8,6 +8,7 @@ namespace OCP\AppFramework\Http; use OCP\AppFramework\Http; +use Symfony\Component\HttpFoundation\HeaderUtils; /** * Prompts the user to download the a file @@ -29,9 +30,11 @@ class DownloadResponse extends Response { public function __construct(string $filename, string $contentType, int $status = Http::STATUS_OK, array $headers = []) { parent::__construct($status, $headers); - $filename = strtr($filename, ['"' => '\\"', '\\' => '\\\\']); - - $this->addHeader('Content-Disposition', 'attachment; filename="' . $filename . '"'); + $sanitized = str_replace(['/', '\\'], '-', $filename); + $fallback = @iconv('UTF-8', 'ASCII//TRANSLIT', $sanitized) ?: $sanitized; + $fallback = preg_replace('/[^\x20-\x7e]/', '', $fallback); + $fallback = str_replace('%', '', $fallback); + $this->addHeader('Content-Disposition', HeaderUtils::makeDisposition(HeaderUtils::DISPOSITION_ATTACHMENT, $sanitized, $fallback)); $this->addHeader('Content-Type', $contentType); } } diff --git a/tests/lib/AppFramework/Http/DownloadResponseTest.php b/tests/lib/AppFramework/Http/DownloadResponseTest.php index b2f60edd9999e..08f9b2a0ee2cd 100644 --- a/tests/lib/AppFramework/Http/DownloadResponseTest.php +++ b/tests/lib/AppFramework/Http/DownloadResponseTest.php @@ -23,26 +23,37 @@ public function testHeaders(): void { $response = new ChildDownloadResponse('file', 'content'); $headers = $response->getHeaders(); - $this->assertEquals('attachment; filename="file"', $headers['Content-Disposition']); + $this->assertEquals('attachment; filename=file', $headers['Content-Disposition']); $this->assertEquals('content', $headers['Content-Type']); } #[\PHPUnit\Framework\Attributes\DataProvider('filenameEncodingProvider')] - public function testFilenameEncoding(string $input, string $expected): void { + public function testFilenameEncoding(string $input, string $expectedDisposition): void { $response = new ChildDownloadResponse($input, 'content'); $headers = $response->getHeaders(); - $this->assertEquals('attachment; filename="' . $expected . '"', $headers['Content-Disposition']); + $this->assertEquals($expectedDisposition, $headers['Content-Disposition']); } - public static function filenameEncodingProvider() : array { + public static function filenameEncodingProvider(): array { return [ - ['TestName.txt', 'TestName.txt'], - ['A "Quoted" Filename.txt', 'A \\"Quoted\\" Filename.txt'], - ['A "Quoted" Filename.txt', 'A \\"Quoted\\" Filename.txt'], - ['A "Quoted" Filename With A Backslash \\.txt', 'A \\"Quoted\\" Filename With A Backslash \\\\.txt'], - ['A "Very" Weird Filename \ / & <> " >\'""""\.text', 'A \\"Very\\" Weird Filename \\\\ / & <> \\" >\'\\"\\"\\"\\"\\\\.text'], - ['\\\\\\\\\\\\', '\\\\\\\\\\\\\\\\\\\\\\\\'], + ['TestName.txt', 'attachment; filename=TestName.txt'], + ['A "Quoted" Filename.txt', 'attachment; filename="A \"Quoted\" Filename.txt"'], + ['A "Quoted" Filename.txt', 'attachment; filename="A \"Quoted\" Filename.txt"'], + ['A "Quoted" Filename With A Backslash \\.txt', 'attachment; filename="A \"Quoted\" Filename With A Backslash -.txt"'], + ['A "Very" Weird Filename \ / & <> " >\'""""\.text', 'attachment; filename="A \"Very\" Weird Filename - - & <> \" >\'\"\"\"\"-.text"'], + ['\\\\\\\\\\\\', 'attachment; filename=------'], ]; } + + public function testSpecialCharactersInFilename(): void { + $filename = 'document "draft" with; special&chars.pdf'; + $response = new ChildDownloadResponse($filename, 'application/pdf'); + $headers = $response->getHeaders(); + + $this->assertEquals( + 'attachment; filename="document \"draft\" with; special&chars.pdf"', + $headers['Content-Disposition'] + ); + } }