Merge pull request #22192 from owncloud/fix_19685

Only set the default expiration date on share creation

4 files changed, 51 insertions, 7 deletions

diff --git a/lib/private/share20/manager.php b/lib/private/share20/manager.php
index 3a19bd2dc3a..c13bf965676 100644
--- a/lib/private/share20/manager.php
+++ b/lib/private/share20/manager.php
@@ -261,7 +261,14 @@ class Manager implements IManager {
 		}
 
 		// 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 @@ class Manager implements IManager {
 		$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 @@ class Manager implements IManager {
 		$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 @@ class Manager implements IManager {
 			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 @@ class Manager implements IManager {
 	 *
 	 * @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
diff --git a/lib/private/share20/share.php b/lib/private/share20/share.php
index e84d52b63a0..323d8c8e8ab 100644
--- a/lib/private/share20/share.php
+++ b/lib/private/share20/share.php
@@ -90,6 +90,9 @@ class Share implements \OCP\Share\IShare {
 	 * @inheritdoc
 	 */
 	public function getFullId() {
+		if ($this->providerId === null || $this->id === null) {
+			throw new \UnexpectedValueException;
+		}
 		return $this->providerId . ':' . $this->id;
 	}
 
diff --git a/lib/public/share/ishare.php b/lib/public/share/ishare.php
index 5054a886af5..fdf40f19e56 100644
--- a/lib/public/share/ishare.php
+++ b/lib/public/share/ishare.php
@@ -48,6 +48,7 @@ interface IShare {
 	 *
 	 * @return string
 	 * @since 9.0.0
+	 * @throws \UnexpectedValueException If the fullId could not be constructed
 	 */
 	public function getFullId();
 
diff --git a/tests/lib/share20/managertest.php b/tests/lib/share20/managertest.php
index ea0fb1838d7..73a1b0a6530 100644
--- a/tests/lib/share20/managertest.php
+++ b/tests/lib/share20/managertest.php
@@ -890,6 +890,22 @@ class ManagerTest extends \Test\TestCase {
 		$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