refactor: introduce http Request object (#3926)

actions/ConnectivityAction.php

@@ -19,13 +19,13 @@ class ConnectivityAction implements ActionInterface
         $this->bridgeFactory = new BridgeFactory();
     }
 
-    public function execute(array $request)
+    public function execute(Request $request)
     {
         if (!Debug::isEnabled()) {
             return new Response('This action is only available in debug mode!', 403);
         }
 
-        $bridgeName = $request['bridge'] ?? null;
+        $bridgeName = $request->get('bridge');
         if (!$bridgeName) {
             return render_template('connectivity.html.php');
         }

actions/DetectAction.php

@@ -2,10 +2,10 @@
 
 class DetectAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
-        $targetURL = $request['url'] ?? null;
+        $targetURL = $request->get('url');
-        $format = $request['format'] ?? null;
+        $format = $request->get('format');
 
         if (!$targetURL) {
             throw new \Exception('You must specify a url!');

actions/DisplayAction.php

@@ -11,9 +11,13 @@ class DisplayAction implements ActionInterface
         $this->logger = RssBridge::getLogger();
     }
 
-    public function execute(array $request)
+    public function execute(Request $request)
     {
-        $cacheKey = 'http_' . json_encode($request);
+        $bridgeName = $request->get('bridge');
+        $format = $request->get('format');
+        $noproxy = $request->get('_noproxy');
+
+        $cacheKey = 'http_' . json_encode($request->toArray());
         /** @var Response $cachedResponse */
         $cachedResponse = $this->cache->get($cacheKey);
         if ($cachedResponse) {
@@ -31,7 +35,6 @@ class DisplayAction implements ActionInterface
             return $cachedResponse;
         }
 
-        $bridgeName = $request['bridge'] ?? null;
         if (!$bridgeName) {
             return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Missing bridge parameter']), 400);
         }
@@ -40,7 +43,7 @@ class DisplayAction implements ActionInterface
         if (!$bridgeClassName) {
             return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'Bridge not found']), 404);
         }
-        $format = $request['format'] ?? null;
+
         if (!$format) {
             return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'You must specify a format']), 400);
         }
@@ -48,7 +51,7 @@ class DisplayAction implements ActionInterface
             return new Response(render(__DIR__ . '/../templates/error.html.php', ['message' => 'This bridge is not whitelisted']), 400);
         }
 
