Merge pull request #59843 from nextcloud/fix/dav/unifiy-content-disposition-header-escaping

This commit is contained in:
Benjamin Gaussorgues 2026-04-22 17:37:02 +02:00 committed by GitHub
commit ace8b82187
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
6 changed files with 100 additions and 16 deletions

View file

@ -14,6 +14,7 @@ use Sabre\DAV\Server;
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 @@ class ImageExportPlugin extends ServerPlugin {
$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());

View file

@ -17,6 +17,7 @@ use Sabre\DAV\Server;
use Sabre\DAV\ServerPlugin;
use Sabre\HTTP\RequestInterface;
use Sabre\HTTP\ResponseInterface;
use Symfony\Component\HttpFoundation\HeaderUtils;
class AppleProvisioningPlugin extends ServerPlugin {
/**
@ -123,7 +124,11 @@ class AppleProvisioningPlugin extends ServerPlugin {
));
$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);

View file

@ -171,4 +171,64 @@ class ImageExportPluginTest extends TestCase {
$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);
}
}

View file

@ -148,7 +148,7 @@ class AppleProvisioningPluginTest extends TestCase {
->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))

View file

@ -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);
}
}

View file

@ -23,26 +23,37 @@ class DownloadResponseTest extends \Test\TestCase {
$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']
);
}
}