Merge pull request #1473 from hemberger/session-refactor

Session: modify how pages are handled internally

phpstan.neon.dist

@@ -27,7 +27,6 @@ parameters:
         - RECAPTCHA_PRIVATE
         - SMTP_HOSTNAME
         - TWITTER_CONSUMER_KEY
-        - USING_AJAX
         # We code in protection against a value of 0
         - SmrMines::TOTAL_ENEMY_MINES_MODIFIER
 

src/bootstrap.php

@@ -27,6 +27,8 @@ function logException(Throwable $e): void {
 			$message .= $delim;
 		}
 
+		$message .= 'ajax: ' . var_export($session->ajax, true) . "\n";
+
 		$var = $session->hasCurrentVar() ? $session->getCurrentVar() : null;
 		$message .= '$var: ' . print_r($var, true) . $delim;
 	}
@@ -41,7 +43,6 @@ function logException(Throwable $e): void {
 	$message .=
 		'User IP: ' . getIpAddress() . "\n" .
 		'User Agent: ' . ($_SERVER['HTTP_USER_AGENT'] ?? 'undefined') . "\n" .
-		'USING_AJAX: ' . (defined('USING_AJAX') ? var_export(USING_AJAX, true) : 'undefined') . "\n" .
 		'URL: ' . (defined('URL') ? URL : 'undefined');
 
 	// Try to release lock so they can carry on normally
@@ -122,7 +123,7 @@ function handleException(Throwable $e): void {
 
 	// If this is an ajax update, we don't really have a way to redirect
 	// to an error page at this time.
-	if (!ENABLE_DEBUG && (!defined('USING_AJAX') || !USING_AJAX)) {
+	if (!ENABLE_DEBUG) {
 		header('location: /error.php?msg=' . urlencode($errorType));
 	}
 }

src/config.php

@@ -471,5 +471,3 @@
 
 const AJAX_DEFAULT_REFRESH_TIME = 1500;
 const AJAX_UNPROTECTED_REFRESH_TIME = 800;
-
-define('USING_AJAX', isset($_REQUEST['ajax']) && $_REQUEST['ajax'] == 1);

src/engine/Default/message_view.php

@@ -51,11 +51,6 @@
 	$template->assign('NextPageHREF', $container->href());
 }
 
-// remove entry for this folder from unread msg table
-if ($page == 0 && !USING_AJAX) {
-	$player->setMessagesRead($messageBox['Type']);
-}
-
 $messageBox['Name'] = Messages::getMessageTypeNames($folderID);
 $template->assign('PageTopic', 'Viewing ' . $messageBox['Name']);
 
@@ -94,10 +89,12 @@
 		$messageBox['Messages'][] = displayMessage($dbRecord->getInt('message_id'), $dbRecord->getInt('account_id'), $dbRecord->getInt('sender_id'), $player->getGameID(), $dbRecord->getString('message_text'), $dbRecord->getInt('send_time'), $dbRecord->getBoolean('msg_read'), $folderID, $player->getAccount());
 	}
 }
