refactor: less reliance on super globals (#4228)

RSS-Bridge · Aug 21, 2024 · 05e2c35 · 05e2c35
1 parent 4a3919c
commit 05e2c35
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 16 deletions.
diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php
@@ -21,12 +21,13 @@ public function __invoke(Request $request): Response
         /** @var Response $cachedResponse */
         $cachedResponse = $this->cache->get($cacheKey);
         if ($cachedResponse) {
-            $ifModifiedSince = $_SERVER['HTTP_IF_MODIFIED_SINCE'] ?? null;
+            $ifModifiedSince = $request->server('HTTP_IF_MODIFIED_SINCE');
             $lastModified = $cachedResponse->getHeader('last-modified');
             if ($ifModifiedSince && $lastModified) {
                 $lastModified = new \DateTimeImmutable($lastModified);
                 $lastModifiedTimestamp = $lastModified->getTimestamp();
                 $modifiedSince = strtotime($ifModifiedSince);
+                // TODO: \DateTimeImmutable can be compared directly
                 if ($lastModifiedTimestamp <= $modifiedSince) {
                     $modificationTimeGMT = gmdate('D, d M Y H:i:s ', $lastModifiedTimestamp);
                     return new Response('', 304, ['last-modified' => $modificationTimeGMT . 'GMT']);
@@ -182,7 +183,7 @@ private function createFeedItemFromException($e, BridgeAbstract $bridge): array
         $content = render_template(__DIR__ . '/../templates/bridge-error.html.php', [
             'error' => render_template(__DIR__ . '/../templates/exception.html.php', ['e' => $e]),
             'searchUrl' => self::createGithubSearchUrl($bridge),
-            'issueUrl' => self::createGithubIssueUrl($bridge, $e, create_sane_exception_message($e)),
+            'issueUrl' => self::createGithubIssueUrl($bridge, $e),
             'maintainer' => $bridge->getMaintainer(),
         ]);
         $item['content'] = $content;
@@ -211,7 +212,7 @@ private function logBridgeError($bridgeName, $code)
         return $report['count'];
     }
 
-    private static function createGithubIssueUrl(BridgeAbstract $bridge, \Throwable $e, string $message): string
+    private static function createGithubIssueUrl(BridgeAbstract $bridge, \Throwable $e): string
     {
         $maintainer = $bridge->getMaintainer();
         if (str_contains($maintainer, ',')) {
@@ -221,13 +222,14 @@ private static function createGithubIssueUrl(BridgeAbstract $bridge, \Throwable
         }
         $maintainers = array_map('trim', $maintainers);
 
+        $queryString = $_SERVER['QUERY_STRING'] ?? '';
         $query = [
             'title' => $bridge->getName() . ' failed with: ' . $e->getMessage(),
             'body' => sprintf(
                 "```\n%s\n\n%s\n\nQuery string: %s\nVersion: %s\nOs: %s\nPHP version: %s\n```\nMaintainer: @%s",
-                $message,
+                create_sane_exception_message($e),
                 implode("\n", trace_to_call_points(trace_from_exception($e))),
-                $_SERVER['QUERY_STRING'] ?? '',
+                $queryString,
                 Configuration::getVersion(),
                 PHP_OS_FAMILY,
                 phpversion() ?: 'Unknown',

diff --git a/formats/HtmlFormat.php b/formats/HtmlFormat.php
@@ -9,6 +9,9 @@ public function stringify()
         // This query string is url encoded
         $queryString = $_SERVER['QUERY_STRING'];
 
+        // TODO: this should be the proper bridge short name and not user provided string
+        $bridgeName = $_GET['bridge'];
+
         $feedArray = $this->getFeed();
         $formatFactory = new FormatFactory();
         $formats = [];
@@ -48,6 +51,7 @@ public function stringify()
         }
 
         $html = render_template(__DIR__ . '/../templates/html-format.html.php', [
+            'bridge_name'   => $bridgeName,
             'charset'       => $this->getCharset(),
             'title'         => $feedArray['name'],
             'formats'       => $formats,

diff --git a/index.php b/index.php
@@ -76,9 +76,17 @@
 
 date_default_timezone_set(Configuration::getConfig('system', 'timezone'));
 
+$argv = $argv ?? null;
+if ($argv) {
+    parse_str(implode('&', array_slice($argv, 1)), $cliArgs);
+    $request = Request::fromCli($cliArgs);
+} else {
+    $request = Request::fromGlobals();
+}
+
 try {
     $rssBridge = new RssBridge($logger, $cache, $httpClient);
-    $response = $rssBridge->main($argv ?? []);
+    $response = $rssBridge->main($request);
     $response->send();
 } catch (\Throwable $e) {
     // Probably an exception inside an action

diff --git a/lib/RssBridge.php b/lib/RssBridge.php
@@ -16,15 +16,8 @@ public function __construct(
         self::$httpClient = $httpClient;
     }
 
-    public function main(array $argv = []): Response
+    public function main(Request $request): Response
     {
-        if ($argv) {
-            parse_str(implode('&', array_slice($argv, 1)), $cliArgs);
-            $request = Request::fromCli($cliArgs);
-        } else {
-            $request = Request::fromGlobals();
-        }
-
         foreach ($request->toArray() as $key => $value) {
             if (!is_string($value)) {
                 return new Response(render(__DIR__ . '/../templates/error.html.php', [

diff --git a/templates/base.html.php b/templates/base.html.php
@@ -4,7 +4,7 @@
     <meta charset="utf-8">
     <meta name="viewport" content="width=device-width, initial-scale=1.0" />
     <meta name="description" content="RSS-Bridge" />
-    <title><?= e($_title ?? 'RSS-Bridge') ?></title>
+    <title>RSS-Bridge</title>
     <link href="static/style.css?2023-03-24" rel="stylesheet">
     <link rel="icon" type="image/png" href="static/favicon.png">
 

diff --git a/templates/html-format.html.php b/templates/html-format.html.php
@@ -30,7 +30,7 @@
         </h1>
 
         <div class="buttons">
-            <a href="./#bridge-<?= $_GET['bridge'] ?>">
+            <a href="./#bridge-<?= e($bridge_name) ?>">
                 <button class="backbutton">← back to rss-bridge</button>
             </a>