diff --git a/CHANGELOG.md b/CHANGELOG.md index e26722029..45fb24c2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ 1. [](#new) * Added `FlexForm::setSubmitMethod()` to customize form submit action +1. [](#improved) + * Improved GPM error handling 1. [](#bugfix) * Fixed `bin/gpm uninstall` script not working because of bad typehint [#3172](https://github.com/getgrav/grav/issues/3172) * Fixed `login: visibility_requires_access` not working in pages [#3176](https://github.com/getgrav/grav/issues/3176) diff --git a/system/src/Grav/Common/GPM/AbstractCollection.php b/system/src/Grav/Common/GPM/AbstractCollection.php index 77df48062..6d1c914d2 100644 --- a/system/src/Grav/Common/GPM/AbstractCollection.php +++ b/system/src/Grav/Common/GPM/AbstractCollection.php @@ -22,7 +22,7 @@ abstract class AbstractCollection extends Iterator */ public function toJson() { - return json_encode($this->toArray()); + return json_encode($this->toArray(), JSON_THROW_ON_ERROR); } /** diff --git a/system/src/Grav/Common/GPM/Common/AbstractPackageCollection.php b/system/src/Grav/Common/GPM/Common/AbstractPackageCollection.php index 936ee0b31..99f8ef0ff 100644 --- a/system/src/Grav/Common/GPM/Common/AbstractPackageCollection.php +++ b/system/src/Grav/Common/GPM/Common/AbstractPackageCollection.php @@ -31,7 +31,7 @@ abstract class AbstractPackageCollection extends Iterator $items[$name] = $package->toArray(); } - return json_encode($items); + return json_encode($items, JSON_THROW_ON_ERROR); } /** diff --git a/system/src/Grav/Common/GPM/Common/Package.php b/system/src/Grav/Common/GPM/Common/Package.php index d6b90e45e..2f49e45b5 100644 --- a/system/src/Grav/Common/GPM/Common/Package.php +++ b/system/src/Grav/Common/GPM/Common/Package.php @@ -53,6 +53,7 @@ class Package /** * @param string $key * @param mixed $value + * @return void */ public function __set($key, $value) { diff --git a/system/src/Grav/Common/GPM/GPM.php b/system/src/Grav/Common/GPM/GPM.php index 735f6cd30..289561a54 100644 --- a/system/src/Grav/Common/GPM/GPM.php +++ b/system/src/Grav/Common/GPM/GPM.php @@ -32,10 +32,9 @@ class GPM extends Iterator { /** @var Local\Packages Local installed Packages */ private $installed; - /** @var Remote\Packages Remote available Packages */ + /** @var Remote\Packages|null Remote available Packages */ private $repository; - - /** @var Remote\GravCore */ + /** @var Remote\GravCore|null Remove Grav Packages */ public $grav; /** @var array Internal cache */ @@ -93,6 +92,7 @@ class GPM extends Iterator $items[$type] = $to_install; $items['total'] += count($to_install); } + return $items; } @@ -116,15 +116,7 @@ class GPM extends Iterator */ public function getInstalledPackage($slug) { - if (isset($this->installed['plugins'][$slug])) { - return $this->installed['plugins'][$slug]; - } - - if (isset($this->installed['themes'][$slug])) { - return $this->installed['themes'][$slug]; - } - - return null; + return $this->getInstalledPlugin($slug) ?? $this->getInstalledTheme($slug); } /** @@ -135,7 +127,7 @@ class GPM extends Iterator */ public function getInstalledPlugin($slug) { - return $this->installed['plugins'][$slug]; + return $this->installed['plugins'][$slug] ?? null; } /** @@ -164,7 +156,9 @@ class GPM extends Iterator */ public function isPluginInstalledAsSymlink($slug) { - return $this->installed['plugins'][$slug]->symlink; + $plugin = $this->getInstalledPlugin($slug); + + return (bool)($plugin->symlink ?? false); } /** @@ -175,7 +169,7 @@ class GPM extends Iterator */ public function getInstalledTheme($slug) { - return $this->installed['themes'][$slug]; + return $this->installed['themes'][$slug] ?? null; } /** @@ -206,12 +200,7 @@ class GPM extends Iterator */ public function countUpdates() { - $count = 0; - - $count += count($this->getUpdatablePlugins()); - $count += count($this->getUpdatableThemes()); - - return $count; + return count($this->getUpdatablePlugins()) + count($this->getUpdatableThemes()); } /** @@ -248,7 +237,7 @@ class GPM extends Iterator { $items = []; - if (!$this->repository) { + if (null === $this->repository) { return $items; } @@ -264,7 +253,7 @@ class GPM extends Iterator continue; } - $local_version = $plugin->version ?: 'Unknown'; + $local_version = $plugin->version ?? 'Unknown'; $remote_version = $repository[$slug]->version; if (version_compare($local_version, $remote_version) < 0) { @@ -288,6 +277,10 @@ class GPM extends Iterator */ public function getLatestVersionOfPackage($package_name) { + if (null === $this->repository) { + return null; + } + $repository = $this->repository['plugins']; if (isset($repository[$package_name])) { return $repository[$package_name]->available ?: $repository[$package_name]->version; @@ -334,7 +327,7 @@ class GPM extends Iterator { $items = []; - if (!$this->repository) { + if (null === $this->repository) { return $items; } @@ -350,7 +343,7 @@ class GPM extends Iterator continue; } - $local_version = $plugin->version ?: 'Unknown'; + $local_version = $plugin->version ?? 'Unknown'; $remote_version = $repository[$slug]->version; if (version_compare($local_version, $remote_version) < 0) { @@ -385,7 +378,7 @@ class GPM extends Iterator */ public function getReleaseType($package_name) { - if (!$this->repository) { + if (null === $this->repository) { return null; } @@ -422,8 +415,8 @@ class GPM extends Iterator */ public function isTestingRelease($package_name) { - $hasTesting = isset($this->getInstalledPackage($package_name)->testing); - $testing = $hasTesting ? $this->getInstalledPackage($package_name)->testing : false; + $package = $this->getInstalledPackage($package_name); + $testing = $package->testing ?? false; return $this->getReleaseType($package_name) === 'testing' || $testing; } @@ -432,17 +425,19 @@ class GPM extends Iterator * Returns a Plugin from the repository * * @param string $slug The slug of the Plugin - * @return mixed Package if found, NULL if not + * @return Remote\Package|null Package if found, NULL if not */ public function getRepositoryPlugin($slug) { - return $this->repository['plugins'][$slug] ?? null; + $packages = $this->getRepositoryPlugins(); + + return $packages ? ($packages[$slug] ?? null) : null; } /** * Returns the list of Plugins available in the repository * - * @return Iterator The Plugins remotely available + * @return Iterator|null The Plugins remotely available */ public function getRepositoryPlugins() { @@ -453,17 +448,19 @@ class GPM extends Iterator * Returns a Theme from the repository * * @param string $slug The slug of the Theme - * @return mixed Package if found, NULL if not + * @return Remote\Package|null Package if found, NULL if not */ public function getRepositoryTheme($slug) { - return $this->repository['themes'][$slug] ?? null; + $packages = $this->getRepositoryThemes(); + + return $packages ? ($packages[$slug] ?? null) : null; } /** * Returns the list of Themes available in the repository * - * @return Iterator The Themes remotely available + * @return Iterator|null The Themes remotely available */ public function getRepositoryThemes() { @@ -473,7 +470,7 @@ class GPM extends Iterator /** * Returns the list of Plugins and Themes available in the repository * - * @return Remote\Packages Available Plugins and Themes + * @return Remote\Packages|null Available Plugins and Themes * Format: ['plugins' => array, 'themes' => array] */ public function getRepository() @@ -492,12 +489,7 @@ class GPM extends Iterator { $search = strtolower($search); - $found = $this->getRepositoryTheme($search); - if ($found) { - return $found; - } - - $found = $this->getRepositoryPlugin($search); + $found = $this->getRepositoryPlugin($search) ?? $this->getRepositoryTheme($search); if ($found) { return $found; } @@ -505,31 +497,27 @@ class GPM extends Iterator $themes = $this->getRepositoryThemes(); $plugins = $this->getRepositoryPlugins(); - if (!$themes && !$plugins) { + if (null === $themes || null === $plugins) { if (!is_writable(ROOT_DIR . '/cache/gpm')) { - throw new RuntimeException("The cache/gpm folder is not writable. Please check the folder permissions."); + throw new RuntimeException('The cache/gpm folder is not writable. Please check the folder permissions.'); } if ($ignore_exception) { return false; } - throw new RuntimeException("GPM not reachable. Please check your internet connection or check the Grav site is reachable"); + throw new RuntimeException('GPM not reachable. Please check your internet connection or check the Grav site is reachable'); } - if ($themes) { - foreach ($themes as $slug => $theme) { - if ($search == $slug || $search == $theme->name) { - return $theme; - } + foreach ($themes as $slug => $theme) { + if ($search === $slug || $search === $theme->name) { + return $theme; } } - if ($plugins) { - foreach ($plugins as $slug => $plugin) { - if ($search == $slug || $search == $plugin->name) { - return $plugin; - } + foreach ($plugins as $slug => $plugin) { + if ($search === $slug || $search === $plugin->name) { + return $plugin; } } @@ -546,9 +534,13 @@ class GPM extends Iterator public static function downloadPackage($package_file, $tmp) { $package = parse_url($package_file); - $filename = basename($package['path']); + if (!is_array($package)) { + throw new \RuntimeException("Malformed GPM URL: {$package_file}"); + } - if (Grav::instance()['config']->get('system.gpm.official_gpm_only') && $package['host'] !== 'getgrav.org') { + $filename = basename($package['path'] ?? ''); + + if (Grav::instance()['config']->get('system.gpm.official_gpm_only') && ($package['host'] ?? null) !== 'getgrav.org') { throw new RuntimeException('Only official GPM URLs are allowed. You can modify this behavior in the System configuration.'); } @@ -574,10 +566,10 @@ class GPM extends Iterator { $package_file = realpath($package_file); - if (file_exists($package_file)) { + if ($package_file && file_exists($package_file)) { $filename = basename($package_file); Folder::create($tmp); - copy(realpath($package_file), $tmp . DS . $filename); + copy($package_file, $tmp . DS . $filename); return $tmp . DS . $filename; } @@ -614,8 +606,13 @@ class GPM extends Iterator if (Utils::contains($name, 'plugin')) { return 'plugin'; } - foreach (glob($source . '*.php') as $filename) { + + $glob = glob($source . '*.php') ?: []; + foreach ($glob as $filename) { $contents = file_get_contents($filename); + if (!$contents) { + continue; + } if (preg_match($theme_regex, $contents)) { return 'theme'; } @@ -638,13 +635,16 @@ class GPM extends Iterator { $ignore_yaml_files = ['blueprints', 'languages']; - foreach (glob($source . '*.yaml') as $filename) { + $glob = glob($source . '*.yaml') ?: []; + foreach ($glob as $filename) { $name = strtolower(basename($filename, '.yaml')); if (in_array($name, $ignore_yaml_files)) { continue; } + return $name; } + return false; } @@ -684,6 +684,7 @@ class GPM extends Iterator } else { $install_path = $locator->findResource('plugins://', false) . DS . $name; } + return $install_path; } @@ -783,7 +784,7 @@ class GPM extends Iterator */ public function getVersionOfDependencyRequiredByPackage($package_slug, $dependency_slug) { - $dependencies = $this->getInstalledPackage($package_slug)->dependencies; + $dependencies = $this->getInstalledPackage($package_slug)->dependencies ?? []; foreach ($dependencies as $dependency) { if (isset($dependency[$dependency_slug])) { return $dependency[$dependency_slug]; @@ -827,13 +828,11 @@ class GPM extends Iterator $version, $other_dependency_version ); - if (!$compatible) { - if (!in_array($dependent_package, $ignore_packages_list, true)) { - throw new RuntimeException( - "Package $slug is required in an older version by package $dependent_package. This package needs a newer version, and because of this it cannot be installed. The $dependent_package package must be updated to use a newer release of $slug.", - 2 - ); - } + if (!$compatible && !in_array($dependent_package, $ignore_packages_list, true)) { + throw new RuntimeException( + "Package $slug is required in an older version by package $dependent_package. This package needs a newer version, and because of this it cannot be installed. The $dependent_package package must be updated to use a newer release of $slug.", + 2 + ); } } } @@ -876,6 +875,7 @@ class GPM extends Iterator { $dependencies = $this->calculateMergedDependenciesOfPackages($packages); foreach ($dependencies as $dependency_slug => $dependencyVersionWithOperator) { + $dependency_slug = (string)$dependency_slug; if (in_array($dependency_slug, $packages, true)) { unset($dependencies[$dependency_slug]); continue; @@ -883,10 +883,9 @@ class GPM extends Iterator // Check PHP version if ($dependency_slug === 'php') { - $current_php_version = phpversion(); if (version_compare( $this->calculateVersionNumberFromDependencyVersion($dependencyVersionWithOperator), - $current_php_version + PHP_VERSION ) === 1 ) { //Needs a Grav update first @@ -929,19 +928,18 @@ class GPM extends Iterator $currentlyInstalledVersion = $package_yaml['version']; // if requirement is next significant release, check is compatible with currently installed version, might not be - if ($this->versionFormatIsNextSignificantRelease($dependencyVersionWithOperator)) { - if ($this->firstVersionIsLower($dependencyVersion, $currentlyInstalledVersion)) { - $compatible = $this->checkNextSignificantReleasesAreCompatible( - $dependencyVersion, - $currentlyInstalledVersion - ); + if ($this->versionFormatIsNextSignificantRelease($dependencyVersionWithOperator) + && $this->firstVersionIsLower($dependencyVersion, $currentlyInstalledVersion)) { + $compatible = $this->checkNextSignificantReleasesAreCompatible( + $dependencyVersion, + $currentlyInstalledVersion + ); - if (!$compatible) { - throw new RuntimeException( - 'Dependency ' . $dependency_slug . ' is required in an older version than the one installed. This package must be updated. Please get in touch with its developer.', - 2 - ); - } + if (!$compatible) { + throw new RuntimeException( + 'Dependency ' . $dependency_slug . ' is required in an older version than the one installed. This package must be updated. Please get in touch with its developer.', + 2 + ); } } @@ -958,13 +956,11 @@ class GPM extends Iterator if ($this->firstVersionIsLower($currentlyInstalledVersion, $dependencyVersion)) { $dependencies[$dependency_slug] = 'update'; + } elseif ($currentlyInstalledVersion === $latestRelease) { + unset($dependencies[$dependency_slug]); } else { - if ($currentlyInstalledVersion == $latestRelease) { - unset($dependencies[$dependency_slug]); - } else { - // an update is not strictly required mark as 'ignore' - $dependencies[$dependency_slug] = 'ignore'; - } + // an update is not strictly required mark as 'ignore' + $dependencies[$dependency_slug] = 'ignore'; } } else { $dependencyVersion = $this->calculateVersionNumberFromDependencyVersion($dependencyVersionWithOperator); @@ -1212,7 +1208,7 @@ class GPM extends Iterator $i = 0; while ($i < count($version1array) - 1) { - if ($version1array[$i] != $version2array[$i]) { + if ($version1array[$i] !== $version2array[$i]) { return false; } $i++; diff --git a/system/src/Grav/Common/GPM/Installer.php b/system/src/Grav/Common/GPM/Installer.php index c894b74b4..9b9f90548 100644 --- a/system/src/Grav/Common/GPM/Installer.php +++ b/system/src/Grav/Common/GPM/Installer.php @@ -187,7 +187,11 @@ class Installer return false; } - $package_folder_name = preg_replace('#\./$#', '', $zip->getNameIndex(0)); + $package_folder_name = $zip->getNameIndex(0); + if ($package_folder_name === false) { + throw new \RuntimeException('Bad package file: ' . basename($zip_file)); + } + $package_folder_name = preg_replace('#\./$#', '', $package_folder_name); $zip->close(); return $destination . '/' . $package_folder_name; @@ -208,8 +212,6 @@ class Installer */ private static function loadInstaller($installer_file_folder, $is_install) { - $installer = null; - $installer_file_folder = rtrim($installer_file_folder, DS); $install_file = $installer_file_folder . DS . 'install.php'; @@ -242,13 +244,13 @@ class Installer return $class_name; } - $class_name_alphanumeric = preg_replace('/[^a-zA-Z0-9]+/', '', $class_name); + $class_name_alphanumeric = preg_replace('/[^a-zA-Z0-9]+/', '', $class_name) ?? $class_name; if (class_exists($class_name_alphanumeric)) { return $class_name_alphanumeric; } - return $installer; + return null; } /** @@ -308,7 +310,8 @@ class Installer } if ($file->getFilename() === 'bin') { - foreach (glob($path . DS . '*') as $bin_file) { + $glob = glob($path . DS . '*') ?: []; + foreach ($glob as $bin_file) { @chmod($bin_file, 0755); } } @@ -528,6 +531,7 @@ class Installer * Allows to manually set an error * * @param int|string $error the Error code + * @return void */ public static function setError($error) { diff --git a/system/src/Grav/Common/GPM/Licenses.php b/system/src/Grav/Common/GPM/Licenses.php index 1383afb1b..c8a5fbcc3 100644 --- a/system/src/Grav/Common/GPM/Licenses.php +++ b/system/src/Grav/Common/GPM/Licenses.php @@ -70,12 +70,13 @@ class Licenses $licenses = self::getLicenseFile(); $data = (array)$licenses->content(); $licenses->free(); - $slug = strtolower($slug); - if (!$slug) { + if (null === $slug) { return $data['licenses'] ?? []; } + $slug = strtolower($slug); + return $data['licenses'][$slug] ?? ''; } @@ -92,7 +93,7 @@ class Licenses return false; } - return preg_match('#' . self::$regex. '#', $license); + return (bool)preg_match('#' . self::$regex. '#', $license); } /** diff --git a/system/src/Grav/Common/GPM/Remote/GravCore.php b/system/src/Grav/Common/GPM/Remote/GravCore.php index 574a095f8..892aa9f05 100644 --- a/system/src/Grav/Common/GPM/Remote/GravCore.php +++ b/system/src/Grav/Common/GPM/Remote/GravCore.php @@ -127,14 +127,15 @@ class GravCore extends AbstractPackageCollection /** * Returns the minimum PHP version * - * @return null|string + * @return string */ public function getMinPHPVersion() { // If non min set, assume current PHP version if (null === $this->min_php) { - $this->min_php = phpversion(); + $this->min_php = PHP_VERSION; } + return $this->min_php; } diff --git a/system/src/Grav/Common/GPM/Remote/Package.php b/system/src/Grav/Common/GPM/Remote/Package.php index 1245d7ac3..8dd82b75c 100644 --- a/system/src/Grav/Common/GPM/Remote/Package.php +++ b/system/src/Grav/Common/GPM/Remote/Package.php @@ -51,7 +51,7 @@ class Package extends BasePackage implements \JsonSerializable $diffLog = []; foreach ((array)$this->data['changelog'] as $version => $changelog) { - preg_match("/[\w\-\.]+/", $version, $cleanVersion); + preg_match("/[\w\-.]+/", $version, $cleanVersion); if (!$cleanVersion || version_compare($diff, $cleanVersion[0], '>=')) { continue; diff --git a/system/src/Grav/Common/GPM/Response.php b/system/src/Grav/Common/GPM/Response.php index 85e57bc92..29665f0b0 100644 --- a/system/src/Grav/Common/GPM/Response.php +++ b/system/src/Grav/Common/GPM/Response.php @@ -30,7 +30,7 @@ class Response { /** @var callable The callback for the progress, either a function or callback in array notation */ public static $callback = null; - + /** @var string[] */ private static $headers = [ 'User-Agent' => 'Grav CMS' ]; @@ -80,7 +80,7 @@ class Response // Use callback if provided if ($callback) { self::$callback = $callback; - $options->setOnProgress(['Grav\Common\GPM\Response', 'progress']); + $options->setOnProgress([Response::class, 'progress']); } $preferred_method = $config->get('system.gpm.method', 'auto'); diff --git a/system/src/Grav/Common/GPM/Upgrader.php b/system/src/Grav/Common/GPM/Upgrader.php index 43934a7e6..3164ed825 100644 --- a/system/src/Grav/Common/GPM/Upgrader.php +++ b/system/src/Grav/Common/GPM/Upgrader.php @@ -95,8 +95,7 @@ class Upgrader */ public function meetsRequirements() { - $current_php_version = phpversion(); - if (version_compare($current_php_version, $this->minPHPVersion(), '<')) { + if (version_compare(PHP_VERSION, $this->minPHPVersion(), '<')) { return false; } @@ -113,6 +112,7 @@ class Upgrader if (null === $this->min_php) { $this->min_php = $this->remote->getMinPHPVersion(); } + return $this->min_php; } @@ -131,7 +131,6 @@ class Upgrader * * @return bool True if Grav is symlinked, False otherwise. */ - public function isSymlink() { return $this->remote->isSymlink(); diff --git a/system/src/Grav/Console/Gpm/DirectInstallCommand.php b/system/src/Grav/Console/Gpm/DirectInstallCommand.php index 8712916c0..a79bd6a44 100644 --- a/system/src/Grav/Console/Gpm/DirectInstallCommand.php +++ b/system/src/Grav/Console/Gpm/DirectInstallCommand.php @@ -113,6 +113,7 @@ class DirectInstallCommand extends GpmCommand $io->newLine(); $io->writeln("Preparing to install {$package_file}"); + $zip = null; if (Response::isRemote($package_file)) { $io->write(' |- Downloading package... 0%'); try { @@ -140,7 +141,7 @@ class DirectInstallCommand extends GpmCommand } } - if (file_exists($zip)) { + if ($zip && file_exists($zip)) { $tmp_source = $tmp_dir . uniqid('/Grav-', false); $io->write(' |- Extracting package... ');