-        $noproxy = $request['_noproxy'] ?? null;
+
         if (
             Configuration::getConfig('proxy', 'url')
             && Configuration::getConfig('proxy', 'by_bridge')
@@ -65,7 +68,7 @@ class DisplayAction implements ActionInterface
         $response = $this->createResponse($request, $bridge, $format);
 
         if ($response->getCode() === 200) {
-            $ttl = $request['_cache_timeout'] ?? null;
+            $ttl = $request->get('_cache_timeout');
             if (Configuration::getConfig('cache', 'custom_timeout') && $ttl) {
                 $ttl = (int) $ttl;
             } else {
@@ -90,7 +93,7 @@ class DisplayAction implements ActionInterface
         return $response;
     }
 
-    private function createResponse(array $request, BridgeAbstract $bridge, FormatAbstract $format)
+    private function createResponse(Request $request, BridgeAbstract $bridge, FormatAbstract $format)
     {
         $items = [];
         $feed = [];
@@ -107,7 +110,8 @@ class DisplayAction implements ActionInterface
                 '_error_time',
                 '_', // Some RSS readers add a cache-busting parameter (_=<timestamp>) to feed URLs, detect and ignore them.
             ];
-            $input = array_diff_key($request, array_fill_keys($remove, ''));
+            $requestArray = $request->toArray();
+            $input = array_diff_key($requestArray, array_fill_keys($remove, ''));
             $bridge->setInput($input);
             $bridge->collectData();
             $items = $bridge->getItems();

actions/FindfeedAction.php

@@ -7,10 +7,10 @@
  */
 class FindfeedAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
-        $targetURL = $request['url'] ?? null;
+        $targetURL = $request->get('url');
-        $format = $request['format'] ?? null;
+        $format = $request->get('format');
 
         if (!$targetURL) {
             return new Response('You must specify a url', 400);

actions/FrontpageAction.php

@@ -2,10 +2,11 @@
 
 final class FrontpageAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
+        $showInactive = (bool) $request->get('show_inactive');
+
         $messages = [];
-        $showInactive = (bool) ($request['show_inactive'] ?? null);
         $activeBridges = 0;
 
         $bridgeFactory = new BridgeFactory();
@@ -18,16 +19,13 @@ final class FrontpageAction implements ActionInterface
             ];
         }
 
-        $formatFactory = new FormatFactory();
-        $formats = $formatFactory->getFormatNames();
-
         $body = '';
         foreach ($bridgeClassNames as $bridgeClassName) {
             if ($bridgeFactory->isEnabled($bridgeClassName)) {
-                $body .= BridgeCard::displayBridgeCard($bridgeClassName);
+                $body .= BridgeCard::render($bridgeClassName);
                 $activeBridges++;
             } elseif ($showInactive) {
-                $body .= BridgeCard::displayBridgeCard($bridgeClassName, false) . "\n";
+                $body .= BridgeCard::render($bridgeClassName, false) . "\n";
             }
         }
 

actions/HealthAction.php

@@ -4,7 +4,7 @@ declare(strict_types=1);
 
 class HealthAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
         $response = [
             'code' => 200,

actions/ListAction.php

@@ -2,7 +2,7 @@
 
 class ListAction implements ActionInterface
 {
-    public function execute(array $request)
+    public function execute(Request $request)
     {
         $list = new \stdClass();
         $list->bridges = [];

actions/SetBridgeCacheAction.php

@@ -9,15 +9,17 @@ class SetBridgeCacheAction implements ActionInterface
         $this->cache = RssBridge::getCache();
     }
 
-    public function execute(array $request)
+    public function execute(Request $request)
     {
+        $requestArray = $request->toArray();
+
         // Authentication
         $accessTokenInConfig = Configuration::getConfig('authentication', 'access_token');
         if (!$accessTokenInConfig) {
             return new Response('Access token is not set in this instance', 403, ['content-type' => 'text/plain']);
         }
-        if (isset($request['access_token'])) {
+        if (isset($requestArray['access_token'])) {
-            $accessTokenGiven = $request['access_token'];
+            $accessTokenGiven = $requestArray['access_token'];
         } else {
             $header = trim($_SERVER['HTTP_AUTHORIZATION'] ?? '');
             $position = strrpos($header, 'Bearer ');
@@ -35,33 +37,32 @@ class SetBridgeCacheAction implements ActionInterface
         }
 
         // Begin actual work
-        $key = $request['key'] ?? null;
+        $key = $requestArray['key'] ?? null;
         if (!$key) {
-            returnClientError('You must specify key!');
+            return new Response('You must specify key', 400, ['content-type' => 'text/plain']);
         }
 
         $bridgeFactory = new BridgeFactory();
 
-        $bridgeName = $request['bridge'] ?? null;
+        $bridgeName = $requestArray['bridge'] ?? null;
         $bridgeClassName = $bridgeFactory->createBridgeClassName($bridgeName);
         if (!$bridgeClassName) {
-            throw new \Exception(sprintf('Bridge not found: %s', $bridgeName));
+            return new Response(sprintf('Bridge not found: %s', $bridgeName), 400, ['content-type' => 'text/plain']);
         }
 
         // whitelist control
         if (!$bridgeFactory->isEnabled($bridgeClassName)) {
-            throw new \Exception('This bridge is not whitelisted', 401);
+            return new Response('This bridge is not whitelisted', 401, ['content-type' => 'text/plain']);
         }
 
         $bridge = $bridgeFactory->create($bridgeClassName);
         $bridge->loadConfiguration();
-        $value = $request['value'];
+        $value = $requestArray['value'];
 
         $cacheKey = get_class($bridge) . '_' . $key;
         $ttl = 86400 * 3;
         $this->cache->set($cacheKey, $value, $ttl);
 
-        header('Content-Type: text/plain');
+        return new Response('done', 200, ['Content-Type' => 'text/plain']);
-        echo 'done';
     }
 }

bridges/StreamCzBridge.php

@@ -63,7 +63,7 @@ class StreamCzBridge extends BridgeAbstract
             $imageUrlNode = reset($episode['node']['images']);
             $item = [
                 'title' => $episode['node']['name'],
-                'uri' => "${fixedUrl}/${episodeUrl}",
+                'uri' => $fixedUrl . '/' . $episodeUrl,
                 'content' => $imageUrlNode ? '<img src="' . $imageUrlNode['url'] . '" />' : '',
                 'timestamp' => $episode['node']['publishTime']['timestamp']
             ];

formats/HtmlFormat.php

@@ -12,20 +12,21 @@ class HtmlFormat extends FormatAbstract
         $formatFactory = new FormatFactory();
         $buttons = [];
         $linkTags = [];
-        foreach ($formatFactory->getFormatNames() as $format) {
+        foreach ($formatFactory->getFormatNames() as $formatName) {
             // Dynamically build buttons for all formats (except HTML)
-            if ($format === 'Html') {
+            if ($formatName === 'Html') {
                 continue;
             }
-            $formatUrl = '?' . str_ireplace('format=Html', 'format=' . $format, htmlentities($queryString));
+            $formatUrl = '?' . str_ireplace('format=Html', 'format=' . $formatName, htmlentities($queryString));
             $buttons[] = [
                 'href' => $formatUrl,
-                'value' => $format,
+                'value' => $formatName,
             ];
+            $format = $formatFactory->create($formatName);
             $linkTags[] = [
                 'href' => $formatUrl,
-                'title' => $format,
+                'title' => $formatName,
-                'type' => $formatFactory->create($format)->getMimeType(),
+                'type' => $format->getMimeType(),
             ];
         }
 

lib/ActionInterface.php

@@ -5,5 +5,5 @@ interface ActionInterface
     /**
      * @return string|Response
      */
-    public function execute(array $request);
+    public function execute(Request $request);
 }

lib/BridgeCard.php

@@ -3,13 +3,13 @@
 final class BridgeCard
 {
     /**
-     * Gets a single bridge card
+     * Render bridge card
      *
      * @param class-string<BridgeAbstract> $bridgeClassName The bridge name
      * @param bool $isActive Indicates if the bridge is active or not
      * @return string The bridge card
      */
-    public static function displayBridgeCard($bridgeClassName, $isActive = true)
+    public static function render($bridgeClassName, $isActive = true)
     {
         $bridgeFactory = new BridgeFactory();
 
@@ -56,10 +56,10 @@ final class BridgeCard
         // If we don't have any parameter for the bridge, we print a generic form to load it.
         if (count($contexts) === 0) {
             // The bridge has zero parameters
-            $card .= self::getForm($bridgeClassName, $isActive);
+            $card .= self::renderForm($bridgeClassName, $isActive);
         } elseif (count($contexts) === 1 && array_key_exists('global', $contexts)) {
             // The bridge has a single context with key 'global'
-            $card .= self::getForm($bridgeClassName, $isActive, '', $contexts['global']);
+            $card .= self::renderForm($bridgeClassName, $isActive, '', $contexts['global']);
         } else {
             // The bridge has one or more contexts (named or unnamed)
             foreach ($contexts as $contextName => $contextParameters) {
@@ -77,7 +77,7 @@ final class BridgeCard
                     $card .= '<h5>' . $contextName . '</h5>' . PHP_EOL;
                 }
 
-                $card .= self::getForm($bridgeClassName, $isActive, $contextName, $contextParameters);
+                $card .= self::renderForm($bridgeClassName, $isActive, $contextName, $contextParameters);
             }
         }
 
@@ -97,7 +97,7 @@ final class BridgeCard
         return $card;
     }
 
-    private static function getForm(
+    private static function renderForm(
         string $bridgeClassName,
         bool $isActive = false,
         string $contextName = '',

lib/FeedExpander.php

@@ -26,6 +26,7 @@ abstract class FeedExpander extends BridgeAbstract
         $badStrings = [
             ' ',
             '»',
+            '’',
         ];
         $xmlString = str_replace($badStrings, '', $xmlString);
         $feedParser = new FeedParser();

lib/FormatFactory.php

@@ -2,32 +2,26 @@
 
 class FormatFactory
 {
-    private $folder;
+    private array $formatNames = [];
-    private $formatNames;
 
-    public function __construct(string $folder = PATH_LIB_FORMATS)
+    public function __construct()
     {
-        $this->folder = $folder;
+        $iterator = new \FilesystemIterator(__DIR__ . '/../formats');
-
+        foreach ($iterator as $file) {
-        // create format names
+            if (preg_match('/^([^.]+)Format\.php$/U', $file->getFilename(), $m)) {
-        foreach (scandir($this->folder) as $file) {
-            if (preg_match('/^([^.]+)Format\.php$/U', $file, $m)) {
                 $this->formatNames[] = $m[1];
             }
         }
+        sort($this->formatNames);
     }
 
-    /**
-     * @throws \InvalidArgumentException
-     * @param string $name The name of the format e.g. "Atom", "Mrss" or "Json"
-     */
     public function create(string $name): FormatAbstract
     {
         if (! preg_match('/^[a-zA-Z0-9-]*$/', $name)) {
             throw new \InvalidArgumentException('Format name invalid!');
         }
-        $sanitizedName = $this->sanitizeFormatName($name);
+        $sanitizedName = $this->sanitizeName($name);
-        if ($sanitizedName === null) {
+        if (!$sanitizedName) {
             throw new \InvalidArgumentException(sprintf('Unknown format given `%s`', $name));
         }
         $className = '\\' . $sanitizedName . 'Format';
@@ -39,15 +33,13 @@ class FormatFactory
         return $this->formatNames;
     }
 
-    protected function sanitizeFormatName(string $name)
+    protected function sanitizeName(string $name): ?string
     {
         $name = ucfirst(strtolower($name));
-
         // Trim trailing '.php' if exists
         if (preg_match('/(.+)(?:\.php)/', $name, $matches)) {
             $name = $matches[1];
         }
-
         // Trim trailing 'Format' if exists
         if (preg_match('/(.+)(?:Format)/i', $name, $matches)) {
             $name = $matches[1];

lib/RssBridge.php

@@ -27,9 +27,17 @@ final class RssBridge
     {
         if ($argv) {
             parse_str(implode('&', array_slice($argv, 1)), $cliArgs);
-            $request = $cliArgs;
+            $request = Request::fromCli($cliArgs);
         } else {
-            $request = array_merge($_GET, $_POST);
+            $request = Request::fromGlobals();
+        }
+
+        foreach ($request->toArray() as $key => $value) {
+            if (!is_string($value)) {
+                return new Response(render(__DIR__ . '/../templates/error.html.php', [
+                    'message' => "Query parameter \"$key\" is not a string.",
+                ]), 400);
+            }
         }
 
         if (Configuration::getConfig('system', 'enable_maintenance_mode')) {
@@ -43,8 +51,8 @@ final class RssBridge
             if (Configuration::getConfig('authentication', 'password') === '') {
                 return new Response('The authentication password cannot be the empty string', 500);
             }
-            $user = $_SERVER['PHP_AUTH_USER'] ?? null;
+            $user = $request->server('PHP_AUTH_USER');
-            $password = $_SERVER['PHP_AUTH_PW'] ?? null;
+            $password = $request->server('PHP_AUTH_PW');
             if ($user === null || $password === null) {
                 $html = render(__DIR__ . '/../templates/error.html.php', [
                     'message' => 'Please authenticate in order to access this instance!',
@@ -63,16 +71,8 @@ final class RssBridge
             // At this point the username and password was correct
         }
 
-        foreach ($request as $key => $value) {
+        $action = $request->get('action', 'Frontpage');
-            if (!is_string($value)) {
+        $actionName = strtolower($action) . 'Action';
-                return new Response(render(__DIR__ . '/../templates/error.html.php', [
-                    'message' => "Query parameter \"$key\" is not a string.",
-                ]), 400);
-            }
-        }
-
-        $actionName = $request['action'] ?? 'Frontpage';
-        $actionName = strtolower($actionName) . 'Action';
         $actionName = implode(array_map('ucfirst', explode('-', $actionName)));
         $filePath = __DIR__ . '/../actions/' . $actionName . '.php';
         if (!file_exists($filePath)) {
@@ -80,9 +80,9 @@ final class RssBridge
         }
 
         $className = '\\' . $actionName;
-        $action = new $className();
+        $actionObject = new $className();
 
-        $response = $action->execute($request);
+        $response = $actionObject->execute($request);
 
         if (is_string($response)) {
             $response = new Response($response);

lib/bootstrap.php

@@ -1,12 +1,6 @@
 <?php
 
-// Path to the formats library
-const PATH_LIB_FORMATS = __DIR__ . '/../formats/';
-
-/** Path to the caches library */
 const PATH_LIB_CACHES = __DIR__ . '/../caches/';
-
-/** Path to the cache folder */
 const PATH_CACHE = __DIR__ . '/../cache/';
 
 // Allow larger files for simple_html_dom

lib/http.php

@@ -166,6 +166,46 @@ final class CurlHttpClient implements HttpClient
     }
 }
 
+final class Request
+{
+    private array $get;
+    private array $server;
+
+    private function __construct()
+    {
+    }
+
+    public static function fromGlobals(): self
+    {
+        $self = new self();
+        $self->get = $_GET;
+        $self->server = $_SERVER;
+        return $self;
+    }
+
+    public static function fromCli(array $cliArgs): self
+    {
+        $self = new self();
+        $self->get = $cliArgs;
+        return $self;
+    }
+
+    public function get(string $key, $default = null): ?string
+    {
+        return $this->get[$key] ?? $default;
+    }
+
+    public function server(string $key, string $default = null): ?string
+    {
+        return $this->server[$key] ?? $default;
+    }
+
+    public function toArray(): array
+    {
+        return $this->get;
+    }
+}
+
 final class Response
 {
     public const STATUS_CODES = [

static/connectivity.js

@@ -4,7 +4,7 @@ var abort = false;
 
 window.onload = function() {
 
-	fetch(remote + '/index.php?action=list').then(function(response) {
+	fetch(remote + '/?action=list').then(function(response) {
 		return response.text()
 	}).then(function(data){
 		processBridgeList(data);
@@ -46,9 +46,9 @@ function buildTable(bridgeList) {
 		var td_bridge = document.createElement('td');
 		td_bridge.innerText = bridgeList.bridges[bridge].name;
 
-		// Link to the actual bridge on index.php
+		// Link to the actual bridge on frontpage
 		var a = document.createElement('a');
-		a.href = remote + "/index.php?show_inactive=1#bridge-" + bridge;
+		a.href = remote + "/?show_inactive=1#bridge-" + bridge;
 		a.target = '_blank';
 		a.innerText = '[Show]';
 		a.style.marginLeft = '5px';
@@ -104,7 +104,7 @@ function checkNextBridgeAsync() {
 
 			msg.getElementsByTagName('span')[0].textContent = 'Processing ' + bridge + '...';
 
-			fetch(remote + '/index.php?action=Connectivity&bridge=' + bridge)
+			fetch(remote + '/?action=Connectivity&bridge=' + bridge)
 			.then(function(response) { return response.text() })
 			.then(JSON.parse)
 			.then(processBridgeResultAsync)

tests/Formats/FormatImplementationTest.php

@@ -30,7 +30,7 @@ class FormatImplementationTest extends TestCase
     public function dataFormatsProvider()
     {
         $formats = [];
-        foreach (glob(PATH_LIB_FORMATS . '*.php') as $path) {
+        foreach (glob(__DIR__ . '/../formats/*.php') as $path) {
             $formats[basename($path, '.php')] = [$path];
         }
         return $formats;