Mobile-Detect icon indicating copy to clipboard operation
Mobile-Detect copied to clipboard

Fix deprecation error in PHP 8.1

Open jdanino opened this issue 2 years ago • 6 comments

Fix errors in php8.1 : preg_match(): Passing null to parameter #2 ($subject) of type string is deprecated in

jdanino avatar Nov 29 '21 11:11 jdanino

This isn't correct, when $userAgent is empty, it may use $this->userAgent instead.

A more correct fix (note the $subject &&):

--- a/Mobile_Detect.php	2021-11-26 20:24:58.942859134 +0000
+++ b/Mobile_Detect.php	2021-11-26 20:25:08.058923323 +0000
@@ -1457,7 +1457,8 @@
      */
     public function match($regex, $userAgent = null)
     {
-        $match = (bool) preg_match(sprintf('#%s#is', $regex), (false === empty($userAgent) ? $userAgent : $this->userAgent), $matches);
+        $subject = (false === empty($userAgent) ? $userAgent : $this->userAgent);
+        $match = $subject && (bool) preg_match(sprintf('#%s#is', $regex), $subject, $matches);
         // If positive match is found, store the results for debug.
         if ($match) {
             $this->matchingRegex = $regex;

francislavoie avatar Nov 29 '21 21:11 francislavoie

false === empty(...) ? ...

Is this necessary? empty already returns a bool itself. Just use !empty or empty itself for the condition, is way more readable?

DrLightman avatar Nov 30 '21 09:11 DrLightman

Readability is subjective. To some, false === is more readable than ! because it's easier to notice the negation (I'm of the space before ! camp myself). But either way, I'd rather retain the original "style" than change it. I don't want to make any assumptions about what the maintainer will accept, so I prefer to keep it the same.

francislavoie avatar Nov 30 '21 12:11 francislavoie

Either way, we need to remove the deprecation warnings thrown, as they make currently a lot of noise in test suites.

dereuromark avatar Dec 01 '21 22:12 dereuromark

There is a second pull request started with issue #876 with an easy fix (just return false in case of empty UA string). This solution works fine and all warnings and deprecated message are gone (with the addition of my comment in the pull request.)

darapa1 avatar Jan 09 '22 09:01 darapa1

thanks @darapa1. For an unknown reason my PR was not target to the main project but on my fork 😑. I've just open the PR to be merged in this project. Check PR #879

julienbornstein avatar Jan 09 '22 11:01 julienbornstein