diff --git a/actions/DisplayAction.php b/actions/DisplayAction.php index 3ad8495f..834f8bc4 100644 --- a/actions/DisplayAction.php +++ b/actions/DisplayAction.php @@ -98,7 +98,16 @@ class DisplayAction implements ActionInterface try { $bridge->loadConfiguration(); // Remove parameters that don't concern bridges - $input = array_diff_key($request, array_fill_keys(['action', 'bridge', 'format', '_noproxy', '_cache_timeout', '_error_time'], '')); + $remove = [ + 'action', + 'bridge', + 'format', + '_noproxy', + '_cache_timeout', + '_error_time', + '_', // Some RSS readers add a cache-busting parameter (_=) to feed URLs, detect and ignore them. + ]; + $input = array_diff_key($request, array_fill_keys($remove, '')); $bridge->setInput($input); $bridge->collectData(); $items = $bridge->getItems(); diff --git a/actions/FrontpageAction.php b/actions/FrontpageAction.php index ad48927d..7606018d 100644 --- a/actions/FrontpageAction.php +++ b/actions/FrontpageAction.php @@ -24,14 +24,14 @@ final class FrontpageAction implements ActionInterface $body = ''; foreach ($bridgeClassNames as $bridgeClassName) { if ($bridgeFactory->isEnabled($bridgeClassName)) { - $body .= BridgeCard::displayBridgeCard($bridgeClassName, $formats); + $body .= BridgeCard::displayBridgeCard($bridgeClassName); $activeBridges++; } elseif ($showInactive) { - $body .= BridgeCard::displayBridgeCard($bridgeClassName, $formats, false) . PHP_EOL; + $body .= BridgeCard::displayBridgeCard($bridgeClassName, false) . "\n"; } } - // todo: cache this renderered template + // todo: cache this renderered template? return render(__DIR__ . '/../templates/frontpage.html.php', [ 'messages' => $messages, 'admin_email' => Configuration::getConfig('admin', 'email'), diff --git a/bridges/PresidenciaPTBridge.php b/bridges/PresidenciaPTBridge.php index 8b02a481..052b2751 100644 --- a/bridges/PresidenciaPTBridge.php +++ b/bridges/PresidenciaPTBridge.php @@ -52,7 +52,9 @@ class PresidenciaPTBridge extends BridgeAbstract public function collectData() { - foreach (array_keys($this->getParameters()['Section']) as $k) { + $contexts = $this->getParameters(); + + foreach (array_keys($contexts['Section']) as $k) { Debug::log('Key: ' . var_export($k, true)); if ($this->getInput($k)) { $html = getSimpleHTMLDOMCached($this->getURI() . $k); diff --git a/lib/BridgeAbstract.php b/lib/BridgeAbstract.php index 8001ba4f..5dc425f9 100644 --- a/lib/BridgeAbstract.php +++ b/lib/BridgeAbstract.php @@ -92,6 +92,9 @@ abstract class BridgeAbstract return static::MAINTAINER; } + /** + * A more correct method name would have been "getContexts" + */ public function getParameters(): array { return static::PARAMETERS; @@ -128,16 +131,17 @@ abstract class BridgeAbstract public function setInput(array $input) { - $context = $input['context'] ?? null; - if ($context) { + // This is the submitted context + $contextName = $input['context'] ?? null; + if ($contextName) { // Context hinting (optional) - $this->queriedContext = $context; + $this->queriedContext = $contextName; unset($input['context']); } - $parameters = $this->getParameters(); + $contexts = $this->getParameters(); - if (!$parameters) { + if (!$contexts) { if ($input) { throw new \Exception('Invalid parameters value(s)'); } @@ -146,15 +150,16 @@ abstract class BridgeAbstract $validator = new ParameterValidator(); - // $input is passed by reference! - if (!$validator->validateInput($input, $parameters)) { - $invalidParameterKeys = array_column($validator->getInvalidParameters(), 'name'); + // $input IS PASSED BY REFERENCE! + $errors = $validator->validateInput($input, $contexts); + if ($errors !== []) { + $invalidParameterKeys = array_column($errors, 'name'); throw new \Exception(sprintf('Invalid parameters value(s): %s', implode(', ', $invalidParameterKeys))); } // Guess the context from input data if (empty($this->queriedContext)) { - $queriedContext = $validator->getQueriedContext($input, $parameters); + $queriedContext = $validator->getQueriedContext($input, $contexts); $this->queriedContext = $queriedContext; } @@ -179,12 +184,12 @@ abstract class BridgeAbstract } // Apply default values to missing data - $contexts = [$queriedContext]; + $contextNames = [$queriedContext]; if (array_key_exists('global', $this->getParameters())) { - $contexts[] = 'global'; + $contextNames[] = 'global'; } - foreach ($contexts as $context) { + foreach ($contextNames as $context) { if (!isset($this->getParameters()[$context])) { // unknown context provided by client, throw exception here? or continue? } @@ -240,7 +245,9 @@ abstract class BridgeAbstract // Only keep guessed context parameters values if (isset($this->inputs[$queriedContext])) { - $this->inputs = [$queriedContext => $this->inputs[$queriedContext]]; + $this->inputs = [ + $queriedContext => $this->inputs[$queriedContext], + ]; } else { $this->inputs = []; } @@ -263,17 +270,20 @@ abstract class BridgeAbstract if (!isset($this->inputs[$this->queriedContext][$input]['value'])) { return null; } - if (array_key_exists('global', $this->getParameters())) { - if (array_key_exists($input, $this->getParameters()['global'])) { - $context = 'global'; + + $contexts = $this->getParameters(); + + if (array_key_exists('global', $contexts)) { + if (array_key_exists($input, $contexts['global'])) { + $contextName = 'global'; } } - if (!isset($context)) { - $context = $this->queriedContext; + if (!isset($contextName)) { + $contextName = $this->queriedContext; } $needle = $this->inputs[$this->queriedContext][$input]['value']; - foreach ($this->getParameters()[$context][$input]['values'] as $first_level_key => $first_level_value) { + foreach ($contexts[$contextName][$input]['values'] as $first_level_key => $first_level_value) { if (!is_array($first_level_value) && $needle === (string)$first_level_value) { return $first_level_key; } elseif (is_array($first_level_value)) { @@ -289,8 +299,11 @@ abstract class BridgeAbstract public function detectParameters($url) { $regex = '/^(https?:\/\/)?(www\.)?(.+?)(\/)?$/'; + + $contexts = $this->getParameters(); + if ( - empty($this->getParameters()) + empty($contexts) && preg_match($regex, $url, $urlMatches) > 0 && preg_match($regex, static::URI, $bridgeUriMatches) > 0 && $urlMatches[3] === $bridgeUriMatches[3] diff --git a/lib/BridgeCard.php b/lib/BridgeCard.php index 8dc2472a..6b835c15 100644 --- a/lib/BridgeCard.php +++ b/lib/BridgeCard.php @@ -6,33 +6,30 @@ final class BridgeCard * Gets a single bridge card * * @param class-string $bridgeClassName The bridge name - * @param array $formats A list of formats * @param bool $isActive Indicates if the bridge is active or not * @return string The bridge card */ - public static function displayBridgeCard($bridgeClassName, $formats, $isActive = true) + public static function displayBridgeCard($bridgeClassName, $isActive = true) { $bridgeFactory = new BridgeFactory(); $bridge = $bridgeFactory->create($bridgeClassName); - $isHttps = str_starts_with($bridge->getURI(), 'https'); - $uri = $bridge->getURI(); $name = $bridge->getName(); $icon = $bridge->getIcon(); $description = $bridge->getDescription(); - $parameters = $bridge->getParameters(); + $contexts = $bridge->getParameters(); if (Configuration::getConfig('proxy', 'url') && Configuration::getConfig('proxy', 'by_bridge')) { - $parameters['global']['_noproxy'] = [ + $contexts['global']['_noproxy'] = [ 'name' => 'Disable proxy (' . (Configuration::getConfig('proxy', 'name') ?: Configuration::getConfig('proxy', 'url')) . ')', 'type' => 'checkbox' ]; } if (Configuration::getConfig('cache', 'custom_timeout')) { - $parameters['global']['_cache_timeout'] = [ + $contexts['global']['_cache_timeout'] = [ 'name' => 'Cache timeout in seconds', 'type' => 'number', 'defaultValue' => $bridge->getCacheTimeout() @@ -41,45 +38,51 @@ final class BridgeCard $shortName = $bridge->getShortName(); $card = << +
-

{$name}

-

{$description}

- - -CARD; +

{$name}

+

{$description}

+ + + + + CARD; // If we don't have any parameter for the bridge, we print a generic form to load it. - if (count($parameters) === 0) { - $card .= self::getForm($bridgeClassName, $formats, $isActive, $isHttps); - - // Display form with cache timeout and/or noproxy options (if enabled) when bridge has no parameters - } elseif (count($parameters) === 1 && array_key_exists('global', $parameters)) { - $card .= self::getForm($bridgeClassName, $formats, $isActive, $isHttps, '', $parameters['global']); + if (count($contexts) === 0) { + // The bridge has zero parameters + $card .= self::getForm($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']); } else { - foreach ($parameters as $parameterName => $parameter) { - if (!is_numeric($parameterName) && $parameterName === 'global') { + // The bridge has one or more contexts (named or unnamed) + foreach ($contexts as $contextName => $contextParameters) { + if ($contextName === 'global') { continue; } - if (array_key_exists('global', $parameters)) { - $parameter = array_merge($parameter, $parameters['global']); + if (array_key_exists('global', $contexts)) { + // Merge the global parameters into current context + $contextParameters = array_merge($contextParameters, $contexts['global']); } - if (!is_numeric($parameterName)) { - $card .= '
' . $parameterName . '
' . PHP_EOL; + if (!is_numeric($contextName)) { + // This is a named context + $card .= '
' . $contextName . '
' . PHP_EOL; } - $card .= self::getForm($bridgeClassName, $formats, $isActive, $isHttps, $parameterName, $parameter); + $card .= self::getForm($bridgeClassName, $isActive, $contextName, $contextParameters); } } $card .= sprintf('', $bridgeClassName); + if ($bridge->getDonationURI() !== '' && Configuration::getConfig('admin', 'donations')) { $card .= sprintf( '

%s ~ Donate

', @@ -94,31 +97,27 @@ CARD; return $card; } - /** - * Get the form body for a bridge - * - * @param class-string $bridgeClassName The bridge name - * @param array $formats A list of supported formats - * @param bool $isActive Indicates if a bridge is enabled or not - * @param bool $isHttps Indicates if a bridge uses HTTPS or not - * @param string $parameterName Sets the bridge context for the current form - * @param array $parameters The bridge parameters - * @return string The form body - */ private static function getForm( - $bridgeClassName, - $formats, - $isActive = false, - $isHttps = false, - $parameterName = '', - $parameters = [] + string $bridgeClassName, + bool $isActive = false, + string $contextName = '', + array $contextParameters = [] ) { - $form = self::getFormHeader($bridgeClassName, $isHttps, $parameterName); + $form = << + + - if (count($parameters) > 0) { + EOD; + + if (!empty($contextName)) { + $form .= sprintf('', $contextName); + } + + if (count($contextParameters) > 0) { $form .= '
'; - foreach ($parameters as $id => $inputEntry) { + foreach ($contextParameters as $id => $inputEntry) { if (!isset($inputEntry['exampleValue'])) { $inputEntry['exampleValue'] = ''; } @@ -127,19 +126,25 @@ CARD; $inputEntry['defaultValue'] = ''; } - $idArg = 'arg-' . urlencode($bridgeClassName) . '-' . urlencode($parameterName) . '-' . urlencode($id); + $idArg = 'arg-' . urlencode($bridgeClassName) . '-' . urlencode($contextName) . '-' . urlencode($id); $inputName = filter_var($inputEntry['name'], FILTER_SANITIZE_FULL_SPECIAL_CHARS); $form .= '' . PHP_EOL; - if (!isset($inputEntry['type']) || $inputEntry['type'] === 'text') { - $form .= self::getTextInput($inputEntry, $idArg, $id); + if ( + !isset($inputEntry['type']) + || $inputEntry['type'] === 'text' + ) { + $form .= self::getTextInput($inputEntry, $idArg, $id) . "\n"; } elseif ($inputEntry['type'] === 'number') { $form .= self::getNumberInput($inputEntry, $idArg, $id); } elseif ($inputEntry['type'] === 'list') { - $form .= self::getListInput($inputEntry, $idArg, $id); + $form .= self::getListInput($inputEntry, $idArg, $id) . "\n"; } elseif ($inputEntry['type'] === 'checkbox') { $form .= self::getCheckboxInput($inputEntry, $idArg, $id); + } else { + $foo = 2; + // oops? } $infoText = []; @@ -171,39 +176,13 @@ CARD; return $form . '' . PHP_EOL; } - /** - * Get the form header for a bridge card - * - * @param class-string $bridgeClassName The bridge name - * @param bool $isHttps If disabled, adds a warning to the form - * @return string The form header - */ - private static function getFormHeader($bridgeClassName, $isHttps = false, $parameterName = '') - { - $form = << - - -EOD; - - if (!empty($parameterName)) { - $form .= sprintf('', $parameterName); - } - - if (!$isHttps) { - $form .= '
Warning: This bridge is not fetching its content through a secure connection
'; - } - - return $form; - } - public static function getTextInput(array $entry, string $id, string $name): string { $defaultValue = filter_var($entry['defaultValue'], FILTER_SANITIZE_FULL_SPECIAL_CHARS); $exampleValue = filter_var($entry['exampleValue'], FILTER_SANITIZE_FULL_SPECIAL_CHARS); $attributes = self::getInputAttributes($entry); - return sprintf('' . "\n", $attributes, $id, $defaultValue, $exampleValue, $name); + return sprintf('', $attributes, $id, $defaultValue, $exampleValue, $name); } public static function getNumberInput(array $entry, string $id, string $name): string @@ -224,7 +203,7 @@ EOD; } $attributes = self::getInputAttributes($entry); - $list = sprintf('' . "\n", $attributes, $id, $name); foreach ($entry['values'] as $name => $value) { if (is_array($value)) { @@ -245,9 +224,9 @@ EOD; $entry['defaultValue'] === $name || $entry['defaultValue'] === $value ) { - $list .= ''; + $list .= '' . "\n"; } else { - $list .= ''; + $list .= '' . "\n"; } } } diff --git a/lib/ParameterValidator.php b/lib/ParameterValidator.php index e8de754c..e2783586 100644 --- a/lib/ParameterValidator.php +++ b/lib/ParameterValidator.php @@ -2,37 +2,26 @@ class ParameterValidator { - private array $invalid = []; - /** - * Check that inputs are actually present in the bridge parameters. - * - * Also check whether input values are allowed. + * Validate and sanitize user inputs against configured bridge parameters (contexts) */ - public function validateInput(&$input, $parameters): bool + public function validateInput(array &$input, $contexts): array { - if (!is_array($input)) { - return false; - } + $errors = []; foreach ($input as $name => $value) { - // Some RSS readers add a cache-busting parameter (_=) to feed URLs, detect and ignore them. - if ($name === '_') { - continue; - } - $registered = false; - foreach ($parameters as $context => $set) { - if (!array_key_exists($name, $set)) { + foreach ($contexts as $contextName => $contextParameters) { + if (!array_key_exists($name, $contextParameters)) { continue; } $registered = true; - if (!isset($set[$name]['type'])) { + if (!isset($contextParameters[$name]['type'])) { // Default type is text - $set[$name]['type'] = 'text'; + $contextParameters[$name]['type'] = 'text'; } - switch ($set[$name]['type']) { + switch ($contextParameters[$name]['type']) { case 'number': $input[$name] = $this->validateNumberValue($value); break; @@ -40,12 +29,12 @@ class ParameterValidator $input[$name] = $this->validateCheckboxValue($value); break; case 'list': - $input[$name] = $this->validateListValue($value, $set[$name]['values']); + $input[$name] = $this->validateListValue($value, $contextParameters[$name]['values']); break; default: case 'text': - if (isset($set[$name]['pattern'])) { - $input[$name] = $this->validateTextValue($value, $set[$name]['pattern']); + if (isset($contextParameters[$name]['pattern'])) { + $input[$name] = $this->validateTextValue($value, $contextParameters[$name]['pattern']); } else { $input[$name] = $this->validateTextValue($value); } @@ -54,56 +43,56 @@ class ParameterValidator if ( is_null($input[$name]) - && isset($set[$name]['required']) - && $set[$name]['required'] + && isset($contextParameters[$name]['required']) + && $contextParameters[$name]['required'] ) { - $this->invalid[] = ['name' => $name, 'reason' => 'Parameter is invalid!']; + $errors[] = ['name' => $name, 'reason' => 'Parameter is invalid!']; } } if (!$registered) { - $this->invalid[] = ['name' => $name, 'reason' => 'Parameter is not registered!']; + $errors[] = ['name' => $name, 'reason' => 'Parameter is not registered!']; } } - return $this->invalid === []; + return $errors; } /** * Get the name of the context matching the provided inputs * * @param array $input Associative array of user data - * @param array $parameters Array of bridge parameters + * @param array $contexts Array of bridge parameters * @return string|null Returns the context name or null if no match was found */ - public function getQueriedContext($input, $parameters) + public function getQueriedContext(array $input, array $contexts) { $queriedContexts = []; // Detect matching context - foreach ($parameters as $context => $set) { - $queriedContexts[$context] = null; + foreach ($contexts as $contextName => $contextParameters) { + $queriedContexts[$contextName] = null; // Ensure all user data exist in the current context - $notInContext = array_diff_key($input, $set); - if (array_key_exists('global', $parameters)) { - $notInContext = array_diff_key($notInContext, $parameters['global']); + $notInContext = array_diff_key($input, $contextParameters); + if (array_key_exists('global', $contexts)) { + $notInContext = array_diff_key($notInContext, $contexts['global']); } if (count($notInContext) > 0) { continue; } // Check if all parameters of the context are satisfied - foreach ($set as $id => $properties) { - if (isset($input[$id]) && !empty($input[$id])) { - $queriedContexts[$context] = true; + foreach ($contextParameters as $id => $properties) { + if (!empty($input[$id])) { + $queriedContexts[$contextName] = true; } elseif ( isset($properties['type']) && ($properties['type'] === 'checkbox' || $properties['type'] === 'list') ) { continue; } elseif (isset($properties['required']) && $properties['required'] === true) { - $queriedContexts[$context] = false; + $queriedContexts[$contextName] = false; break; } } @@ -111,7 +100,7 @@ class ParameterValidator // Abort if one of the globally required parameters is not satisfied if ( - array_key_exists('global', $parameters) + array_key_exists('global', $contexts) && $queriedContexts['global'] === false ) { return null; @@ -124,9 +113,9 @@ class ParameterValidator if (isset($input['context'])) { return $input['context']; } - foreach ($queriedContexts as $context => $queried) { + foreach ($queriedContexts as $context2 => $queried) { if (is_null($queried)) { - return $context; + return $context2; } } return null; @@ -138,11 +127,6 @@ class ParameterValidator } } - public function getInvalidParameters(): array - { - return $this->invalid; - } - private function validateTextValue($value, $pattern = null) { if (is_null($pattern)) { diff --git a/static/style.css b/static/style.css index b835c98d..cfaf66a1 100644 --- a/static/style.css +++ b/static/style.css @@ -281,16 +281,6 @@ p.maintainer { text-align: right; } -.secure-warning { - background-color: #ffc600; - color: #5f5f5f; - box-shadow: 0px 1px 2px rgba(0, 0, 0, 0.3); - border-radius: 2px; - border: 1px solid transparent; - width: 80%; - margin: auto auto 6px; -} - .error strong { display: inline-block; width: 100px; @@ -395,11 +385,6 @@ button { } } /* @supports (display: grid) */ - - .secure-warning { - width: 100%; - } - } /* Dark theme */ diff --git a/tests/BridgeCardTest.php b/tests/BridgeCardTest.php index 7a8f084d..519ac7f1 100644 --- a/tests/BridgeCardTest.php +++ b/tests/BridgeCardTest.php @@ -30,12 +30,12 @@ class BridgeCardTest extends TestCase 'defaultValue' => 'yo1', 'exampleValue' => 'yo2', ]; - $this->assertSame('' . "\n", BridgeCard::getTextInput($entry, 'id', 'name')); + $this->assertSame('', BridgeCard::getTextInput($entry, 'id', 'name')); $entry = [ 'values' => [], ]; - $this->assertSame('', BridgeCard::getListInput($entry, 'id', 'name')); + $this->assertSame('', BridgeCard::getListInput($entry, 'id', 'name')); $entry = [ 'defaultValue' => 2, @@ -43,7 +43,7 @@ class BridgeCardTest extends TestCase 'foo' => 'bar', ], ]; - $this->assertSame('', BridgeCard::getListInput($entry, 'id', 'name')); + $this->assertSame('', BridgeCard::getListInput($entry, 'id', 'name')); // optgroup $entry = [ @@ -52,6 +52,9 @@ class BridgeCardTest extends TestCase 'f' => 'b', ]], ]; - $this->assertSame('', BridgeCard::getListInput($entry, 'id', 'name')); + $this->assertSame( + '', + BridgeCard::getListInput($entry, 'id', 'name') + ); } -} \ No newline at end of file +} diff --git a/tests/ParameterValidatorTest.php b/tests/ParameterValidatorTest.php index 1b241c2c..59d7b2b9 100644 --- a/tests/ParameterValidatorTest.php +++ b/tests/ParameterValidatorTest.php @@ -20,7 +20,7 @@ class ParameterValidatorTest extends TestCase ], ] ]; - $this->assertTrue($sut->validateInput($input, $parameters)); + $this->assertSame([], $sut->validateInput($input, $parameters)); } public function test2() @@ -35,6 +35,6 @@ class ParameterValidatorTest extends TestCase ], ] ]; - $this->assertFalse($sut->validateInput($input, $parameters)); + $this->assertNotEmpty($sut->validateInput($input, $parameters)); } }