From 8b5a2494502fa6cb1a983905152c9536b003a377 Mon Sep 17 00:00:00 2001
From: Greg Sarjeant <greg@subcultureofone.org>
Date: Thu, 31 Jul 2025 02:39:09 +0000
Subject: [PATCH] Make URL building more resilient and add tests. (#38)

Since the base URL and base path are user inputs, I'd like tkr to be resilient to any combination of leading and trailing slashes so people don't have to worry about that. This adds some helper functions to normalize URLs and adds tests to confirm that all combinations are handled correctly.

Reviewed-on: https://gitea.subcultureofone.org/greg/tkr/pulls/38
Co-authored-by: Greg Sarjeant <greg@subcultureofone.org>
Co-committed-by: Greg Sarjeant <greg@subcultureofone.org>
---
 public/index.php                              |  2 +-
 .../AdminController/AdminController.php       |  2 +-
 .../AuthController/AuthController.php         |  4 +-
 .../EmojiController/EmojiController.php       |  2 +-
 .../HomeController/HomeController.php         |  2 +-
 .../MoodController/MoodController.php         |  2 +-
 src/Framework/Util/Util.php                   | 24 ++++++++++
 templates/main.php                            |  4 +-
 templates/partials/admin.php                  |  2 +-
 templates/partials/css.php                    |  2 +-
 templates/partials/emoji.php                  |  4 +-
 templates/partials/home.php                   |  2 +-
 templates/partials/login.php                  |  2 +-
 templates/partials/navbar.php                 | 16 +++----
 tests/Framework/Util/UtilTest.php             | 47 +++++++++++++++++++
 15 files changed, 94 insertions(+), 23 deletions(-)

diff --git a/public/index.php b/public/index.php
index f7512c6..733cadb 100644
--- a/public/index.php
+++ b/public/index.php
@@ -77,7 +77,7 @@ if ($method === 'POST' && $path != 'setup') {
         if (!Session::isValid($_POST['csrf_token'])) {
             // Invalid session - redirect to /login
             Log::info('Attempt to POST with invalid session. Redirecting to login.');
-            header('Location: ' . $config->basePath . '/login');
+            header('Location: ' . Util::buildRelativeUrl($config->basePath, 'login'));
             exit;
         }
     } else {
diff --git a/src/Controller/AdminController/AdminController.php b/src/Controller/AdminController/AdminController.php
index a0898a3..d0f17ef 100644
--- a/src/Controller/AdminController/AdminController.php
+++ b/src/Controller/AdminController/AdminController.php
@@ -30,7 +30,7 @@ class AdminController extends Controller {
 
     public function handleSave(){
         if (!Session::isLoggedIn()){
-            header('Location: ' . $config->basePath . '/login');
+            header('Location: ' . Util::buildRelativeUrl($config->basePath, 'login'));
             exit;
         }
 
diff --git a/src/Controller/AuthController/AuthController.php b/src/Controller/AuthController/AuthController.php
index 72fe40e..1ab8189 100644
--- a/src/Controller/AuthController/AuthController.php
+++ b/src/Controller/AuthController/AuthController.php
@@ -30,7 +30,7 @@ class AuthController extends Controller {
                 Log::info("Successful login for {$username}");
 
                 Session::newLoginSession($user);
-                header('Location: ' . $config->basePath);
+                header('Location: ' . Util::buildRelativeUrl($config->basePath));
                 exit;
             } else {
                 Log::warning("Failed login for {$username}");
@@ -48,7 +48,7 @@ class AuthController extends Controller {
         Session::end();
 
         global $config;
-        header('Location: ' . $config->basePath);
+        header('Location: ' . Util::buildRelativeUrl($config->basePath));
         exit;
     }
 }
\ No newline at end of file
diff --git a/src/Controller/EmojiController/EmojiController.php b/src/Controller/EmojiController/EmojiController.php
index ad03ee7..07520c9 100644
--- a/src/Controller/EmojiController/EmojiController.php
+++ b/src/Controller/EmojiController/EmojiController.php
@@ -29,7 +29,7 @@
                 break;
             }
 
-            header('Location: ' . $config->basePath . 'admin/emoji');
+            header('Location: ' . Util::buildRelativeUrl($config->basePath, 'admin/emoji'));
             exit;
         }
 
diff --git a/src/Controller/HomeController/HomeController.php b/src/Controller/HomeController/HomeController.php
index e0f8c1e..f417540 100644
--- a/src/Controller/HomeController/HomeController.php
+++ b/src/Controller/HomeController/HomeController.php
@@ -39,7 +39,7 @@ class HomeController extends Controller {
         global $config;
 
         // redirect to the index (will show the latest tick if one was sent)
-        header('Location: ' . $config->basePath);
+        header('Location: ' . Util::buildRelativeUrl($config->basePath));
         exit;
     }
 
diff --git a/src/Controller/MoodController/MoodController.php b/src/Controller/MoodController/MoodController.php
index c15cfae..f586ae2 100644
--- a/src/Controller/MoodController/MoodController.php
+++ b/src/Controller/MoodController/MoodController.php
@@ -35,7 +35,7 @@
                 $user = $user->save();
 
                 // go back to the index and show the updated mood
-                header('Location: ' . $config->basePath);
+                header('Location: ' . Util::buildRelativeUrl($config->basePath));
                 exit;
             }
         }
