From 428453624df41f845e3c977fb1d414920c315a92 Mon Sep 17 00:00:00 2001 From: Dag Date: Thu, 22 Aug 2024 00:21:50 +0200 Subject: [PATCH] refactor: less reliance on super globals --- actions/DisplayAction.php | 12 +++++++----- formats/HtmlFormat.php | 4 ++++ index.php | 10 +++++++++- lib/RssBridge.php | 9 +-------- templates/base.html.php | 2 +- templates/html-format.html.php | 2 +- 6 files changed, 23 insertions(+), 16 deletions(-) diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php index d111e69e4fc..26f1cb40d26 100644 --- 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 index 37ef3a930db..a5bcc451b6e 100644 --- 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 index 1efda44a0d2..c4fe104fa2b 100644 --- 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 index 23f65cf01f3..35318c5ba57 100644 --- 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 index d2557599500..a8ff766040f 100644 --- a/templates/base.html.php +++ b/templates/base.html.php @@ -4,7 +4,7 @@ - <?= e($_title ?? 'RSS-Bridge') ?> + RSS-Bridge diff --git a/templates/html-format.html.php b/templates/html-format.html.php index bc95c5d04e7..a05acddedd5 100644 --- a/templates/html-format.html.php +++ b/templates/html-format.html.php @@ -30,7 +30,7 @@
- +