PHP 8.1 compatibility - Deprecation notice #499

Open daigo75 opened 2 years ago

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.

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?

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.

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?

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.

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.

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.

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.

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.

YahnisElsts / plugin-update-checker

PHP 8.1 compatibility - Deprecation notice #499