Reopen sessions if we need to write to them instead of keeping them open

Sessions are a locking operation until we write close them, so close
them early and reopen later in case we want to write to them

Signed-off-by: Julius Härtl <jus@bitgrid.net>

lib/base.php

@@ -456,6 +456,7 @@ class OC {
 		}
 
 		$session->set('LAST_ACTIVITY', time());
+		$session->close();
 	}
 
 	/**

lib/private/AppFramework/Middleware/SessionMiddleware.php

@@ -51,8 +51,8 @@ class SessionMiddleware extends Middleware {
 	 */
 	public function beforeController($controller, $methodName) {
 		$useSession = $this->reflector->hasAnnotation('UseSession');
-		if (!$useSession) {
-			$this->session->close();
+		if ($useSession) {
+			$this->session->reopen();
 		}
 	}
 

lib/private/Session/CryptoSessionData.php

@@ -97,8 +97,17 @@ class CryptoSessionData implements \ArrayAccess, ISession {
 	 * @param mixed $value
 	 */
 	public function set(string $key, $value) {
+		if ($this->get($key) === $value) {
+			// Do not write the session if the value hasn't changed to avoid reopening
+			return;
+		}
+
+		$reopened = $this->reopen();
 		$this->sessionValues[$key] = $value;
 		$this->isModified = true;
+		if ($reopened) {
+			$this->close();
+		}
 	}
 
 	/**
@@ -131,9 +140,13 @@ class CryptoSessionData implements \ArrayAccess, ISession {
 	 * @param string $key
 	 */
 	public function remove(string $key) {
+		$reopened = $this->reopen();
 		$this->isModified = true;
 		unset($this->sessionValues[$key]);
 		$this->session->remove(self::encryptedSessionName);
+		if ($reopened) {
+			$this->close();
+		}
 	}
 
 	/**
@@ -149,6 +162,10 @@ class CryptoSessionData implements \ArrayAccess, ISession {
 		$this->session->clear();
 	}
 
+	public function reopen(): bool {
+		return $this->session->reopen();
+	}
+
 	/**
 	 * Wrapper around session_regenerate_id
 	 *

lib/private/Session/Internal.php

@@ -68,8 +68,11 @@ class Internal extends Session {
 	 * @param integer $value
 	 */
 	public function set(string $key, $value) {
-		$this->validateSession();
+		$reopened = $this->reopen();
 		$_SESSION[$key] = $value;
+		if ($reopened) {
+			$this->close();
+		}
 	}
 
 	/**
@@ -101,6 +104,7 @@ class Internal extends Session {
 	}
 
 	public function clear() {
+		$this->reopen();
 		$this->invoke('session_unset');
 		$this->regenerateId();
 		$this->startSession(true);
@@ -120,6 +124,7 @@ class Internal extends Session {
 	 * @return void
 	 */
 	public function regenerateId(bool $deleteOldSession = true, bool $updateToken = false) {
+		$this->reopen();
 		$oldId = null;
 
 		if ($updateToken) {
@@ -171,8 +176,14 @@ class Internal extends Session {
 	/**
 	 * @throws \Exception
 	 */
-	public function reopen() {
-		throw new \Exception('The session cannot be reopened - reopen() is only to be used in unit testing.');
+	public function reopen(): bool {
+		if ($this->sessionClosed) {
+			$this->startSession();
+			$this->sessionClosed = false;
+			return true;
+		}
+
+		return false;
 	}
 
 	/**

lib/private/Session/Memory.php

@@ -53,7 +53,6 @@ class Memory extends Session {
 	 * @param integer $value
 	 */
 	public function set(string $key, $value) {
-		$this->validateSession();
 		$this->data[$key] = $value;
 	}
 
@@ -80,7 +79,6 @@ class Memory extends Session {
 	 * @param string $key
 	 */
 	public function remove(string $key) {
-		$this->validateSession();
 		unset($this->data[$key]);
 	}
 
@@ -110,8 +108,10 @@ class Memory extends Session {
 	/**
 	 * Helper function for PHPUnit execution - don't use in non-test code
 	 */
-	public function reopen() {
+	public function reopen(): bool {
+		$reopened = $this->sessionClosed;
 		$this->sessionClosed = false;
+		return $reopened;
 	}
 
 	/**

lib/public/ISession.php

@@ -83,6 +83,14 @@ interface ISession {
 	 */
 	public function clear();
 
+	/**
+	 * Reopen a session for writing again
+	 *
+	 * @return bool true if the session was actually reopened, otherwise false
+	 * @since 25.0.0
+	 */
+	public function reopen(): bool;
+
 	/**
 	 * Close the session and release the lock
 	 * @since 7.0.0