Merge pull request #22192 from owncloud/fix_19685

Only set the default expiration date on share creation

Author: DeepDiver1975

lib/private/share20/manager.php

@@ -261,7 +261,14 @@ protected function validateExpirationDate(\OCP\Share\IShare $share) {
 		}
 
 		// If expiredate is empty set a default one if there is a default
-		if ($expirationDate === null && $this->shareApiLinkDefaultExpireDate()) {
+		$fullId = null;
+		try {
+			$fullId = $share->getFullId();
+		} catch (\UnexpectedValueException $e) {
+			// This is a new share
+		}
+
+		if ($fullId === null && $expirationDate === null && $this->shareApiLinkDefaultExpireDate()) {
 			$expirationDate = new \DateTime();
 			$expirationDate->setTime(0,0,0);
 			$expirationDate->add(new \DateInterval('P'.$this->shareApiLinkDefaultExpireDays().'D'));
@@ -315,8 +322,12 @@ protected function userCreateChecks(\OCP\Share\IShare $share) {
 		$existingShares = $provider->getSharesByPath($share->getNode());
 		foreach($existingShares as $existingShare) {
 			// Ignore if it is the same share
-			if ($existingShare->getFullId() === $share->getFullId()) {
-				continue;
+			try {
+				if ($existingShare->getFullId() === $share->getFullId()) {
+					continue;
+				}
+			} catch (\UnexpectedValueException $e) {
+				//Shares are not identical
 			}
 
 			// Identical share already existst
@@ -360,8 +371,12 @@ protected function groupCreateChecks(\OCP\Share\IShare $share) {
 		$provider = $this->factory->getProviderForType(\OCP\Share::SHARE_TYPE_GROUP);
 		$existingShares = $provider->getSharesByPath($share->getNode());
 		foreach($existingShares as $existingShare) {
-			if ($existingShare->getFullId() === $share->getFullId()) {
-				continue;
+			try {
+				if ($existingShare->getFullId() === $share->getFullId()) {
+					continue;
+				}
+			} catch (\UnexpectedValueException $e) {
+				//It is a new share so just continue
 			}
 
 			if ($existingShare->getSharedWith() === $share->getSharedWith()) {
@@ -558,7 +573,11 @@ public function updateShare(\OCP\Share\IShare $share) {
 			throw new \Exception('The Share API is disabled');
 		}
 
-		$originalShare = $this->getShareById($share->getFullId());
+		try {
+			$originalShare = $this->getShareById($share->getFullId());
+		} catch (\UnexpectedValueException $e) {
+			throw new \InvalidArgumentException('Share does not have a full id');
+		}
 
 		// We can't change the share type!
 		if ($share->getShareType() !== $originalShare->getShareType()) {
@@ -673,10 +692,15 @@ protected function deleteChildren(\OCP\Share\IShare $share) {
 	 *
 	 * @param \OCP\Share\IShare $share
 	 * @throws ShareNotFound
+	 * @throws \InvalidArgumentException
 	 */
 	public function deleteShare(\OCP\Share\IShare $share) {
 		// Just to make sure we have all the info
-		$share = $this->getShareById($share->getFullId());
+		try {
+			$share = $this->getShareById($share->getFullId());
+		} catch (\UnexpectedValueException $e) {
+			throw new \InvalidArgumentException('Share does not have a full id');
+		}
 
 		$formatHookParams = function(\OCP\Share\IShare $share) {
 			// Prepare hook

lib/private/share20/share.php

@@ -90,6 +90,9 @@ public function getId() {
 	 * @inheritdoc
 	 */
 	public function getFullId() {
+		if ($this->providerId === null || $this->id === null) {
+			throw new \UnexpectedValueException;
+		}
 		return $this->providerId . ':' . $this->id;
 	}
 

lib/public/share/ishare.php

@@ -48,6 +48,7 @@ public function getId();
 	 *
 	 * @return string
 	 * @since 9.0.0
+	 * @throws \UnexpectedValueException If the fullId could not be constructed
 	 */
 	public function getFullId();
 

tests/lib/share20/managertest.php

@@ -890,6 +890,22 @@ public function testValidateExpirationDateHookException() {
 		$this->invokePrivate($this->manager, 'validateExpirationDate', [$share]);
 	}
 
+	public function testValidateExpirationDateExistingShareNoDefault() {
+		$share = $this->manager->newShare();
+
+		$share->setId('42')->setProviderId('foo');
+
+		$this->config->method('getAppValue')
+			->will($this->returnValueMap([
+				['core', 'shareapi_default_expire_date', 'no', 'yes'],
+				['core', 'shareapi_expire_after_n_days', '7', '6'],
+			]));
+
+		$this->invokePrivate($this->manager, 'validateExpirationDate', [$share]);
+
+		$this->assertEquals(null, $share->getExpirationDate());
+	}
+
 	/**
 	 * @expectedException Exception
 	 * @expectedExceptionMessage Only sharing with group members is allowed