Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion apps/dav/lib/CardDAV/ImageExportPlugin.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
/**
Expand Down Expand Up @@ -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);

Expand Down
60 changes: 60 additions & 0 deletions apps/dav/tests/unit/CardDAV/ImageExportPluginTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
9 changes: 6 additions & 3 deletions lib/public/AppFramework/Http/DownloadResponse.php
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
}
}
31 changes: 21 additions & 10 deletions tests/lib/AppFramework/Http/DownloadResponseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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']
);
}
}
Loading