-if (!USING_AJAX) {
-	$db->write('UPDATE message SET msg_read = \'TRUE\'
-				WHERE message_type_id = ' . $db->escapeNumber($folderID) . ' AND ' . $player->getSQL());
+
+// This should really be part of a (pre)processing page
+if ($page == 0 && !$session->ajax) {
+	$player->setMessagesRead($folderID);
 }
+
 $template->assign('MessageBox', $messageBox);
 
 

src/engine/Default/toggle_processing.php

@@ -5,7 +5,7 @@
 
 $player->setDisplayWeapons(!$player->isDisplayWeapons());
 // If this is called by ajax, we don't want to do any forwarding
-if (USING_AJAX) {
+if ($session->ajax) {
 	exit;
 }
 

src/htdocs/loader.php

@@ -19,8 +19,6 @@
 	// *
 	// ********************************
 
-	//echo '<pre>';echo_r($session);echo'</pre>';
-	//exit;
 	// do we have a session?
 	$session = Smr\Session::getInstance();
 	if (!$session->hasAccount()) {
@@ -30,7 +28,7 @@
 
 	// check if we got a sn number with our url
 	if (empty($session->getSN())) {
-		if (!USING_AJAX) {
+		if (!$session->ajax) {
 			create_error('Your browser lost the SN. Try to reload the page!');
 		} else {
 			exit;
@@ -39,7 +37,7 @@
 
 	// do we have such a container object in the db?
 	if ($session->hasCurrentVar() === false) {
-		if (!USING_AJAX) {
+		if (!$session->ajax) {
 			create_error('Please avoid using the back button!');
 		} else {
 			exit;

src/lib/Default/AbstractSmrPlayer.php

@@ -873,6 +873,8 @@ public static function sendMessageFromRace(int $raceID, int $gameID, int $receiv
 	public function setMessagesRead(int $messageTypeID): void {
 		$this->db->write('DELETE FROM player_has_unread_messages
 							WHERE ' . $this->SQL . ' AND message_type_id = ' . $this->db->escapeNumber($messageTypeID));
+		$this->db->write('UPDATE message SET msg_read = ' . $this->db->escapeBoolean(true) . '
+				WHERE message_type_id = ' . $this->db->escapeNumber($messageTypeID) . ' AND ' . $this->SQL);
 	}
 
 	public function getSafeAttackRating(): int {
@@ -2254,7 +2256,7 @@ public function removeUnderAttack(): bool {
 			return $var['UnderAttack'];
 		}
 		$underAttack = $this->isUnderAttack();
-		if ($underAttack && !USING_AJAX) {
+		if ($underAttack && !$session->ajax) {
 			$var['UnderAttack'] = $underAttack; //Remember we are under attack for AJAX
 		}
 		$this->setUnderAttack(false);

src/lib/Default/Page.php

@@ -11,12 +11,12 @@
  */
 class Page extends ArrayObject {
 
-	private const ALWAYS_AVAILABLE = 999999;
+	private const ALWAYS_AVAILABLE = true;
 
-	// Defines the number of pages that can be loaded after
-	// this page before the links on this page become invalid
-	// (i.e. before you get a back button error).
-	private const URL_DEFAULT_REMAINING_PAGE_LOADS = [
+	// Defines if the page is is always available, or if it is invalid after one
+	// use (i.e. if you get a back button error when navigating back to it).
+	public const ALWAYS_AVAILABLE_PAGES = [
+			'album_edit.php' => self::ALWAYS_AVAILABLE,
 			'alliance_broadcast.php' => self::ALWAYS_AVAILABLE,
 			'alliance_forces.php' => self::ALWAYS_AVAILABLE,
 			'alliance_list.php' => self::ALWAYS_AVAILABLE,
@@ -33,7 +33,7 @@ class Page extends ArrayObject {
 			'course_plot.php' => self::ALWAYS_AVAILABLE,
 			'changelog_view.php' => self::ALWAYS_AVAILABLE,
 			'chat_rules.php' => self::ALWAYS_AVAILABLE,
-			'chess_play.php' => self::ALWAYS_AVAILABLE,
+			'chess.php' => self::ALWAYS_AVAILABLE,
 			'combat_log_list.php' => self::ALWAYS_AVAILABLE,
 			'combat_log_viewer.php' => self::ALWAYS_AVAILABLE,
 			'current_sector.php' => self::ALWAYS_AVAILABLE,
@@ -50,11 +50,13 @@ class Page extends ArrayObject {
 			'feature_request.php' => self::ALWAYS_AVAILABLE,
 			'forces_list.php' => self::ALWAYS_AVAILABLE,
 			'forces_mass_refresh.php' => self::ALWAYS_AVAILABLE,
-			'hall_of_fame_player_new.php' => self::ALWAYS_AVAILABLE,
+			'galactic_post_current.php' => self::ALWAYS_AVAILABLE,
+			'hall_of_fame_new.php' => self::ALWAYS_AVAILABLE,
 			'hall_of_fame_player_detail.php' => self::ALWAYS_AVAILABLE,
 			'leave_newbie.php' => self::ALWAYS_AVAILABLE,
 			'logoff.php' => self::ALWAYS_AVAILABLE,
 			'map_local.php' => self::ALWAYS_AVAILABLE,
+			'message_box.php' => self::ALWAYS_AVAILABLE,
 			'message_view.php' => self::ALWAYS_AVAILABLE,
 			'message_send.php' => self::ALWAYS_AVAILABLE,
 			'news_read_advanced.php' => self::ALWAYS_AVAILABLE,
@@ -82,6 +84,7 @@ class Page extends ArrayObject {
 			'rankings_race.php' => self::ALWAYS_AVAILABLE,
 			'rankings_sector_kill.php' => self::ALWAYS_AVAILABLE,
 			'rankings_view.php' => self::ALWAYS_AVAILABLE,
+			'smr_file_create.php' => self::ALWAYS_AVAILABLE,
 			'trader_bounties.php' => self::ALWAYS_AVAILABLE,
 			'trader_relations.php' => self::ALWAYS_AVAILABLE,
 			'trader_savings.php' => self::ALWAYS_AVAILABLE,
@@ -93,7 +96,7 @@ class Page extends ArrayObject {
 			'alliance_message_add_processing.php' => self::ALWAYS_AVAILABLE,
 			'alliance_message_delete_processing.php' => self::ALWAYS_AVAILABLE,
 			'alliance_pick_processing.php' => self::ALWAYS_AVAILABLE,
-			'chess_move_processing.php' => self::ALWAYS_AVAILABLE,
+			'game_leave_processing.php' => self::ALWAYS_AVAILABLE,
 			'toggle_processing.php' => self::ALWAYS_AVAILABLE,
 			//Admin pages
 			'admin/account_edit.php' => self::ALWAYS_AVAILABLE,
@@ -116,7 +119,7 @@ class Page extends ArrayObject {
 			'admin/unigen/universe_create_warps.php' => self::ALWAYS_AVAILABLE,
 		];
 
-	public int $remainingPageLoads;
+	public readonly bool $reusable;
 
 	/**
 	 * @param array<string, mixed> $data
@@ -127,7 +130,9 @@ protected function __construct(
 		public readonly bool $skipRedirect // to skip redirect hooks at beginning of page processing
 	) {
 		parent::__construct($data);
-		$this->remainingPageLoads = self::URL_DEFAULT_REMAINING_PAGE_LOADS[$file] ?? 1; // Allow refreshing
+
+		// Pages are single-use unless explicitly whitelisted
+		$this->reusable = self::ALWAYS_AVAILABLE_PAGES[$file] ?? false;
 	}
 
 	/**
@@ -188,11 +193,8 @@ public function addVar(string $source, string $dest = null): void {
 	 * the next request, we can grab the container out of the Smr\Session.
 	 */
 	public function href(bool $forceFullURL = false): string {
-
-		// We need to make a clone because the instance saved in the Session
-		// must not be modified if we re-use this instance to create more
-		// links. Ideally this would not be necessary, but the usage of this
-		// method would need to change globally first (no Page re-use).
+		// We need to clone this instance in case it is modified after being added
+		// to the session links. This would not be necessary if Page was readonly.
 		$sn = Smr\Session::getInstance()->addLink(self::copy($this));
 
 		$href = '?sn=' . $sn;
@@ -202,22 +204,6 @@ public function href(bool $forceFullURL = false): string {
 		return $href;
 	}
 
-	/**
-	 * Returns a hash of the contents of the container to identify when two
-	 * containers are equivalent (apart from page-load tracking metadata, which
-	 * we strip out to prevent false differences).
-	 *
-	 * CommonID MUST be unique to a specific action. If there will be different
-	 * outcomes from containers given the same ID then problems will arise.
-	 */
-	public function getCommonID(): string {
-		$commonContainer = $this->getArrayCopy();
-		$commonContainer['_FILE'] = $this->file; // must include file in ID
-		// NOTE: This ID will change if the order of elements in the container
-		// changes. If this causes unnecessary SN changes, sort the container!
-		return md5(serialize($commonContainer));
-	}
-
 	/**
 	 * Process this page by executing the associated file.
 	 */

src/lib/Default/smr.inc.php

@@ -202,7 +202,8 @@ function handleUserError(string $message): never {
 	// Template from the original page.
 	DiContainer::getContainer()->reset(Template::class);
 
-	if (Session::getInstance()->hasGame()) {
+	$session = Session::getInstance();
+	if ($session->hasGame()) {
 		$container = Page::create('current_sector.php');
 		$errorMsg = '<span class="red bold">ERROR: </span>' . $message;
 		$container['errorMsg'] = $errorMsg;
@@ -211,7 +212,7 @@ function handleUserError(string $message): never {
 		$container['message'] = $message;
 	}
 
-	if (USING_AJAX) {
+	if ($session->ajax) {
 		// To avoid the page just not refreshing when an error is encountered
 		// during ajax updates, use javascript to auto-redirect to the
 		// appropriate error page.
@@ -308,7 +309,7 @@ function do_voodoo(): never {
 		define('AJAX_CONTAINER', isset($var['AJAX']) && $var['AJAX'] === true);
 	}
 
-	if (!AJAX_CONTAINER && USING_AJAX && $session->hasChangedSN()) {
+	if (!AJAX_CONTAINER && $session->ajax && $session->hasChangedSN()) {
 		exit;
 	}
 	//ob_clean();
@@ -322,7 +323,7 @@ function do_voodoo(): never {
 		$player = $session->getPlayer();
 		$sectorID = $player->getSectorID();
 
-		if (!USING_AJAX //AJAX should never do anything that requires a lock.
+		if (!$session->ajax //AJAX should never do anything that requires a lock.
 			//&& ($var->file == 'current_sector.php' || $var->file == 'map_local.php') //Neither should CS or LM and they gets loaded a lot so should reduce lag issues with big groups.
 		) {
 			// We skip locking if we've already failed to display error page
@@ -358,7 +359,7 @@ function do_voodoo(): never {
 		$player->updateTurns();
 
 		// Check if we need to redirect to a different page
-		if (!$var->skipRedirect && !USING_AJAX) {
+		if (!$var->skipRedirect && !$session->ajax) {
 			if ($player->getGame()->hasEnded()) {
 				Page::create('game_leave_processing.php', ['forward_to' => 'game_play.php', 'errorMsg' => 'The game has ended.'], skipRedirect: true)->go();
 			}
@@ -408,7 +409,7 @@ function do_voodoo(): never {
 	}
 	$template->assign('AJAX_ENABLE_REFRESH', $ajaxRefresh);
 
-	$template->display('skeleton.php', USING_AJAX || AJAX_CONTAINER);
+	$template->display('skeleton.php', $session->ajax || AJAX_CONTAINER);
 
 	$session->update();