diff --git a/src/Framework/Util/Util.php b/src/Framework/Util/Util.php
index 20a912e..9456cb3 100644
--- a/src/Framework/Util/Util.php
+++ b/src/Framework/Util/Util.php
@@ -79,4 +79,28 @@ class Util {
         
         return $baseUrl . $basePath . $path;
     }
+
+    public static function buildRelativeUrl(string $basePath, string $path = ''): string {
+        // Ensure basePath starts with / for relative URLs
+        $basePath = '/' . ltrim($basePath, '/');
+        
+        // Remove trailing slash unless it's just '/'
+        if ($basePath !== '/') {
+            $basePath = rtrim($basePath, '/');
+        }
+        
+        // Add path
+        $path = ltrim($path, '/');
+        
+        if ($path === '') {
+            return $basePath;
+        }
+        
+        // If basePath is root, don't add extra slash
+        if ($basePath === '/') {
+            return '/' . $path;
+        }
+        
+        return $basePath . '/' . $path;
+    }
 }
\ No newline at end of file
diff --git a/templates/main.php b/templates/main.php
index 6c6bf7c..0045384 100644
--- a/templates/main.php
+++ b/templates/main.php
@@ -10,10 +10,10 @@
         <meta charset="UTF-8">
         <meta name="viewport" content="width=device-width, initial-scale=1.0">
         <link rel="stylesheet"
-              href="<?= Util::escape_html($config->basePath) ?>css/default.css">
+              href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'css/default.css')) ?>">
 <?php if (!empty($config->cssId)): ?>
         <link rel="stylesheet"
-              href="<?= Util::escape_html($config->basePath) ?>css/custom/<?= Util::escape_html($config->customCssFilename()) ?>">
+              href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'css/custom/' . $config->customCssFilename())) ?>">
 <?php endif; ?>
         <link rel="alternate"
               type="application/rss+xml"
diff --git a/templates/partials/admin.php b/templates/partials/admin.php
index 2bdf051..e478d00 100644
--- a/templates/partials/admin.php
+++ b/templates/partials/admin.php
@@ -4,7 +4,7 @@
         <h1><?php if ($isSetup): ?>Setup<?php else: ?>Admin<?php endif; ?></h1>
         <main>
             <form
-                action="<?php echo $config->basePath . ($isSetup ? 'setup' : 'admin') ?>"
+                action="<?php echo Util::buildRelativeUrl($config->basePath, ($isSetup ? 'setup' : 'admin')) ?>"
                 method="post">
                 <input type="hidden" name="csrf_token" value="<?= Util::escape_html($_SESSION['csrf_token']) ?>">
                 <fieldset>
diff --git a/templates/partials/css.php b/templates/partials/css.php
index 91826fd..75b1084 100644
--- a/templates/partials/css.php
+++ b/templates/partials/css.php
@@ -2,7 +2,7 @@
 <?php /** @var Array $customCss */ ?>
         <h1>CSS Management</h1>
         <main>
-            <form action="<?= $config->basePath ?>admin/css" method="post" enctype="multipart/form-data">
+            <form action="<?= Util::buildRelativeUrl($config->basePath, 'admin/css') ?>" method="post" enctype="multipart/form-data">
                 <input type="hidden" name="csrf_token" value="<?= Util::escape_html($_SESSION['csrf_token']) ?>">
                 <fieldset>
                     <legend>Manage</legend>
