From 94b5440d9788f18b1eeed2e12b6585306e9a18f4 Mon Sep 17 00:00:00 2001
From: Dan Hemberger <daniel.hemberger@gmail.com>
Date: Tue, 17 May 2022 17:10:31 -0700
Subject: [PATCH] Fix incorrect comparison for LOADER_URI

This resulted in every page being treated as "not the loader", which
had two subtle consequences:

1. All errors would go to the external error.php page rather than the
   internal redirection within the loader (e.g. current sector).

2. All HREFs would be absolute paths, where they should typically be
   relative (i.e. just the query string when in the loader).

Looking at the documentation for `$_SERVER`, we don't want to use the
`REQUEST_URI`, because this includes the query string. Here are some
of the values on a regular page:

  'REQUEST_URI' => '/loader.php?sn=gwbdpa',
  'SCRIPT_NAME' => '/loader.php',
  'PHP_SELF' => '/loader.php',

As we can see, either `PHP_SELF` or `SCRIPT_NAME` should work for our
purposes.

Another benefit of this is that the NPC script will do the correct
thing now as well. Previously, it couldn't handle errors properly
because CLI scripts don't define `REQUEST_URI`, but we do have:

  'SCRIPT_NAME' => 'src/tools/npc/npc.php',
  'PHP_SELF' => 'src/tools/npc/npc.php',

Again, either of these work, because we do _not_ want it to behave as
if it is in the loader.
---
 src/lib/Default/Page.php                                 | 2 +-
 src/lib/Default/smr.inc.php                              | 2 +-
 test/SmrTest/lib/DefaultGame/PageIntegrationTest.php     | 8 +-------
 test/SmrTest/lib/DefaultGame/VoteSiteIntegrationTest.php | 7 ++-----
 4 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/src/lib/Default/Page.php b/src/lib/Default/Page.php
index a8e3bee42..84cc22fb6 100644
--- a/src/lib/Default/Page.php
+++ b/src/lib/Default/Page.php
@@ -215,7 +215,7 @@ public function href(bool $forceFullURL = false): string {
 		$sn = Smr\Session::getInstance()->addLink($copy);
 
 		$href = '?sn=' . $sn;
-		if ($forceFullURL === true || $_SERVER['REQUEST_URI'] !== LOADER_URI) {
+		if ($forceFullURL === true || $_SERVER['SCRIPT_NAME'] !== LOADER_URI) {
 			return LOADER_URI . $href;
 		}
 		return $href;
diff --git a/src/lib/Default/smr.inc.php b/src/lib/Default/smr.inc.php
index a4c0fcc5b..dddfdfccc 100644
--- a/src/lib/Default/smr.inc.php
+++ b/src/lib/Default/smr.inc.php
@@ -175,7 +175,7 @@ function bbifyMessage(string $message, bool $noLinks = false): string {
 }
 
 function create_error(string $message): never {
-	if ($_SERVER['REQUEST_URI'] !== LOADER_URI) {
+	if ($_SERVER['SCRIPT_NAME'] !== LOADER_URI) {
 		header('Location: /error.php?msg=' . urlencode($message));
 		exit;
 	}
diff --git a/test/SmrTest/lib/DefaultGame/PageIntegrationTest.php b/test/SmrTest/lib/DefaultGame/PageIntegrationTest.php
index 2254f2003..eccdd53de 100644
--- a/test/SmrTest/lib/DefaultGame/PageIntegrationTest.php
+++ b/test/SmrTest/lib/DefaultGame/PageIntegrationTest.php
@@ -21,11 +21,6 @@ protected function setUp(): void {
 		DiContainer::initialize(false);
 	}
 
-	protected function tearDown(): void {
-		// Clear superglobals to avoid impacting other tests
-		$_SERVER = [];
-	}
-
 	/**
 	 * Insert a mock Session into the DI container to return the input $var
 	 * when getCurrentVar is called on it.
@@ -85,9 +80,8 @@ public function test_href(): void {
 		// The Page should not be modified when href() is called
 		$expected = $page->getArrayCopy();
 		srand(0); // for a deterministic SN
-		$_SERVER['REQUEST_URI'] = LOADER_URI; // prevent "Undefined array key"
 		$href = $page->href();
-		self::assertSame('?sn=qpbqzr', $href);
+		self::assertSame(LOADER_URI . '?sn=qpbqzr', $href);
 		self::assertSame($expected, $page->getArrayCopy());
 	}
 
diff --git a/test/SmrTest/lib/DefaultGame/VoteSiteIntegrationTest.php b/test/SmrTest/lib/DefaultGame/VoteSiteIntegrationTest.php
index 84b9d9921..4e1466fd9 100644
--- a/test/SmrTest/lib/DefaultGame/VoteSiteIntegrationTest.php
+++ b/test/SmrTest/lib/DefaultGame/VoteSiteIntegrationTest.php
@@ -15,8 +15,6 @@ class VoteSiteIntegrationTest extends BaseIntegrationSpec {
 
 	protected function tearDown(): void {
 		VoteSite::clearCache();
-		// Clear superglobals to avoid impacting other tests
-		$_SERVER = [];
 	}
 
 	public function test_getTimeUntilFreeTurns_invalid(): void {
@@ -58,17 +56,16 @@ public function test_vote_button_properties(): void {
 		$gameID = 42;
 
 		// Set expected results when free turns are available
-		$_SERVER['REQUEST_URI'] = LOADER_URI; // prevent "Undefined array key"
 		$expected = [
 			VoteSite::LINK_ID_TWG => [
 				'img' => 'twg_vote.png',
 				'url' => 'http://topwebgames.com/in.aspx?ID=136&account=7&game=42&link=3&alwaysreward=1',
-				'sn' => '?sn=gbuyay',
+				'sn' => LOADER_URI . '?sn=gbuyay',
 			],
 			VoteSite::LINK_ID_DOG => [
 				'img' => 'dog_vote.png',
 				'url' => 'http://www.directoryofgames.com/main.php?view=topgames&action=vote&v_tgame=2315&votedef=7,42,4',
-				'sn' => '?sn=wnpclr',
+				'sn' => LOADER_URI . '?sn=wnpclr',
 			],
 			VoteSite::LINK_ID_PBBG => [
 				'img' => 'pbbg.png',