Don't set Content-Disposition header if one already exists

If a Content-Disposition header is already set by another plugin we don't need to set another one as this breaks clients.

Fixes https://github.com/nextcloud/server/issues/1992

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

apps/dav/lib/Connector/Sabre/FilesPlugin.php

@@ -247,8 +247,10 @@ class FilesPlugin extends ServerPlugin {
 		$node = $this->tree->getNodeForPath($request->getPath());
 		if (!($node instanceof IFile)) return;
 
-		// adds a 'Content-Disposition: attachment' header
-		if ($this->downloadAttachment) {
+		// adds a 'Content-Disposition: attachment' header in case no disposition
+		// header has been set before
+		if ($this->downloadAttachment &&
+			$response->getHeader('Content-Disposition') === null) {
 			$filename = $node->getName();
 			if ($this->request->isUserAgent(
 				[

build/integration/data/bjoern.vcf

@@ -0,0 +1,51 @@
+BEGIN:VCARD
+VERSION:3.0
+FN:Björn Schießle
+ORG:Nextcloud
+PHOTO;ENCODING=b;TYPE=jpeg:/9j/4AAQSkZJRgABAQEAbABrAAD//gA7Q1JFQVRPUjogZ2Qt
+ anBlZyB2MS4wICh1c2luZyBJSkcgSlBFRyB2ODApLCBxdWFsaXR5ID0gOTAK/9sAQwAIBgYHBg
+ UIBwcHCQkICgwUDQwLCwwZEhMPFB0aHx4dGhwcICQuJyAiLCMcHCg3KSwwMTQ0NB8nOT04Mjwu
+ MzQy/9sAQwEJCQkMCwwYDQ0YMiEcITIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMjIyMj
+ IyMjIyMjIyMjIyMjIyMjIy/8AAEQgAUABQAwEiAAIRAQMRAf/EAB8AAAEFAQEBAQEBAAAAAAAA
+ AAABAgMEBQYHCAkKC//EALUQAAIBAwMCBAMFBQQEAAABfQECAwAEEQUSITFBBhNRYQcicRQygZ
+ GhCCNCscEVUtHwJDNicoIJChYXGBkaJSYnKCkqNDU2Nzg5OkNERUZHSElKU1RVVldYWVpjZGVm
+ Z2hpanN0dXZ3eHl6g4SFhoeIiYqSk5SVlpeYmZqio6Slpqeoqaqys7S1tre4ubrCw8TFxsfIyc
+ rS09TV1tfY2drh4uPk5ebn6Onq8fLz9PX29/j5+v/EAB8BAAMBAQEBAQEBAQEAAAAAAAABAgME
+ BQYHCAkKC//EALURAAIBAgQEAwQHBQQEAAECdwABAgMRBAUhMQYSQVEHYXETIjKBCBRCkaGxwQ
+ kjM1LwFWJy0QoWJDThJfEXGBkaJicoKSo1Njc4OTpDREVGR0hJSlNUVVZXWFlaY2RlZmdoaWpz
+ dHV2d3h5eoKDhIWGh4iJipKTlJWWl5iZmqKjpKWmp6ipqrKztLW2t7i5usLDxMXGx8jJytLT1N
+ XW19jZ2uLj5OXm5+jp6vLz9PX29/j5+v/aAAwDAQACEQMRAD8A8/zqMcG0GTZKh3R/wouMYxVG
+ SFVtyqoDjocVMuovJZym3kYLtLAlumB1Ofc1FYmR9KaWVtzAYH8q54bmEE7mLHay3d2IYUyzcV
+ 1Vn4VsLaITX8hkkI4j3YFaHg7QJryaOGKNRLcfM0h6ovrXsVh8MNKkt1893Y+vrWt9bHVGGl2e
+ KyW+mBW8i3iVV43beW9ce1YrJapcPKW2xzfLIGbqM9iORX0PcfC3Qw27Yx+hrkvEvwjs5LR5tJ
+ kMdzHllRuje1JtbMv2V1dHjmpwzXMKTRxRsOg29UwOme4rFwVToG9T6V0c9pLFC58tYbmBsMnf
+ IrBlG+X5djscE+WtEVbQwaa3IBnAwcc1u27wuq2lum8qCWlZtoJ9cVSj0+ZnQNBMwI3EIOn1Pa
+ rOoyvCyJFxEy8MowGHTj24pNpuxMlc2g+m6fNPC6b125V22jCk9PSpAy3NmzIyvu4BQcYrK1Zb
+ VdPhFrIZJFXdKWGDtbp296v28BstIRN2WTk4P51mtNTOLtqenfCVIpdNldV+dcKSa9bjkk2gYA
+ ArxHwHqb6F4dvLxVX/AI+NoON38I7D6102geM9R1XX47KRColbCkxFMVfU9SCvFI9MMjkc1VmA
+ IJrlfE2saro8gW3JbJ+XZHvNLoXiWbVFWK4EvmMM4kh8v8qTdy1C2x494/EcOpXoVdpaQ4I9a8
+ /tof8ASTK0yxgYbAOD+Ar0v4laVcSeMXjgT91JGJF+vQiuDisL8k3MaFio2xnPaqv7pxVtGW7i
+ Vp7JpfswRWK5meQrnn+L1rLvHWSaEqMKiBflk3g/T0+lat3cTajpZtQYC4PGRtYgdK52EORsQE
+ sfQVFNdzCJ1Uq239non2iMXDgONoyce/pRbxj7BLHtJYkgDOcc+tVLyw8iczB1NqZNm9Dyvvj0
+ rqLJbb7Kl2nmBA3BK5ZuOv1qG7IzvY9H+HdlGdGmgvraOFcqNobOdqKu78cZrp7CLRbXV1EDIj
+ Rn5nd8nntzXm3hCS8fSLoeYU3P8meDnHzVetorW5ZtPmW4kvifNGyIk46ZBqou562HalSWp6Pq
+ TaezkXRR4nfAbPQ/0qeG0tIYA0JDAdOc4rlkitLDSJBd210Q6hWk8g5bPTjrT/Dy3duZre4ZzE
+ rfu9/3sehps6ElbczfFNgYrPVNflCsba2Kwp1y5yF/8eINeKQMywmGKaIAg7kL7Sp716l8S9Vv
+ oymnW0mIZIxI6kcMdxx+RFeUanpUemQvPcJMwuBw6YI3deopK2zPMxc1KfKZhuy9wpVAQrDCgd
+ arqnkuNodXHO4fw0y3DB96KxwwIxV2PzpZd8xMaE46YBrbYx2NGeB70CVLkOjbVwF2/UfhWzp8
+ E8drAv2pZEjYou3GV+ase2uRaW8un3EyoCWfenU/7OfwrR0W+e4gSKOIEoeitkjPc/rWEk7GT2
+ N3S/EEtrqjwSTKsAYsBj7zEev4GvR7e+kl06LU9NgiutoKlD94fSvHP7NZ5BqN1EY4I5funjzD
+ /dH9a63R5NSl8OC+sLnZMsrJOjfdkOchvyIFbRouMed9TrwlTlbS2PTdHvL3UoRcX0EdtHH91e
+ mKFvIp2LW5DBjwR/OvPbO61zVP9Gnm2wE/MIyfmH17Vs3F0bRfscLD5Y8yFeijstVTpOrUUFuz
+ rqV7RcuiJfFmnwan5j2xL3MNt5UZzwfm3Ej+VeZvI8ZWO6QvFjLAmvQIrtnkCocv3PpVPU9Btd
+ ZhdXlkhlUYDof6V6uKynmjH2W6/E8GVZzk3I4O5tUtwxWKNV6fKB8wxu4H0qq6w+YeVKgDb2PT
+ of8APatnWdGvdI08W8lsJ7QNv89QcD6+lZMhIsx5ezGMFum7HTFeLKnOk7SVmaRfUu+E/C9rqs
+ 0816kjBMc545rvLLS9O0yEpZ2kajplhktVfQrcWOiQ245cqGc+rMcmtnyfujHAr6TB4CnCKlNX
+ b/AwqVHJnK3epfbPPh1e3W1to5/IinBzuOT2/rXTeCrWw/sK7h+2wuPNaUgcFUwMHB+lU9UsIb
+ tkMwysfIUetUrfw9uguYrW6MCgbwjZO4/3R6VeJwvtY2ZpRr+zd0Xr3xB4d061doLx55scKqEf
+ zxWRBqTatp4ZImg85zuJbJK9v60xPCyNLvmG7HQHnPua2rbTkhCqFGarDYKNGXMh1sTKorMW2i
+ SCERxjFWIl2scd6kWLHT86MDn+6K7zmLUL/uyjqHVht2tzn2rmfEHw8stShMmmsLGflti58pj9
+ P4fqPyrbhkLkv0UcCr8MxZCDXNXoRqL3kOM3F6H/2Q==
+UID:6454bec7-6f5b-46f2-ba22-15537ab215d9
+CATEGORIES:Engineering
+END:VCARD

build/integration/features/bootstrap/CardDavContext.php

@@ -202,4 +202,115 @@ class CardDavContext implements \Behat\Behat\Context\Context {
 		}
 	}
 
+	/**
+	 * @Given :user uploads the contact :fileName to the addressbook :addressbook
+	 */
+	public function uploadsTheContactToTheAddressbook($user, $fileName, $addressBook)  {
+		$davUrl = $this->baseUrl . '/remote.php/dav/addressbooks/users/'.$user.'/'.$addressBook . '/' . $fileName;
+		$password = ($user === 'admin') ? 'admin' : '123456';
+
+		$request = $this->client->createRequest(
+			'PUT',
+			$davUrl,
+			[
+				'body' => file_get_contents(__DIR__ . '/../../data/' . $fileName),
+				'auth' => [
+					$user,
+					$password,
+				],
+				'headers' => [
+					'Content-Type' => 'application/xml;charset=UTF-8',
+				],
+			]
+		);
+
+		$this->response = $this->client->send($request);
+
+		if($this->response->getStatusCode() !== 201) {
+			throw new \Exception(
+				sprintf(
+					'Expected %s got %s',
+					201,
+					$this->response->getStatusCode()
+				)
+			);
+		}
+	}
+
+	/**
+	 * @When Exporting the picture of contact :fileName from addressbook :addressBook as user :user
+	 */
+	public function whenExportingThePictureOfContactFromAddressbookAsUser($fileName, $addressBook, $user)  {
+		$davUrl = $this->baseUrl . '/remote.php/dav/addressbooks/users/'.$user.'/'.$addressBook . '/' . $fileName . '?photo=true';
+		$password = ($user === 'admin') ? 'admin' : '123456';
+
+		try {
+			$request = $this->client->createRequest(
+				'GET',
+				$davUrl,
+				[
+					'auth' => [
+						$user,
+						$password,
+					],
+					'headers' => [
+						'Content-Type' => 'application/xml;charset=UTF-8',
+					],
+				]
+			);
+			$this->response = $this->client->send($request);
+		} catch (\GuzzleHttp\Exception\ClientException $e) {
+			$this->response = $e->getResponse();
+		}
+	}
+
+	/**
+	 * @When Downloading the contact :fileName from addressbook :addressBook as user :user
+	 */
+	public function whenDownloadingTheContactFromAddressbookAsUser($fileName, $addressBook, $user)  {
+		$davUrl = $this->baseUrl . '/remote.php/dav/addressbooks/users/'.$user.'/'.$addressBook . '/' . $fileName;
+		$password = ($user === 'admin') ? 'admin' : '123456';
+
+		try {
+			$request = $this->client->createRequest(
+				'GET',
+				$davUrl,
+				[
+					'auth' => [
+						$user,
+						$password,
+					],
+					'headers' => [
+						'Content-Type' => 'application/xml;charset=UTF-8',
+					],
+				]
+			);
+			$this->response = $this->client->send($request);
+		} catch (\GuzzleHttp\Exception\ClientException $e) {
+				$this->response = $e->getResponse();
+		}
+	}
+
+	/**
+	 * @Then The following HTTP headers should be set
+	 * @param \Behat\Gherkin\Node\TableNode $table
+	 * @throws \Exception
+	 */
+	public function theFollowingHttpHeadersShouldBeSet(\Behat\Gherkin\Node\TableNode $table) {
+		foreach($table->getTable() as $header) {
+			$headerName = $header[0];
+			$expectedHeaderValue = $header[1];
+			$returnedHeader = $this->response->getHeader($headerName);
+			if($returnedHeader !== $expectedHeaderValue) {
+				throw new \Exception(
+					sprintf(
+						"Expected value '%s' for header '%s', got '%s'",
+						$expectedHeaderValue,
+						$headerName,
+						$returnedHeader
+					)
+				);
+			}
+		}
+	}
 }

build/integration/features/carddav.feature

@@ -21,3 +21,33 @@ Feature: carddav
   Scenario: Creating a new addressbook
     When "admin" creates an addressbook named "MyAddressbook" with statuscode "201"
     Then "admin" requests addressbook "admin/MyAddressbook" with statuscode "200"
+
+  Scenario: Accessing ones own contact
+    Given "admin" creates an addressbook named "MyAddressbook" with statuscode "201"
+    Given "admin" uploads the contact "bjoern.vcf" to the addressbook "MyAddressbook"
+    When Downloading the contact "bjoern.vcf" from addressbook "MyAddressbook" as user "admin"
+    Then The following HTTP headers should be set
+        |Content-Disposition|attachment; filename*=UTF-8''bjoern.vcf; filename="bjoern.vcf"|
+        |Content-Type|text/vcard; charset=utf-8|
+        |Content-Security-Policy|default-src 'none';|
+        |X-Content-Type-Options |nosniff|
+        |X-Download-Options|noopen|
+        |X-Frame-Options|Sameorigin|
+        |X-Permitted-Cross-Domain-Policies|none|
+        |X-Robots-Tag|none|
+        |X-XSS-Protection|1; mode=block|
+
+  Scenario: Exporting the picture of ones own contact
+    Given "admin" creates an addressbook named "MyAddressbook" with statuscode "201"
+    Given "admin" uploads the contact "bjoern.vcf" to the addressbook "MyAddressbook"
+    When Exporting the picture of contact "bjoern.vcf" from addressbook "MyAddressbook" as user "admin"
+    Then The following HTTP headers should be set
+      |Content-Disposition|attachment|
+      |Content-Type|image/jpeg|
+      |Content-Security-Policy|default-src 'none';|
+      |X-Content-Type-Options |nosniff|
+      |X-Download-Options|noopen|
+      |X-Frame-Options|Sameorigin|
+      |X-Permitted-Cross-Domain-Policies|none|
+      |X-Robots-Tag|none|
+      |X-XSS-Protection|1; mode=block|