diff --git a/templates/partials/emoji.php b/templates/partials/emoji.php
index 790baa4..7f97f6d 100644
--- a/templates/partials/emoji.php
+++ b/templates/partials/emoji.php
@@ -2,7 +2,7 @@
 <?php /** @var array $emojiList */ ?>
         <h1>Emoji Management</h1>
         <main>
-            <form action="<?= $config->basePath ?>admin/emoji" method="post" enctype="multipart/form-data">
+            <form action="<?= Util::buildRelativeUrl($config->basePath, 'admin/emoji') ?>" method="post" enctype="multipart/form-data">
                 <input type="hidden" name="csrf_token" value="<?= Util::escape_html($_SESSION['csrf_token']) ?>">
                 <fieldset>
                     <legend>Add Emoji</legend>
@@ -24,7 +24,7 @@
                 </fieldset>
             </form>
 <?php if (!empty($emojiList)): ?>
-            <form action="<?= $config->basePath ?>admin/emoji" method="post" enctype="multipart/form-data">
+            <form action="<?= Util::buildRelativeUrl($config->basePath, 'admin/emoji') ?>" method="post" enctype="multipart/form-data">
                 <input type="hidden" name="csrf_token" value="<?= Util::escape_html($_SESSION['csrf_token']) ?>">
                 <fieldset class="delete-emoji-fieldset">
                     <legend>Delete Emoji</legend>
diff --git a/templates/partials/home.php b/templates/partials/home.php
index 120b5c7..ec9b52a 100644
--- a/templates/partials/home.php
+++ b/templates/partials/home.php
@@ -14,7 +14,7 @@
 <?php if (Session::isLoggedIn()): ?>
                         <a
                             <?php if($config->strictAccessibility): ?>tabindex="0"<?php endif; ?>
-                            href="<?= Util::escape_html($config->basePath) ?>mood"
+                            href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'mood')) ?>"
                             class="change-mood">Change mood</a>
 <?php endif ?>
                     </dd>
diff --git a/templates/partials/login.php b/templates/partials/login.php
index fe2994e..32d2f1f 100644
--- a/templates/partials/login.php
+++ b/templates/partials/login.php
@@ -2,7 +2,7 @@
 <?php /** @var string $csrf_token */ ?>
 <?php /** @var string $error */ ?>
     <h2>Login</h2>
-    <form method="post" action="<?= $config->basePath ?>login">
+    <form method="post" action="<?= Util::buildRelativeUrl($config->basePath, 'login') ?>">
         <div class="fieldset-items">
             <input type="hidden" name="csrf_token" value="<?= Util::escape_html($csrf_token) ?>">
             <label for="username">Username:</label>
diff --git a/templates/partials/navbar.php b/templates/partials/navbar.php
index 3ef8cb7..ed2bafb 100644
--- a/templates/partials/navbar.php
+++ b/templates/partials/navbar.php
@@ -2,32 +2,32 @@
 <?php /* https://www.w3schools.com/howto/howto_css_dropdown.asp */ ?>
         <nav aria-label="Main navigation">
             <a <?php if($config->strictAccessibility): ?>tabindex="0"<?php endif; ?>
-               href="<?= Util::escape_html($config->basePath) ?>">home</a>
+               href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath)) ?>">home</a>
             <details>
                 <summary aria-haspopup="true">feeds</summary>
                 <div class="dropdown-items">
                     <a <?php if($config->strictAccessibility): ?>tabindex="0"<?php endif; ?>
-                       href="<?= Util::escape_html($config->basePath) ?>feed/rss">rss</a>
+                       href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'feed/rss')) ?>">rss</a>
                     <a <?php if($config->strictAccessibility): ?>tabindex="0"<?php endif; ?>
-                       href="<?= Util::escape_html($config->basePath) ?>feed/atom">atom</a>
+                       href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'feed/atom')) ?>">atom</a>
                 </div>
             </details>
 <?php if (!Session::isLoggedIn()): ?>
             <a tabindex="0"
-               href="<?= Util::escape_html($config->basePath) ?>login">login</a>
+               href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'login')) ?>">login</a>
 <?php else: ?>
             <details>
                 <summary aria-haspopup="true">admin</summary>
                 <div class="dropdown-items">
                     <a <?php if($config->strictAccessibility): ?>tabindex="0"<?php endif; ?>
-                       href="<?= Util::escape_html($config->basePath) ?>admin">settings</a>
+                       href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'admin')) ?>">settings</a>
                     <a <?php if($config->strictAccessibility): ?>tabindex="0"<?php endif; ?>
