Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP 8.1 compatibility - Deprecation notice #499

Open
daigo75 opened this issue Aug 6, 2022 · 8 comments
Open

PHP 8.1 compatibility - Deprecation notice #499

daigo75 opened this issue Aug 6, 2022 · 8 comments

Comments

@daigo75
Copy link

daigo75 commented Aug 6, 2022

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 daigo75 changed the title PHP 8.1 compatibility PHP 8.1 compatibility - Deprecation notice Aug 6, 2022
@YahnisElsts
Copy link
Owner

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?

@daigo75
Copy link
Author

daigo75 commented Aug 6, 2022

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.

@YahnisElsts
Copy link
Owner

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?

@daigo75
Copy link
Author

daigo75 commented Aug 6, 2022

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.

@YahnisElsts
Copy link
Owner

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.

@daigo75
Copy link
Author

daigo75 commented Aug 6, 2022

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.

@YahnisElsts
Copy link
Owner

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.

@daigo75
Copy link
Author

daigo75 commented Aug 6, 2022

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 added a commit that referenced this issue Aug 6, 2022
In the (unlikely) case where the update metadata URL does not include a path, the getVcsService() method could previously trigger a deprecation notice like "preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated". This is because parse_url() returns NULL when the specified component is not present.

Fixed by always casting the $path to string. The VCS detection code doesn't care about the difference between "empty path" and "no path" - it should correctly return NULL (= no VCS host found) anyway.

Also, let's cast $host to a string as well to avoid another potential notice if the URL somehow has a path but no domain name.

Initially reported in #499.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants