From 1399e87d57e1b91a5d6888021bef310d35f10b4f Mon Sep 17 00:00:00 2001 From: Vincent Petry Date: Thu, 9 Jun 2016 11:29:20 +0200 Subject: [PATCH] DAV now returns file name with Content-Disposition header Fixes issue where Chrome would append ".txt" to XML files when downloaded in the web UI --- apps/dav/lib/Connector/Sabre/FilesPlugin.php | 23 ++++++- .../dav/lib/Connector/Sabre/ServerFactory.php | 1 + apps/dav/lib/Server.php | 1 + .../unit/Connector/Sabre/FilesPluginTest.php | 66 ++++++++++++++++++- .../Connector/Sabre/FilesReportPluginTest.php | 3 +- .../features/webdav-related.feature | 4 +- 6 files changed, 93 insertions(+), 5 deletions(-) diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php index dc47416cca..0a2e6713cb 100644 --- a/apps/dav/lib/Connector/Sabre/FilesPlugin.php +++ b/apps/dav/lib/Connector/Sabre/FilesPlugin.php @@ -42,6 +42,7 @@ use \Sabre\HTTP\RequestInterface; use \Sabre\HTTP\ResponseInterface; use OCP\Files\StorageNotAvailableException; use OCP\IConfig; +use OCP\IRequest; class FilesPlugin extends ServerPlugin { @@ -95,20 +96,29 @@ class FilesPlugin extends ServerPlugin { */ private $config; + /** + * @var IRequest + */ + private $request; + /** * @param Tree $tree * @param View $view + * @param IConfig $config + * @param IRequest $request * @param bool $isPublic * @param bool $downloadAttachment */ public function __construct(Tree $tree, View $view, IConfig $config, + IRequest $request, $isPublic = false, $downloadAttachment = true) { $this->tree = $tree; $this->fileView = $view; $this->config = $config; + $this->request = $request; $this->isPublic = $isPublic; $this->downloadAttachment = $downloadAttachment; } @@ -225,7 +235,18 @@ class FilesPlugin extends ServerPlugin { // adds a 'Content-Disposition: attachment' header if ($this->downloadAttachment) { - $response->addHeader('Content-Disposition', 'attachment'); + $filename = $node->getName(); + if ($this->request->isUserAgent( + [ + \OC\AppFramework\Http\Request::USER_AGENT_IE, + \OC\AppFramework\Http\Request::USER_AGENT_ANDROID_MOBILE_CHROME, + \OC\AppFramework\Http\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) . '"'); + } } if ($node instanceof \OCA\DAV\Connector\Sabre\File) { diff --git a/apps/dav/lib/Connector/Sabre/ServerFactory.php b/apps/dav/lib/Connector/Sabre/ServerFactory.php index b193bfc76c..699dd77166 100644 --- a/apps/dav/lib/Connector/Sabre/ServerFactory.php +++ b/apps/dav/lib/Connector/Sabre/ServerFactory.php @@ -144,6 +144,7 @@ class ServerFactory { $objectTree, $view, $this->config, + $this->request, false, !$this->config->getSystemValue('debug', false) ) diff --git a/apps/dav/lib/Server.php b/apps/dav/lib/Server.php index 179558e97a..e150f441b8 100644 --- a/apps/dav/lib/Server.php +++ b/apps/dav/lib/Server.php @@ -141,6 +141,7 @@ class Server { $this->server->tree, $view, \OC::$server->getConfig(), + $this->request, false, !\OC::$server->getConfig()->getSystemValue('debug', false) ) diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php index 80f284e470..2b3f3e15d1 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesPluginTest.php @@ -73,6 +73,11 @@ class FilesPluginTest extends TestCase { */ private $config; + /** + * @var \OCP\IRequest | \PHPUnit_Framework_MockObject_MockObject + */ + private $request; + public function setUp() { parent::setUp(); $this->server = $this->getMockBuilder('\Sabre\DAV\Server') @@ -88,11 +93,13 @@ class FilesPluginTest extends TestCase { $this->config->expects($this->any())->method('getSystemValue') ->with($this->equalTo('data-fingerprint'), $this->equalTo('')) ->willReturn('my_fingerprint'); + $this->request = $this->getMock('\OCP\IRequest'); $this->plugin = new FilesPlugin( $this->tree, $this->view, - $this->config + $this->config, + $this->request ); $this->plugin->initialize($this->server); } @@ -268,6 +275,7 @@ class FilesPluginTest extends TestCase { $this->tree, $this->view, $this->config, + $this->getMock('\OCP\IRequest'), true); $this->plugin->initialize($this->server); @@ -484,4 +492,60 @@ class FilesPluginTest extends TestCase { $this->plugin->checkMove('FolderA/test.txt', 'test.txt'); } + + public function downloadHeadersProvider() { + return [ + [ + false, + 'attachment; filename*=UTF-8\'\'somefile.xml; filename="somefile.xml"' + ], + [ + true, + 'attachment; filename="somefile.xml"' + ], + ]; + } + + /** + * @dataProvider downloadHeadersProvider + */ + public function testDownloadHeaders($isClumsyAgent, $contentDispositionHeader) { + $request = $this->getMockBuilder('Sabre\HTTP\RequestInterface') + ->disableOriginalConstructor() + ->getMock(); + $response = $this->getMockBuilder('Sabre\HTTP\ResponseInterface') + ->disableOriginalConstructor() + ->getMock(); + + $request + ->expects($this->once()) + ->method('getPath') + ->will($this->returnValue('test/somefile.xml')); + + $node = $this->getMockBuilder('\OCA\DAV\Connector\Sabre\File') + ->disableOriginalConstructor() + ->getMock(); + $node + ->expects($this->once()) + ->method('getName') + ->will($this->returnValue('somefile.xml')); + + $this->tree + ->expects($this->once()) + ->method('getNodeForPath') + ->with('test/somefile.xml') + ->will($this->returnValue($node)); + + $this->request + ->expects($this->once()) + ->method('isUserAgent') + ->will($this->returnValue($isClumsyAgent)); + + $response + ->expects($this->once()) + ->method('addHeader') + ->with('Content-Disposition', $contentDispositionHeader); + + $this->plugin->httpGet($request, $response); + } } diff --git a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php index 41d44efd89..baf4259b21 100644 --- a/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php +++ b/apps/dav/tests/unit/Connector/Sabre/FilesReportPluginTest.php @@ -343,7 +343,8 @@ class FilesReportPluginTest extends \Test\TestCase { new \OCA\DAV\Connector\Sabre\FilesPlugin( $this->tree, $this->view, - $config + $config, + $this->getMock('\OCP\IRequest') ) ); $this->plugin->initialize($this->server); diff --git a/build/integration/features/webdav-related.feature b/build/integration/features/webdav-related.feature index f4d40615fa..14ff505463 100644 --- a/build/integration/features/webdav-related.feature +++ b/build/integration/features/webdav-related.feature @@ -82,7 +82,7 @@ Feature: webdav-related And As an "admin" When Downloading file "/welcome.txt" Then The following headers should be set - |Content-Disposition|attachment| + |Content-Disposition|attachment; filename*=UTF-8''welcome.txt; filename="welcome.txt"| |Content-Security-Policy|default-src 'none';| |X-Content-Type-Options |nosniff| |X-Download-Options|noopen| @@ -97,7 +97,7 @@ Feature: webdav-related And As an "admin" When Downloading file "/welcome.txt" Then The following headers should be set - |Content-Disposition|attachment| + |Content-Disposition|attachment; filename*=UTF-8''welcome.txt; filename="welcome.txt"| |Content-Security-Policy|default-src 'none';| |X-Content-Type-Options |nosniff| |X-Download-Options|noopen|