-                       href="<?= Util::escape_html($config->basePath) ?>admin/css">css</a>
+                       href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'admin/css')) ?>">css</a>
                     <a <?php if($config->strictAccessibility): ?>tabindex="0"<?php endif; ?>
-                       href="<?= Util::escape_html($config->basePath) ?>admin/emoji">emoji</a>
+                       href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'admin/emoji')) ?>">emoji</a>
                 </div>
             </details>
             <a <?php if($config->strictAccessibility): ?>tabindex="0"<?php endif; ?>
-               href="<?= Util::escape_html($config->basePath) ?>logout">logout</a>
+               href="<?= Util::escape_html(Util::buildRelativeUrl($config->basePath, 'logout')) ?>">logout</a>
 <?php endif; ?>
         </nav>
\ No newline at end of file
diff --git a/tests/Framework/Util/UtilTest.php b/tests/Framework/Util/UtilTest.php
index 0b1e3b7..e4bdfb5 100644
--- a/tests/Framework/Util/UtilTest.php
+++ b/tests/Framework/Util/UtilTest.php
@@ -26,4 +26,51 @@ final class UtilTest extends TestCase
         $this->assertSame($relativeTime, $display);
     }
 
+    public static function buildUrlProvider(): array {
+        return [
+            'basic path' => ['https://example.com', 'tkr', 'admin', 'https://example.com/tkr/admin'],
+            'baseUrl with trailing slash' => ['https://example.com/', 'tkr', 'admin', 'https://example.com/tkr/admin'],
+            'empty basePath' => ['https://example.com', '', 'admin', 'https://example.com/admin'],
+            'root basePath' => ['https://example.com', '/', 'admin', 'https://example.com/admin'],
+            'basePath no leading slash' => ['https://example.com', 'tkr', 'admin', 'https://example.com/tkr/admin'],
+            'basePath with leading slash' => ['https://example.com', '/tkr', 'admin', 'https://example.com/tkr/admin'],
+            'basePath with trailing slash' => ['https://example.com', 'tkr/', 'admin', 'https://example.com/tkr/admin'],
+            'basePath with both slashes' => ['https://example.com', '/tkr/', 'admin', 'https://example.com/tkr/admin'],
+            'complex path' => ['https://example.com', 'tkr', 'admin/css/upload', 'https://example.com/tkr/admin/css/upload'],
+            'path with leading slash' => ['https://example.com', 'tkr', '/admin', 'https://example.com/tkr/admin'],
+            'no path - empty basePath' => ['https://example.com', '', '', 'https://example.com/'],
+            'no path - root basePath' => ['https://example.com', '/', '', 'https://example.com/'],
+            'no path - tkr basePath' => ['https://example.com', 'tkr', '', 'https://example.com/tkr/'],
+        ];
+    }
+
+    #[DataProvider('buildUrlProvider')]
+    public function testBuildUrl(string $baseUrl, string $basePath, string $path, string $expected): void {
+        $result = Util::buildUrl($baseUrl, $basePath, $path);
+        $this->assertEquals($expected, $result);
+    }
+
+    public static function buildRelativeUrlProvider(): array {
+        return [
+            'empty basePath with path' => ['', 'admin', '/admin'],
+            'root basePath with path' => ['/', 'admin', '/admin'],
+            'tkr basePath with path' => ['tkr', 'admin', '/tkr/admin'],
+            'tkr with leading slash' => ['/tkr', 'admin', '/tkr/admin'],
+            'tkr with trailing slash' => ['tkr/', 'admin', '/tkr/admin'],
+            'tkr with both slashes' => ['/tkr/', 'admin', '/tkr/admin'],
+            'complex path' => ['tkr', 'admin/css/upload', '/tkr/admin/css/upload'],
+            'path with leading slash' => ['tkr', '/admin', '/tkr/admin'],
+            'no path - empty basePath' => ['', '', '/'],
+            'no path - root basePath' => ['/', '', '/'],
+            'no path - tkr basePath' => ['tkr', '', '/tkr'],
+            'no path - tkr with slashes' => ['/tkr/', '', '/tkr'],
+        ];
+    }
+
+    #[DataProvider('buildRelativeUrlProvider')]
+    public function testBuildRelativeUrl(string $basePath, string $path, string $expected): void {
+        $result = Util::buildRelativeUrl($basePath, $path);
+        $this->assertEquals($expected, $result);
+    }
+
 }
\ No newline at end of file