PHPStan Level 7 for Minz_Request, FreshRSS_Feed, Minz_Error (#5400)

* PHPStan Level 7 for Minz_Request

* PHPStan Level 7 for FreshRSS_Feed

* PHPStan Level 7 for Minz_Error

app/Controllers/authController.php

@@ -126,13 +126,13 @@ class FreshRSS_auth_Controller extends FreshRSS_ActionController {
 			if (FreshRSS_Context::$user_conf == null) {
 				// Initialise the default user to be able to display the error page
 				FreshRSS_Context::initUser(FreshRSS_Context::$system_conf->default_user);
-				Minz_Error::error(403, array(_t('feedback.auth.login.invalid')), false);
+				Minz_Error::error(403, _t('feedback.auth.login.invalid'), false);
 				return;
 			}
 
 			if (!FreshRSS_Context::$user_conf->enabled || FreshRSS_Context::$user_conf->passwordHash == '') {
 				usleep(random_int(100, 5000));	//Primitive mitigation of timing attacks, in μs
-				Minz_Error::error(403, array(_t('feedback.auth.login.invalid')), false);
+				Minz_Error::error(403, _t('feedback.auth.login.invalid'), false);
 				return;
 			}
 

app/Controllers/userController.php

@@ -278,7 +278,6 @@ class FreshRSS_user_Controller extends FreshRSS_ActionController {
 	 *   - r (i.e. a redirection url, optional)
 	 *
 	 * @todo clean up this method. Idea: write a method to init a user with basic information.
-	 * @todo handle r redirection in Minz_Request::forward directly?
 	 */
 	public function createAction(): void {
 		if (!FreshRSS_Auth::hasAccess('admin') && max_registrations_reached()) {
@@ -372,10 +371,7 @@ class FreshRSS_user_Controller extends FreshRSS_ActionController {
 			}
 		}
 
-		$redirect_url = urldecode(Minz_Request::paramString('r', true));
-		if ($redirect_url === '') {
-			$redirect_url = ['c' => 'user', 'a' => 'manage'];
-		}
+		$redirect_url = ['c' => 'user', 'a' => 'manage'];
 		Minz_Request::forward($redirect_url, true);
 	}
 
@@ -534,10 +530,7 @@ class FreshRSS_user_Controller extends FreshRSS_ActionController {
 			Minz_Error::error(403);
 		}
 
-		$redirect_url = urldecode(Minz_Request::paramString('r', true));
-		if ($redirect_url === '') {
-			$redirect_url = ['c' => 'user', 'a' => 'manage'];
-		}
+		$redirect_url = ['c' => 'user', 'a' => 'manage'];
 
 		if (Minz_Request::isPost()) {
 			$ok = true;

app/FreshRSS.php

@@ -28,7 +28,7 @@ class FreshRSS extends Minz_FrontController {
 		FreshRSS_Context::initSystem();
 		if (FreshRSS_Context::$system_conf == null) {
 			$message = 'Error during context system init!';
-			Minz_Error::error(500, [$message], false);
+			Minz_Error::error(500, $message, false);
 			die($message);
 		}
 
@@ -51,7 +51,7 @@ class FreshRSS extends Minz_FrontController {
 		}
 		if (FreshRSS_Context::$user_conf == null) {
 			$message = 'Error during context user init!';
-			Minz_Error::error(500, [$message], false);
+			Minz_Error::error(500, $message, false);
 			die($message);
 		}
 
@@ -84,10 +84,7 @@ class FreshRSS extends Minz_FrontController {
 				)) {
 				// Token-based protection against XSRF attacks, except for the login or self-create user forms
 				self::initI18n();
-				Minz_Error::error(403, array('error' => array(
-						_t('feedback.access.denied'),
-						' [CSRF]'
-					)));
+				Minz_Error::error(403, ['error' => [_t('feedback.access.denied'), ' [CSRF]']]);
 			}
 		}
 	}

app/Models/Entry.php

@@ -42,9 +42,10 @@ class FreshRSS_Entry extends Minz_Model {
 	 * @param int|string $pubdate
 	 * @param bool|int|null $is_read
 	 * @param bool|int|null $is_favorite
+	 * @param string|array<string> $tags
 	 */
 	public function __construct(int $feedId = 0, string $guid = '', string $title = '', string $authors = '', string $content = '',
-			string $link = '', $pubdate = 0, $is_read = false, $is_favorite = false, string $tags = '') {
+			string $link = '', $pubdate = 0, $is_read = false, $is_favorite = false, $tags = '') {
 		$this->_title($title);
 		$this->_authors($authors);
 		$this->_content($content);
@@ -58,7 +59,7 @@ class FreshRSS_Entry extends Minz_Model {
 	}
 
 	/** @param array{'id'?:string,'id_feed'?:int,'guid'?:string,'title'?:string,'author'?:string,'content'?:string,'link'?:string,'date'?:int|string,
-	 *		'is_read'?:bool|int,'is_favorite'?:bool|int,'tags'?:string,'attributes'?:string,'thumbnail'?:string,'timestamp'?:string,'categories'?:string} $dao */
+	 *		'is_read'?:bool|int,'is_favorite'?:bool|int,'tags'?:string|array<string>,'attributes'?:string,'thumbnail'?:string,'timestamp'?:string} $dao */
 	public static function fromArray(array $dao): FreshRSS_Entry {
 		if (empty($dao['content'])) {
 			$dao['content'] = '';
@@ -92,9 +93,6 @@ class FreshRSS_Entry extends Minz_Model {
 		if (!empty($dao['timestamp'])) {
 			$entry->_date(strtotime($dao['timestamp']) ?: 0);
 		}
-		if (!empty($dao['categories'])) {
-			$entry->_tags($dao['categories']);
-		}
 		if (!empty($dao['attributes'])) {
 			$entry->_attributes('', $dao['attributes']);
 		}

app/Models/Feed.php

@@ -175,8 +175,13 @@ class FreshRSS_Feed extends Minz_Model {
 			return $this->httpAuth;
 		} else {
 			$pos_colon = strpos($this->httpAuth, ':');
-			$user = substr($this->httpAuth, 0, $pos_colon);
-			$pass = substr($this->httpAuth, $pos_colon + 1);
+			if ($pos_colon !== false) {
+				$user = substr($this->httpAuth, 0, $pos_colon);
+				$pass = substr($this->httpAuth, $pos_colon + 1);
+			} else {
+				$user = '';
+				$pass = '';
+			}
 
 			return array(
 				'username' => $user,
@@ -234,6 +239,7 @@ class FreshRSS_Feed extends Minz_Model {
 
 		return $this->nbNotRead + ($includePending ? $this->nbPendingNotRead : 0);
 	}
+
 	public function faviconPrepare(): void {
 		require_once(LIB_PATH . '/favicons.php');
 		$url = $this->website;
@@ -252,10 +258,13 @@ class FreshRSS_Feed extends Minz_Model {
 				($ico_mtime == false || $ico_mtime < $txt_mtime || ($ico_mtime < time() - (14 * 86400)))) {
 				// no ico file or we should download a new one.
 				$url = file_get_contents($txt);
-				download_favicon($url, $ico) || touch($ico);
+				if ($url == false || !download_favicon($url, $ico)) {
+					touch($ico);
+				}
 			}
 		}
 	}
+
 	public static function faviconDelete(string $hash): void {
 		$path = DATA_PATH . '/favicons/' . $hash;
 		@unlink($path . '.ico');
@@ -392,21 +401,16 @@ class FreshRSS_Feed extends Minz_Model {
 				if ((!$mtime) || $simplePie->error()) {
 					$errorMessage = $simplePie->error();
 					throw new FreshRSS_Feed_Exception(
-						($errorMessage == '' ? 'Unknown error for feed' : $errorMessage) . ' [' . $this->url . ']',
+						($errorMessage == '' ? 'Unknown error for feed' : json_encode($errorMessage, JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_LINE_TERMINATORS)) .
+							' [' . $this->url . ']',
 						$simplePie->status_code()
 					);
 				}
 
 				$links = $simplePie->get_links('self');
-				$this->selfUrl = empty($links[0]) ? '' : checkUrl($links[0]);
-				if ($this->selfUrl == false) {
-					$this->selfUrl = '';
-				}
+				$this->selfUrl = empty($links[0]) ? '' : (checkUrl($links[0]) ?: '');
 				$links = $simplePie->get_links('hub');
-				$this->hubUrl = empty($links[0]) ? '' : checkUrl($links[0]);
-				if ($this->hubUrl == false) {
-					$this->hubUrl = '';
-				}
+				$this->hubUrl = empty($links[0]) ? '' : (checkUrl($links[0]) ?: '');
 
 				if ($loadDetails) {
 					// si on a utilisé l’auto-discover, notre url va avoir changé
@@ -494,7 +498,7 @@ class FreshRSS_Feed extends Minz_Model {
 			$title = html_only_entity_decode(strip_tags($item->get_title() ?? ''));
 			$authors = $item->get_authors();
 			$link = $item->get_permalink();
-			$date = @strtotime($item->get_date() ?? '');
+			$date = @strtotime((string)($item->get_date() ?? '')) ?: 0;
 
 			//Tag processing (tag == category)
 			$categories = $item->get_categories();
@@ -696,9 +700,11 @@ class FreshRSS_Feed extends Minz_Model {
 				$item['thumbnail'] = $xPathItemThumbnail == '' ? '' : @$xpath->evaluate('normalize-space(' . $xPathItemThumbnail . ')', $node);
 				if ($xPathItemCategories != '') {
 					$itemCategories = @$xpath->query($xPathItemCategories, $node);
-					if ($itemCategories) {
+					if ($itemCategories !== false) {
+						$item['tags'] = [];
+						/** @var DOMNode $itemCategory */
 						foreach ($itemCategories as $itemCategory) {
-							$item['categories'][] = $itemCategory->textContent;
+							$item['tags'][] = $itemCategory->textContent;
 						}
 					}
 				}
@@ -711,7 +717,7 @@ class FreshRSS_Feed extends Minz_Model {
 
 				if ($item['title'] != '' || $item['content'] != '' || $item['link'] != '') {
 					// HTML-encoding/escaping of the relevant fields (all except 'content')
-					foreach (['author', 'categories', 'guid', 'link', 'thumbnail', 'timestamp', 'title'] as $key) {
+					foreach (['author', 'guid', 'link', 'thumbnail', 'timestamp', 'tags', 'title'] as $key) {
 						if (!empty($item[$key]) && is_string($item[$key])) {
 							$item[$key] = Minz_Helper::htmlspecialchars_utf8($item[$key]);
 						}
@@ -748,6 +754,7 @@ class FreshRSS_Feed extends Minz_Model {
 		if ($keepMaxUnread === null) {
 			$keepMaxUnread = FreshRSS_Context::$user_conf->mark_when['max_n_unread'];
 		}
+		$keepMaxUnread = (int)$keepMaxUnread;
 		if ($keepMaxUnread > 0 && $this->nbNotRead(false) + $this->nbPendingNotRead > $keepMaxUnread) {
 			$feedDAO = FreshRSS_Factory::createFeedDao();
 			return $feedDAO->keepMaxUnread($this->id(), max(0, $keepMaxUnread - $this->nbPendingNotRead));
@@ -821,7 +828,7 @@ class FreshRSS_Feed extends Minz_Model {
 
 	public function lock(): bool {
 		$this->lockPath = TMP_PATH . '/' . $this->hash() . '.freshrss.lock';
-		if (file_exists($this->lockPath) && ((time() - @filemtime($this->lockPath)) > 3600)) {
+		if (file_exists($this->lockPath) && ((time() - (@filemtime($this->lockPath) ?: 0)) > 3600)) {
 			@unlink($this->lockPath);
 		}
 		if (($handle = @fopen($this->lockPath, 'x')) === false) {

lib/Minz/Error.php

@@ -8,34 +8,32 @@
  * The Minz_Error class logs and raises framework errors
  */
 class Minz_Error {
-	public function __construct () { }
+	public function __construct() { }
 
 	/**
 	* Permet de lancer une erreur
 	* @param int $code le type de l'erreur, par défaut 404 (page not found)
-	* @param array<string>|array<string,array<string>> $logs logs d'erreurs découpés de la forme
+	* @param string|array<'error'|'warning'|'notice',array<string>> $logs logs d'erreurs découpés de la forme
 	*      > $logs['error']
 	*      > $logs['warning']
 	*      > $logs['notice']
 	* @param bool $redirect indique s'il faut forcer la redirection (les logs ne seront pas transmis)
 	*/
-	public static function error(int $code = 404, array $logs = [], bool $redirect = true): void {
-		$logs = self::processLogs ($logs);
+	public static function error(int $code = 404, $logs = [], bool $redirect = true): void {
+		$logs = self::processLogs($logs);
 		$error_filename = APP_PATH . '/Controllers/errorController.php';
 
-		if (file_exists ($error_filename)) {
+		if (file_exists($error_filename)) {
 			Minz_Session::_params([
 				'error_code' => $code,
 				'error_logs' => $logs,
 			]);
 
-			Minz_Request::forward (array (
-				'c' => 'error'
-			), $redirect);
+			Minz_Request::forward(['c' => 'error'], $redirect);
 		} else {
 			echo '<h1>An error occurred</h1>' . "\n";
 
-			if (!empty ($logs)) {
+			if (!empty($logs)) {
 				echo '<ul>' . "\n";
 				foreach ($logs as $log) {
 					echo '<li>' . $log . '</li>' . "\n";
@@ -43,40 +41,40 @@ class Minz_Error {
 				echo '</ul>' . "\n";
 			}
 
-			exit ();
+			exit();
 		}
 	}
 
 	/**
 	 * Returns filtered logs
-	 * @param array<string,string>|string $logs logs sorted by category (error, warning, notice)
+	 * @param string|array<'error'|'warning'|'notice',array<string>> $logs logs sorted by category (error, warning, notice)
 	 * @return array<string> list of matching logs, without the category, according to environment preferences (production / development)
 	 */
 	private static function processLogs($logs): array {
-		$conf = Minz_Configuration::get('system');
-		$env = $conf->environment;
-		$logs_ok = array ();
-		$error = array ();
-		$warning = array ();
-		$notice = array ();
+		if (is_string($logs)) {
+			return [$logs];
+		}
+
+		$error = [];
+		$warning = [];
+		$notice = [];
 
-		if (isset ($logs['error'])) {
+		if (isset($logs['error']) && is_array($logs['error'])) {
 			$error = $logs['error'];
 		}
-		if (isset ($logs['warning'])) {
+		if (isset($logs['warning']) && is_array($logs['warning'])) {
 			$warning = $logs['warning'];
 		}
-		if (isset ($logs['notice'])) {
+		if (isset($logs['notice']) && is_array($logs['notice'])) {
 			$notice = $logs['notice'];
 		}
 
-		if ($env == 'production') {
-			$logs_ok = $error;
+		switch (Minz_Configuration::get('system')->environment) {
+			case 'development':
+				return array_merge($error, $warning, $notice);
+			case 'production':
+			default:
+					return $error;
 		}
-		if ($env == 'development') {
-			$logs_ok = array_merge ($error, $warning, $notice);
-		}
-
-		return $logs_ok;
 	}
 }

lib/Minz/FrontController.php

@@ -69,11 +69,7 @@ class Minz_FrontController {
 					$e instanceof Minz_ControllerNotExistException ||
 					$e instanceof Minz_ControllerNotActionControllerException ||
 					$e instanceof Minz_ActionException) {
-				Minz_Error::error (
-					404,
-					array('error' => array ($e->getMessage ())),
-					true
-				);
+				Minz_Error::error(404, ['error' => [$e->getMessage()]], true);
 			} else {
 				self::killApp($e->getMessage());
 			}

lib/Minz/Request.php

@@ -20,8 +20,8 @@ class Minz_Request {
 	/** @var string */
 	private static $default_action_name = 'index';
 
-	/** @var array{'c':string,'a':string,'params':array<string,mixed>}|null */
-	private static $originalRequest = null;
+	/** @var array{'c'?:string,'a'?:string,'params'?:array<string,mixed>} */
+	private static $originalRequest = [];
 
 	/**
 	 * Getteurs
@@ -121,7 +121,7 @@ class Minz_Request {
 	 */
 	public static function paramTextToArray(string $key, array $default = []): array {
 		if (isset(self::$params[$key])) {
-			return preg_split('/\R/', self::$params[$key]);
+			return preg_split('/\R/', self::$params[$key]) ?: [];
 		}
 		return $default;
 	}
@@ -141,8 +141,8 @@ class Minz_Request {
 		];
 	}
 
-	/** @return array{'c':string,'a':string,'params':array<string,mixed>}|null */
-	public static function originalRequest(): ?array {
+	/** @return array{'c'?:string,'a'?:string,'params'?:array<string,mixed>} */
+	public static function originalRequest() {
 		return self::$originalRequest;
 	}
 
@@ -219,7 +219,7 @@ class Minz_Request {
 		$prefix = self::extractPrefix();
 		$path = self::extractPath();
 
-		return filter_var("{$protocol}://{$host}{$port}{$prefix}{$path}", FILTER_SANITIZE_URL);
+		return filter_var("{$protocol}://{$host}{$port}{$prefix}{$path}", FILTER_SANITIZE_URL) ?: '';
 	}
 
 	private static function extractProtocol(): string {
@@ -231,11 +231,11 @@ class Minz_Request {
 
 	private static function extractHost(): string {
 		if ('' != $host = ($_SERVER['HTTP_X_FORWARDED_HOST'] ?? '')) {
-			return parse_url("http://{$host}", PHP_URL_HOST);
+			return parse_url("http://{$host}", PHP_URL_HOST) ?: 'localhost';
 		}
 		if ('' != $host = ($_SERVER['HTTP_HOST'] ?? '')) {
 			// Might contain a port number, and mind IPv6 addresses
-			return parse_url("http://{$host}", PHP_URL_HOST);
+			return parse_url("http://{$host}", PHP_URL_HOST) ?: 'localhost';
 		}
 		if ('' != $host = ($_SERVER['SERVER_NAME'] ?? '')) {
 			return $host;
@@ -276,7 +276,7 @@ class Minz_Request {
 	private static function extractPath(): string {
 		$path = $_SERVER['REQUEST_URI'] ?? '';
 		if ($path != '') {
-			$path = parse_url($path, PHP_URL_PATH);
+			$path = parse_url($path, PHP_URL_PATH) ?: '';
 			return substr($path, -1) === '/' ? rtrim($path, '/') : dirname($path);
 		}
 		return '';
@@ -288,7 +288,7 @@ class Minz_Request {
 	public static function getBaseUrl(): string {
 		$conf = Minz_Configuration::get('system');
 		$url = trim($conf->base_url, ' /\\"');
-		return filter_var($url, FILTER_SANITIZE_URL);
+		return filter_var($url, FILTER_SANITIZE_URL) ?: '';
 	}
 
 	/**
@@ -374,21 +374,15 @@ class Minz_Request {
 	}
 
 	/**
-	 * Relance une requête
-	 * @param string|array{'c'?:string,'a'?:string,'params'?:array<string,mixed>} $url l'url vers laquelle est relancée la requête
-	 * @param bool $redirect si vrai, force la redirection http
-	 *                > sinon, le dispatcher recharge en interne
+	 * Restart a request
+	 * @param array{'c'?:string,'a'?:string,'params'?:array<string,mixed>} $url an array presentation of the URL to route to
+	 * @param bool $redirect If true, uses an HTTP redirection, and if false (default), performs an internal dispatcher redirection.
 	 */
 	public static function forward($url = [], bool $redirect = false): void {
 		if (empty(Minz_Request::originalRequest())) {
 			self::$originalRequest = $url;
 		}
 
-		if (!is_array($url)) {
-			header('Location: ' . $url);
-			exit();
-		}
-
 		$url = Minz_Url::checkControllerUrl($url);
 		$url['params']['rid'] = self::requestId();
 
@@ -433,7 +427,8 @@ class Minz_Request {
 		if ('application/json' !== self::extractContentType()) {
 			return;
 		}
-		if ('' === $ORIGINAL_INPUT = file_get_contents('php://input', false, null, 0, 1048576)) {
+		$ORIGINAL_INPUT = file_get_contents('php://input', false, null, 0, 1048576);
+		if ($ORIGINAL_INPUT == false) {
 			return;
 		}
 		if (null === $json = json_decode($ORIGINAL_INPUT, true)) {

lib/Minz/Url.php

@@ -119,7 +119,7 @@ class Minz_Url {
 		];
 	}
 
-	/** @param array<string,string|array<string,string>>|null $url */
+	/** @param array{'c'?:string,'a'?:string,'params'?:array<string,mixed>} $url */
 	public static function serialize(?array $url = []): string {
 		if (empty($url)) {
 			return '';

tests/phpstan-next.txt

@@ -3,8 +3,5 @@
 # Can be regenerated with something like:
 # find . -type d -name 'vendor' -prune -o -name '*.php' -exec sh -c 'vendor/bin/phpstan analyse --level 7 --memory-limit 512M {} >/dev/null 2>/dev/null || echo {}' \;
 
-./app/Models/Feed.php
-./lib/Minz/Error.php
 ./lib/Minz/Mailer.php
 ./lib/Minz/Migrator.php
-./lib/Minz/Request.php