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

diff --git a/apps/dav/lib/Connector/Sabre/FilesPlugin.php b/apps/dav/lib/Connector/Sabre/FilesPlugin.php
index dc47416cca8be65dd390ab5a4dbbb56b20eb7db7..0a2e6713cb483bbfe0a3897d73f1a84abb469555 100644 (file)
--- 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 b193bfc76c78af56454314f33275e828541e480a..699dd77166ecc33f6b983ecff07e6ae5fc75ac5a 100644 (file)
--- 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 179558e97ae5d604c8653cc63ff9345b05b6afc0..e150f441b8238472e18fbd22efeeba9ec121c6b8 100644 (file)
--- 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 80f284e470e65a2509a53f5d2db49cbd1a87f13f..2b3f3e15d1a7162ffd9f0b5ebaa09ce2bba90fa7 100644 (file)
--- 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 41d44efd89caec5c72b768cd473969d76c50bd00..baf4259b2158d836df60265d18064c34b9a89878 100644 (file)
--- 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 f4d40615fa77217c8c6036844289c92b453515c8..14ff505463cbbcca1c86a4ad964c0ad633d17fd8 100644 (file)
--- 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|