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

@@ -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) {

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)
 				)

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)
 					)

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

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

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|