[core] fixes and tests for get_base_href + get_query, see #1024

core/polyfills.php

@@ -306,21 +306,22 @@ function get_base_href(): string
     if (defined("BASE_HREF") && !empty(BASE_HREF)) {
         return BASE_HREF;
     }
-    $possible_vars = ['SCRIPT_NAME', 'PHP_SELF', 'PATH_INFO', 'ORIG_PATH_INFO'];
-    $ok_var = null;
-    foreach ($possible_vars as $var) {
-        if (isset($_SERVER[$var]) && substr($_SERVER[$var], -4) === '.php') {
-            $ok_var = $_SERVER[$var];
-            break;
-        }
+    if(str_ends_with($_SERVER['PHP_SELF'], 'index.php')) {
+        $self = $_SERVER['PHP_SELF'];
     }
-    if(empty($ok_var) && isset($_SERVER['SCRIPT_FILENAME']) && isset($_SERVER['DOCUMENT_ROOT'])) {
-        $ok_var = substr($_SERVER['SCRIPT_FILENAME'], strlen($_SERVER['DOCUMENT_ROOT']));
+    elseif(isset($_SERVER['SCRIPT_FILENAME']) && isset($_SERVER['DOCUMENT_ROOT'])) {
+        $self = substr($_SERVER['SCRIPT_FILENAME'], strlen(rtrim($_SERVER['DOCUMENT_ROOT'], "/")));
     }
-    assert(!empty($ok_var));
-    $dir = dirname($ok_var);
+    /*
+    die(var_export([
+        $_SERVER['PHP_SELF'],
+        $_SERVER['SCRIPT_FILENAME'],
+        $_SERVER['DOCUMENT_ROOT'],
+        $self
+    ], true));
+    */
+    $dir = dirname($self);
     $dir = str_replace("\\", "/", $dir);
-    $dir = str_replace("//", "/", $dir);
     $dir = rtrim($dir, "/");
     return $dir;
 }

core/tests/PolyfillsTest.php

@@ -254,4 +254,42 @@ class PolyfillsTest extends TestCase
         deltree($dir);
         $this->assertFalse(file_exists($dir));
     }
+
+    private function _tbh(array $vars, string $result): void
+    {
+        // update $_SERVER with $vars, call get_base_href() and check result, then reset $_SERVER to original value
+        $old_server = $_SERVER;
+        $_SERVER = array_merge($_SERVER, $vars);
+        $this->assertEquals($result, get_base_href());
+        $_SERVER = $old_server;
+    }
+
+    public function test_get_base_href(): void
+    {
+        // PHP_SELF should point to "the currently executing script
+        // relative to the document root"
+        $this->_tbh(["PHP_SELF" => "/index.php"], "");
+        $this->_tbh(["PHP_SELF" => "/mydir/index.php"], "/mydir");
+
+
+        // SCRIPT_FILENAME should point to "the absolute pathname of
+        // the currently executing script" and DOCUMENT_ROOT should
+        // point to "the document root directory under which the
+        // current script is executing"
+        $this->_tbh([
+            "PHP_SELF" => "<invalid>",
+            "SCRIPT_FILENAME" => "/var/www/html/mydir/index.php",
+            "DOCUMENT_ROOT" => "/var/www/html",
+        ], "/mydir");
+        $this->_tbh([
+            "PHP_SELF" => "<invalid>",
+            "SCRIPT_FILENAME" => "/var/www/html/mydir/index.php",
+            "DOCUMENT_ROOT" => "/var/www/html/",
+        ], "/mydir");
+        $this->_tbh([
+            "PHP_SELF" => "<invalid>",
+            "SCRIPT_FILENAME" => "/var/www/html/index.php",
+            "DOCUMENT_ROOT" => "/var/www/html",
+        ], "");
+    }
 }

core/tests/UtilTest.php

@@ -162,4 +162,23 @@ class UtilTest extends TestCase
             path_to_tags("/category:/tag/baz.jpg")
         );
     }
+
+    public function test_get_query(): void
+    {
+        // no query string
+        $_SERVER["REQUEST_URI"] = "/tasty/cake";
+        $this->assertEquals("/tasty/cake", _get_query());
+
+        // query string
+        $_SERVER["REQUEST_URI"] = "index.php?q=/tasty/cake";
+        $this->assertEquals("/tasty/cake", _get_query());
+
+        // leave url encoding alone
+        $_SERVER["REQUEST_URI"] = "index.php?q=/tasty/cake%20pie";
+        $this->assertEquals("/tasty/cake%20pie", _get_query());
+
+        // if just viewing index.php
+        $_SERVER["REQUEST_URI"] = "index.php";
+        $this->assertEquals("/", _get_query());
+    }
 }

core/util.php

@@ -715,19 +715,33 @@ function _get_user(): User
 
 function _get_query(): string
 {
-    // if query is explicitly set, use it
-    $q = @$_POST["q"] ?: @$_GET["q"];
-    if(!empty($q)) {
-        return $q;
+    // if q is set in POST, use that
+    if(isset($_POST["q"])) {
+        return $_POST["q"];
     }
+
+    // if q is set in GET, use that
+    // (we need to manually parse the query string because PHP's $_GET
+    // does an extra round of URL decoding, which we don't want)
+    $parts = parse_url($_SERVER['REQUEST_URI']);
+    $qs = [];
+    foreach(explode('&', $parts['query'] ?? "") as $z) {
+        $qps = explode('=', $z, 2);
+        if(count($qps) == 2) {
+            $qs[$qps[0]] = $qps[1];
+        }
+    }
+    if(isset($qs["q"])) {
+        return $qs["q"];
+    }
+
     // if we're just looking at index.php, use the default query
-    elseif (str_contains($_SERVER['REQUEST_URI'], "index.php")) {
+    if(str_ends_with($parts["path"], "index.php")) {
         return "/";
     }
+
     // otherwise, use the request URI
-    else {
-        return explode("?", $_SERVER['REQUEST_URI'])[0];
-    }
+    return $parts["path"];
 }
 
 

tests/bootstrap.php

@@ -19,7 +19,10 @@ require_once "core/sys_config.php";
 require_once "core/polyfills.php";
 require_once "core/util.php";
 
+$_SERVER['SCRIPT_FILENAME'] = '/var/www/html/test/index.php';
+$_SERVER['DOCUMENT_ROOT'] = '/var/www/html';
 $_SERVER['QUERY_STRING'] = '/';
+
 if (file_exists("data/test-trace.json")) {
     unlink("data/test-trace.json");
 }

tests/defines.php

@@ -20,7 +20,6 @@ define("VERSION", 'unit-tests');
 define("TRACE_FILE", null);
 define("TRACE_THRESHOLD", 0.0);
 define("TIMEZONE", 'UTC');
-define("BASE_HREF", "/test");
 define("CLI_LOG_LEVEL", 50);
 define("STATSD_HOST", null);
 define("TRUSTED_PROXIES", []);