diff --git a/apps/dav/lib/CardDAV/ImageExportPlugin.php b/apps/dav/lib/CardDAV/ImageExportPlugin.php index 74a8b032e42fd..92d0534a7a414 100644 --- a/apps/dav/lib/CardDAV/ImageExportPlugin.php +++ b/apps/dav/lib/CardDAV/ImageExportPlugin.php @@ -7,6 +7,7 @@ */ namespace OCA\DAV\CardDAV; +use OC\Http\ContentDisposition; use OCP\AppFramework\Http; use OCP\Files\NotFoundException; use Sabre\CardDAV\Card; @@ -86,7 +87,7 @@ 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"); + $response->setHeader('Content-Disposition', ContentDisposition::make('attachment', $fileName)); $response->setStatus(Http::STATUS_OK); $response->setBody($file->getContent()); diff --git a/apps/dav/lib/CardDAV/MultiGetExportPlugin.php b/apps/dav/lib/CardDAV/MultiGetExportPlugin.php index 9d6b0df838e88..bc7ad342ce2ef 100644 --- a/apps/dav/lib/CardDAV/MultiGetExportPlugin.php +++ b/apps/dav/lib/CardDAV/MultiGetExportPlugin.php @@ -8,6 +8,7 @@ */ namespace OCA\DAV\CardDAV; +use OC\Http\ContentDisposition; use OCP\AppFramework\Http; use Sabre\DAV; use Sabre\DAV\Server; @@ -63,7 +64,7 @@ public function httpReport(RequestInterface $request, ResponseInterface $respons // Build and override the response $filename = 'vcfexport-' . date('Y-m-d') . '.vcf'; - $response->setHeader('Content-Disposition', 'attachment; filename="' . $filename . '"'); + $response->setHeader('Content-Disposition', ContentDisposition::make('attachment', $filename)); $response->setHeader('Content-Type', 'text/vcard'); $response->setStatus(Http::STATUS_OK); diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index f8fb88a2d2584..bc9267a26b848 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -7,8 +7,8 @@ */ namespace OCA\DAV\Connector\Sabre; -use OC\AppFramework\Http\Request; use OC\FilesMetadata\Model\FilesMetadata; +use OC\Http\ContentDisposition; use OC\User\NoUserException; use OCA\DAV\Connector\Sabre\Exception\InvalidPath; use OCA\Files_Sharing\External\Mount as SharingExternalMount; @@ -257,18 +257,7 @@ public function httpGet(RequestInterface $request, ResponseInterface $response) // header has been set before if ($this->downloadAttachment && $response->getHeader('Content-Disposition') === null) { - $filename = $node->getName(); - if ($this->request->isUserAgent( - [ - Request::USER_AGENT_IE, - Request::USER_AGENT_ANDROID_MOBILE_CHROME, - Request::USER_AGENT_FREEBOX, - ])) { - $response->addHeader('Content-Disposition', 'attachment; filename="' . rawurlencode($filename) . '"'); - } else { - $response->addHeader('Content-Disposition', 'attachment; filename*=UTF-8\'\'' . rawurlencode($filename) - . '; filename="' . rawurlencode($filename) . '"'); - } + $response->addHeader('Content-Disposition', ContentDisposition::make('attachment', $node->getName())); } if ($node instanceof File) { diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php index 2e2be008829fa..ac2d476c9ac55 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php @@ -601,21 +601,7 @@ public function testCopyDestinationInvalid(): void { $this->plugin->checkCopy('FolderA/test.txt', 'invalid\\path.txt'); } - public static function downloadHeadersProvider(): array { - return [ - [ - false, - 'attachment; filename*=UTF-8\'\'somefile.xml; filename="somefile.xml"' - ], - [ - true, - 'attachment; filename="somefile.xml"' - ], - ]; - } - - #[\PHPUnit\Framework\Attributes\DataProvider(methodName: 'downloadHeadersProvider')] - public function testDownloadHeaders(bool $isClumsyAgent, string $contentDispositionHeader): void { + public function testDownloadHeaders(): void { $request = $this->createMock(RequestInterface::class); $response = $this->createMock(ResponseInterface::class); @@ -636,13 +622,8 @@ public function testDownloadHeaders(bool $isClumsyAgent, string $contentDisposit ->with('test/somefile.xml') ->willReturn($node); - $this->request - ->expects($this->once()) - ->method('isUserAgent') - ->willReturn($isClumsyAgent); - $calls = [ - ['Content-Disposition', $contentDispositionHeader], + ['Content-Disposition', 'attachment; filename="somefile.xml"'], ['X-Accel-Buffering', 'no'], ]; $response diff --git a/apps/files_trashbin/lib/Sabre/TrashbinPlugin.php b/apps/files_trashbin/lib/Sabre/TrashbinPlugin.php index 9afa5092e0914..3d08d1cfb7da0 100644 --- a/apps/files_trashbin/lib/Sabre/TrashbinPlugin.php +++ b/apps/files_trashbin/lib/Sabre/TrashbinPlugin.php @@ -10,6 +10,7 @@ use OC\Files\FileInfo; use OC\Files\View; +use OC\Http\ContentDisposition; use OCA\DAV\Connector\Sabre\FilesPlugin; use OCA\Files_Trashbin\Trash\ITrashItem; use OCP\IPreview; @@ -127,10 +128,7 @@ public function httpGet(RequestInterface $request, ResponseInterface $response): return; } - $response->addHeader( - 'Content-Disposition', - 'attachment; filename="' . $node->getFilename() . '"' // TODO: Confirm `filename` value is ASCII; add `filename*=UTF-8` support w/ encoding - ); + $response->addHeader('Content-Disposition', ContentDisposition::make('attachment', $node->getFilename())); } /** diff --git a/apps/files_versions/lib/Sabre/Plugin.php b/apps/files_versions/lib/Sabre/Plugin.php index 76b7633076bcb..5263ffd96361b 100644 --- a/apps/files_versions/lib/Sabre/Plugin.php +++ b/apps/files_versions/lib/Sabre/Plugin.php @@ -8,10 +8,9 @@ */ namespace OCA\Files_Versions\Sabre; -use OC\AppFramework\Http\Request; +use OC\Http\ContentDisposition; use OCA\DAV\Connector\Sabre\FilesPlugin; use OCP\IPreview; -use OCP\IRequest; use Sabre\DAV\Exception\NotFound; use Sabre\DAV\INode; use Sabre\DAV\PropFind; @@ -32,15 +31,10 @@ class Plugin extends ServerPlugin { public const AUTHOR = 'author'; public const VERSION_LABEL = '{http://nextcloud.org/ns}version-label'; public const VERSION_AUTHOR = '{http://nextcloud.org/ns}version-author'; - private const LEGACY_FILENAME_HEADER_USER_AGENTS = [ // Quirky clients - Request::USER_AGENT_IE, - Request::USER_AGENT_ANDROID_MOBILE_CHROME, - Request::USER_AGENT_FREEBOX, - ]; + private Server $server; public function __construct( - private readonly IRequest $request, private readonly IPreview $previewManager, ) { } @@ -77,7 +71,7 @@ public function afterGet(RequestInterface $request, ResponseInterface $response) } $filename = $node->getVersion()->getSourceFileName(); - $this->addContentDispositionHeader($response, $filename); + $response->addHeader('Content-Disposition', ContentDisposition::make('attachment', $filename)); } /** @@ -122,39 +116,4 @@ public function propPatch(string $path, PropPatch $propPatch): void { fn (string $label) => $node->setMetadataValue(self::LABEL, $label) ); } - - /** - * Add a Content-Disposition header in a way that attempts to be broadly compatible with various user agents. - * - * Sends both 'filename' (legacy quoted) and 'filename*' (UTF-8 encoded) per RFC 6266, - * except for known quirky agents known to mishandle the `filename*`, which only get `filename`. - * - * Note: The quoting/escaping should strictly follow RFC 6266 and RFC 5987. - * - * TODO: Currently uses rawurlencode($filename) for both parameters, which is wrong: filename= should be plain - * quoted ASCII (with necessary escaping), while filename* should be UTF-8 percent-encoded. - * TODO: This logic appears elsewhere (sometimes with different quoting/filename handling) and could benefit - * from a shared utility function. See Symfony example: - * - https://github.com/symfony/symfony/blob/175775eb21508becf7e7a16d65959488e522c39a/src/Symfony/Component/HttpFoundation/BinaryFileResponse.php#L146-L155 - * - https://github.com/symfony/symfony/blob/175775eb21508becf7e7a16d65959488e522c39a/src/Symfony/Component/HttpFoundation/HeaderUtils.php#L152-L165 - * - * @param ResponseInterface $response HTTP response object to add the header to - * @param string $filename Download filename - */ - private function addContentDispositionHeader(ResponseInterface $response, string $filename): void { - if (!$this->request->isUserAgent(self::LEGACY_FILENAME_HEADER_USER_AGENTS)) { - // Modern clients will use 'filename*'; older clients will refer to `filename`. - // The older fallback must be listed first per RFC. - // In theory this is all we actually need to handle both client types. - $response->addHeader( - 'Content-Disposition', - 'attachment; filename="' . rawurlencode($filename) . '"; filename*=UTF-8\'\'' . rawurlencode($filename) - ); - } else { - // Quirky clients that choke on `filename*`: only send `filename=` - $response->addHeader( - 'Content-Disposition', - 'attachment; filename="' . rawurlencode($filename) . '"'); - } - } } diff --git a/apps/settings/lib/Controller/LogSettingsController.php b/apps/settings/lib/Controller/LogSettingsController.php index 4f4986122d265..dffb1aa3817c2 100644 --- a/apps/settings/lib/Controller/LogSettingsController.php +++ b/apps/settings/lib/Controller/LogSettingsController.php @@ -7,6 +7,7 @@ */ namespace OCA\Settings\Controller; +use OC\Http\ContentDisposition; use OC\Log; use OCP\AppFramework\Controller; use OCP\AppFramework\Http; @@ -43,7 +44,7 @@ public function download() { Http::STATUS_OK, [ 'Content-Type' => 'application/octet-stream', - 'Content-Disposition' => 'attachment; filename="nextcloud.log"', + 'Content-Disposition' => ContentDisposition::make('attachment', 'nextcloud.log'), ], ); } diff --git a/build/integration/dav_features/dav-v2.feature b/build/integration/dav_features/dav-v2.feature index dbd2295497fcf..55d58ea1d8704 100644 --- a/build/integration/dav_features/dav-v2.feature +++ b/build/integration/dav_features/dav-v2.feature @@ -35,7 +35,7 @@ Feature: dav-v2 And As an "admin" When Downloading file "/welcome.txt" Then The following headers should be set - |Content-Disposition|attachment; filename*=UTF-8''welcome.txt; filename="welcome.txt"| + |Content-Disposition|attachment; filename="welcome.txt"| |Content-Security-Policy|default-src 'none';| |X-Content-Type-Options |nosniff| |X-Frame-Options|SAMEORIGIN| diff --git a/core/Controller/TaskProcessingApiController.php b/core/Controller/TaskProcessingApiController.php index 5f2023dd0c242..cc74a4707043a 100644 --- a/core/Controller/TaskProcessingApiController.php +++ b/core/Controller/TaskProcessingApiController.php @@ -12,6 +12,7 @@ use OC\Core\ResponseDefinitions; use OC\Files\SimpleFS\SimpleFile; +use OC\Http\ContentDisposition; use OCP\AppFramework\Http; use OCP\AppFramework\Http\Attribute\ApiRoute; use OCP\AppFramework\Http\Attribute\ExAppRequired; @@ -546,10 +547,7 @@ private function getFileContentsInternal(Task $task, int $fileId): StreamRespons } $response = new StreamResponse($node->fopen('rb')); - $response->addHeader( - 'Content-Disposition', - 'attachment; filename="' . rawurldecode($node->getName()) . '"' - ); + $response->addHeader('Content-Disposition', ContentDisposition::make('attachment', $node->getName())); $response->addHeader('Content-Type', $contentType); return $response; } diff --git a/lib/composer/composer/autoload_classmap.php b/lib/composer/composer/autoload_classmap.php index 35992e16837d2..a424890970853 100644 --- a/lib/composer/composer/autoload_classmap.php +++ b/lib/composer/composer/autoload_classmap.php @@ -1861,6 +1861,7 @@ 'OC\\Http\\Client\\GuzzlePromiseAdapter' => $baseDir . '/lib/private/Http/Client/GuzzlePromiseAdapter.php', 'OC\\Http\\Client\\NegativeDnsCache' => $baseDir . '/lib/private/Http/Client/NegativeDnsCache.php', 'OC\\Http\\Client\\Response' => $baseDir . '/lib/private/Http/Client/Response.php', + 'OC\\Http\\ContentDisposition' => $baseDir . '/lib/private/Http/ContentDisposition.php', 'OC\\Http\\CookieHelper' => $baseDir . '/lib/private/Http/CookieHelper.php', 'OC\\Http\\WellKnown\\RequestManager' => $baseDir . '/lib/private/Http/WellKnown/RequestManager.php', 'OC\\Image' => $baseDir . '/lib/private/Image.php', diff --git a/lib/composer/composer/autoload_static.php b/lib/composer/composer/autoload_static.php index 79c4de8f32767..c5d09aa785d15 100644 --- a/lib/composer/composer/autoload_static.php +++ b/lib/composer/composer/autoload_static.php @@ -1902,6 +1902,7 @@ class ComposerStaticInit749170dad3f5e7f9ca158f5a9f04f6a2 'OC\\Http\\Client\\GuzzlePromiseAdapter' => __DIR__ . '/../../..' . '/lib/private/Http/Client/GuzzlePromiseAdapter.php', 'OC\\Http\\Client\\NegativeDnsCache' => __DIR__ . '/../../..' . '/lib/private/Http/Client/NegativeDnsCache.php', 'OC\\Http\\Client\\Response' => __DIR__ . '/../../..' . '/lib/private/Http/Client/Response.php', + 'OC\\Http\\ContentDisposition' => __DIR__ . '/../../..' . '/lib/private/Http/ContentDisposition.php', 'OC\\Http\\CookieHelper' => __DIR__ . '/../../..' . '/lib/private/Http/CookieHelper.php', 'OC\\Http\\WellKnown\\RequestManager' => __DIR__ . '/../../..' . '/lib/private/Http/WellKnown/RequestManager.php', 'OC\\Image' => __DIR__ . '/../../..' . '/lib/private/Image.php', diff --git a/lib/private/Http/ContentDisposition.php b/lib/private/Http/ContentDisposition.php new file mode 100644 index 0000000000000..d2fc926552493 --- /dev/null +++ b/lib/private/Http/ContentDisposition.php @@ -0,0 +1,67 @@ + '\\"', '\\' => '\\\\']); - - $this->addHeader('Content-Disposition', 'attachment; filename="' . $filename . '"'); + $this->addHeader('Content-Disposition', ContentDisposition::make('attachment', $filename)); $this->addHeader('Content-Type', $contentType); } } diff --git a/lib/public/AppFramework/Http/FileDisplayResponse.php b/lib/public/AppFramework/Http/FileDisplayResponse.php index 87dd895f7adbd..bfc190d652693 100644 --- a/lib/public/AppFramework/Http/FileDisplayResponse.php +++ b/lib/public/AppFramework/Http/FileDisplayResponse.php @@ -6,6 +6,7 @@ */ namespace OCP\AppFramework\Http; +use OC\Http\ContentDisposition; use OCP\AppFramework\Http; use OCP\Files\File; use OCP\Files\SimpleFS\ISimpleFile; @@ -33,7 +34,7 @@ public function __construct(File|ISimpleFile $file, int $statusCode = Http::STAT parent::__construct($statusCode, $headers); $this->file = $file; - $this->addHeader('Content-Disposition', 'inline; filename="' . rawurldecode($file->getName()) . '"'); + $this->addHeader('Content-Disposition', ContentDisposition::make('inline', $file->getName())); $this->setETag($file->getEtag()); $lastModified = new \DateTime(); diff --git a/tests/lib/AppFramework/Http/DownloadResponseTest.php b/tests/lib/AppFramework/Http/DownloadResponseTest.php index b2f60edd9999e..30ac76d67258c 100644 --- a/tests/lib/AppFramework/Http/DownloadResponseTest.php +++ b/tests/lib/AppFramework/Http/DownloadResponseTest.php @@ -32,17 +32,14 @@ public function testFilenameEncoding(string $input, string $expected): void { $response = new ChildDownloadResponse($input, 'content'); $headers = $response->getHeaders(); - $this->assertEquals('attachment; filename="' . $expected . '"', $headers['Content-Disposition']); + $this->assertStringContainsString('attachment', $headers['Content-Disposition']); + $this->assertStringContainsString($expected, $headers['Content-Disposition']); } 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', 'filename="TestName.txt"'], + ['A "Quoted" Filename.txt', 'filename="A \\"Quoted\\" Filename.txt"'], + ['file with spaces.txt', 'filename="file with spaces.txt"'], ]; } } diff --git a/tests/lib/Http/ContentDispositionTest.php b/tests/lib/Http/ContentDispositionTest.php new file mode 100644 index 0000000000000..967961f0ab365 --- /dev/null +++ b/tests/lib/Http/ContentDispositionTest.php @@ -0,0 +1,65 @@ +assertEquals($expected, ContentDisposition::make('attachment', $filename)); + } + + public static function provideAttachmentCases(): array { + return [ + 'simple ASCII' => [ + 'report.pdf', + 'attachment; filename="report.pdf"', + ], + 'ASCII with spaces' => [ + 'my report.pdf', + 'attachment; filename="my report.pdf"', + ], + 'non-ASCII produces fallback and filename*' => [ + 'Ässembly Ünits.pdf', + "attachment; filename=\"_ssembly _nits.pdf\"; filename*=utf-8''%C3%84ssembly%20%C3%9Cnits.pdf", + ], + 'CJK filename' => [ + 'テスト.txt', + "attachment; filename=\"___.txt\"; filename*=utf-8''%E3%83%86%E3%82%B9%E3%83%88.txt", + ], + 'emoji filename' => [ + '🎉party.txt', + "attachment; filename=\"_party.txt\"; filename*=utf-8''%F0%9F%8E%89party.txt", + ], + 'percent in filename' => [ + '100% done.txt', + "attachment; filename=\"100_ done.txt\"; filename*=utf-8''100%25%20done.txt", + ], + 'quotes in filename' => [ + 'say "hello".txt', + "attachment; filename=\"say \\\"hello\\\".txt\"", + ], + ]; + } + + public function testInline(): void { + $result = ContentDisposition::make('inline', 'image.png'); + $this->assertEquals('inline; filename="image.png"', $result); + } + + public function testInlineNonAscii(): void { + $result = ContentDisposition::make('inline', 'Ürlaub.jpg'); + $this->assertStringContainsString('inline', $result); + $this->assertStringContainsString("filename*=utf-8''", $result); + $this->assertStringContainsString('filename="_rlaub.jpg"', $result); + } +}