refactor/fix (#3924)

This commit is contained in:
Dag 2024-01-25 13:03:00 +01:00 committed by GitHub
parent 06b299e627
commit 9574c17ddc
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 153 additions and 178 deletions

View file

@ -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 (_=<timestamp>) to feed URLs, detect and ignore them.
];
$input = array_diff_key($request, array_fill_keys($remove, ''));
$bridge->setInput($input);
$bridge->collectData();
$items = $bridge->getItems();

View file

@ -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'),

View file

@ -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);

View file

@ -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]

View file

@ -6,33 +6,30 @@ final class BridgeCard
* Gets a single bridge card
*
* @param class-string<BridgeAbstract> $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 = <<<CARD
<section
class="bridge-card"
id="bridge-{$bridgeClassName}"
data-ref="{$name}"
data-short-name="$shortName"
>
<section
class="bridge-card"
id="bridge-{$bridgeClassName}"
data-ref="{$name}"
data-short-name="$shortName"
>
<h2><a href="{$uri}">{$name}</a></h2>
<p class="description">{$description}</p>
<input type="checkbox" class="showmore-box" id="showmore-{$bridgeClassName}" />
<label class="showmore" for="showmore-{$bridgeClassName}">Show more</label>
CARD;
<h2><a href="{$uri}">{$name}</a></h2>
<p class="description">{$description}</p>
<input type="checkbox" class="showmore-box" id="showmore-{$bridgeClassName}" />
<label class="showmore" for="showmore-{$bridgeClassName}">Show more</label>
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 .= '<h5>' . $parameterName . '</h5>' . PHP_EOL;
if (!is_numeric($contextName)) {
// This is a named context
$card .= '<h5>' . $contextName . '</h5>' . PHP_EOL;
}
$card .= self::getForm($bridgeClassName, $formats, $isActive, $isHttps, $parameterName, $parameter);
$card .= self::getForm($bridgeClassName, $isActive, $contextName, $contextParameters);
}
}
$card .= sprintf('<label class="showless" for="showmore-%s">Show less</label>', $bridgeClassName);
if ($bridge->getDonationURI() !== '' && Configuration::getConfig('admin', 'donations')) {
$card .= sprintf(
'<p class="maintainer">%s ~ <a href="%s">Donate</a></p>',
@ -94,31 +97,27 @@ CARD;
return $card;
}
/**
* Get the form body for a bridge
*
* @param class-string<BridgeAbstract> $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 = <<<EOD
<form method="GET" action="?">
<input type="hidden" name="action" value="display" />
<input type="hidden" name="bridge" value="{$bridgeClassName}" />
if (count($parameters) > 0) {
EOD;
if (!empty($contextName)) {
$form .= sprintf('<input type="hidden" name="context" value="%s" />', $contextName);
}
if (count($contextParameters) > 0) {
$form .= '<div class="parameters">';
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 .= '<label for="' . $idArg . '">' . $inputName . '</label>' . 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 . '</form>' . PHP_EOL;
}
/**
* Get the form header for a bridge card
*
* @param class-string<BridgeAbstract> $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
<form method="GET" action="?">
<input type="hidden" name="action" value="display" />
<input type="hidden" name="bridge" value="{$bridgeClassName}" />
EOD;
if (!empty($parameterName)) {
$form .= sprintf('<input type="hidden" name="context" value="%s" />', $parameterName);
}
if (!$isHttps) {
$form .= '<div class="secure-warning">Warning: This bridge is not fetching its content through a secure connection</div>';
}
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('<input %s id="%s" type="text" value="%s" placeholder="%s" name="%s" />' . "\n", $attributes, $id, $defaultValue, $exampleValue, $name);
return sprintf('<input %s id="%s" type="text" value="%s" placeholder="%s" name="%s" />', $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('<select %s id="%s" name="%s" >', $attributes, $id, $name);
$list = sprintf('<select %s id="%s" name="%s" >' . "\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 .= '<option value="' . $value . '" selected>' . $name . '</option>';
$list .= '<option value="' . $value . '" selected>' . $name . '</option>' . "\n";
} else {
$list .= '<option value="' . $value . '">' . $name . '</option>';
$list .= '<option value="' . $value . '">' . $name . '</option>' . "\n";
}
}
}

View file

@ -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 (_=<timestamp>) 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)) {

View file

@ -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 */

View file

@ -30,12 +30,12 @@ class BridgeCardTest extends TestCase
'defaultValue' => 'yo1',
'exampleValue' => 'yo2',
];
$this->assertSame('<input id="id" type="text" value="yo1" placeholder="yo2" name="name" />' . "\n", BridgeCard::getTextInput($entry, 'id', 'name'));
$this->assertSame('<input id="id" type="text" value="yo1" placeholder="yo2" name="name" />', BridgeCard::getTextInput($entry, 'id', 'name'));
$entry = [
'values' => [],
];
$this->assertSame('<select id="id" name="name" ></select>', BridgeCard::getListInput($entry, 'id', 'name'));
$this->assertSame('<select id="id" name="name" >' . "\n" . '</select>', BridgeCard::getListInput($entry, 'id', 'name'));
$entry = [
'defaultValue' => 2,
@ -43,7 +43,7 @@ class BridgeCardTest extends TestCase
'foo' => 'bar',
],
];
$this->assertSame('<select id="id" name="name" ><option value="bar">foo</option></select>', BridgeCard::getListInput($entry, 'id', 'name'));
$this->assertSame('<select id="id" name="name" >' . "\n" . '<option value="bar">foo</option>' . "\n" . '</select>', BridgeCard::getListInput($entry, 'id', 'name'));
// optgroup
$entry = [
@ -52,6 +52,9 @@ class BridgeCardTest extends TestCase
'f' => 'b',
]],
];
$this->assertSame('<select id="id" name="name" ><optgroup label="kek"><option value="b">f</option></optgroup></select>', BridgeCard::getListInput($entry, 'id', 'name'));
$this->assertSame(
'<select id="id" name="name" >' . "\n" . '<optgroup label="kek"><option value="b">f</option></optgroup></select>',
BridgeCard::getListInput($entry, 'id', 'name')
);
}
}
}

View file

@ -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));
}
}