plugin-update-checker icon indicating copy to clipboard operation
plugin-update-checker copied to clipboard

PHP 8.1 compatibility - Deprecation notice

Open daigo75 opened this issue 3 years ago • 8 comments

There seems to be a minor glitch when using this library and PHP 8.1. On that version, the following deprecation appears:

Deprecated: preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated in path/to/yahnis-elsts/plugin-update-checker\Puc\v4p13\Factory.php on line 268

Which refers to the following:

//Which hosting service does the URL point to?
$host = parse_url($metadataUrl, PHP_URL_HOST);
$path = parse_url($metadataUrl, PHP_URL_PATH);

//Check if the path looks like "/user-name/repository".
//For GitLab.com it can also be "/user/group1/group2/.../repository".
$repoRegex = '@^/?([^/]+?)/([^/#?&]+?)/?$@';
if ( $host === 'gitlab.com' ) {
	$repoRegex = '@^/?(?:[^/#?&]++/){1,20}(?:[^/#?&]++)/?$@';
}

// Line 268 is the following one
if ( preg_match($repoRegex, $path) ) {
   // ...
}

If $path is null, the deprecation notice appears. I reckon that typecasting the path to a string, as in $path = (string)parse_url($metadataUrl, PHP_URL_PATH); could be sufficient to remove the deprecation notice.

daigo75 avatar Aug 06 '22 12:08 daigo75

It seems to me that $path should never be null during normal operation. What kind of a URL/configuration did you use where the path is null?

YahnisElsts avatar Aug 06 '22 13:08 YahnisElsts

So far, I've seen it occurring on a development server and on localhost. On other sites, it doesn't happen. However, a safeguard should be easy to implement.

daigo75 avatar Aug 06 '22 13:08 daigo75

In general, implementing a workaround without understanding what caused the problem can lead to more bugs in the future.

As far as I know, parse_url() should only return null if the component doesn't exist. Do you really have an update metadata URL that doesn't include a path, not even a /? Is it a valid URL?

YahnisElsts avatar Aug 06 '22 13:08 YahnisElsts

I don't have an update URL without a path. The URL and the path are always the same in all cases. Anyway, not a big deal, I will see what's the difference between the sites that work and the one that raises the notice.

daigo75 avatar Aug 06 '22 13:08 daigo75

In that case, it seems that $path should never be null, no matter what the PHP version is (as long as it's >= 5.2).

If you can find a code sample that reliably triggers this notice, that would be useful.

YahnisElsts avatar Aug 06 '22 13:08 YahnisElsts

Actually, the issue is simple to reproduce:

// Use a URL without a trailing slash. It's a valid URL, anyway, as the trailing slash is optional
$url= 'https://example.org';
// The following could be a URL with some arguments. No trailing slash, still a valid URL
$url= 'https://example.org?some_arg=123';
// In this case, $path will be null
$path = parse_url($metadataUrl, PHP_URL_PATH);

// A URL with a trailing slash doesn't cause the issue
$url= 'https://example.org/';
// In this case, $path will be "/"
$path = parse_url($metadataUrl, PHP_URL_PATH);

Possible fixes

  • Typecast the path as a string.
  • Replace an empty path with "/".

I think the first option would be the least "invasive", so to speak.

daigo75 avatar Aug 06 '22 14:08 daigo75

That is all true, and I'll add a string cast for unusual situations like that.

However, that only applies if you're using a metadata URL that does not have a path or a trailing slash after the domain name. If your URL does include a path, and you're still getting that deprecation notice, that means there must be something else going on. Fixing the problem for URLs that don't have a path may or may not do anything to solve whatever the root cause is in your case.

YahnisElsts avatar Aug 06 '22 14:08 YahnisElsts

That's precisely what I'm checking now. So far, the two sites where the issue occurs have the URL without the trailing slash. I'm checking the other ones, to see if, by any chance, they have the slash. That could be the simplest explanation to why the notice appears only in some cases.

daigo75 avatar Aug 06 '22 14:08 daigo75