Skip to content

Commit

Permalink
Fix incorrect comparison for LOADER_URI
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
hemberger committed May 18, 2022
1 parent fc11dcd commit 94b5440
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 14 deletions.
2 changes: 1 addition & 1 deletion src/lib/Default/Page.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion src/lib/Default/smr.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
8 changes: 1 addition & 7 deletions test/SmrTest/lib/DefaultGame/PageIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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());
}

Expand Down
7 changes: 2 additions & 5 deletions test/SmrTest/lib/DefaultGame/VoteSiteIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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',
Expand Down

0 comments on commit 94b5440

Please sign in to comment.