[3.10] [PHP 8.1] Fixes Router/Route.php deprecated null to string in strpos()

[beat]

Pull Request for Issue # none

Summary of Changes

Fixes Deprecated: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in libraries/src/Router/Route.php on line 124

Testing Instructions

Source-code review should be enough.

Otherwise on PHP 8.1 with all errors on and Joomla Debug ON, go to frontend in registration page (of CB if needed)

Actual result BEFORE applying this Pull Request

Deprecated: strpos(): Passing null to parameter #1 ($haystack) of type string is deprecated in libraries/src/Router/Route.php on line 124

Expected result AFTER applying this Pull Request

Warning disappears.

Documentation Changes Required

None.

[alikon]

I have tested this item ✅ successfully on 121e5cb

---

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/36798.}

[zero-24]

Do you have a stack trace so we know where the original issue is comming from?

[beat]

Do you have a stack trace so we know where the original issue is comming from?

Hi @zero-24,
Sure! Here it is:

[zero-24]

Hmm can you run a var_dump($items); here: https://github.com/joomla/joomla-cms/blob/3.10-dev/modules/mod_breadcrumbs/helper.php#L45

From my understanding the issue looks like that for some reason the ->link is not set right?

[beat]

Hmm can you run a var_dump($items); here: https://github.com/joomla/joomla-cms/blob/3.10-dev/modules/mod_breadcrumbs/helper.php#L45

Here you go:

modules/mod_breadcrumbs/helper.php:46:
array (size=2)
  0 => 
    object(stdClass)[525]
      public 'name' => string 'CB Profile' (length=10)
      public 'link' => string 'index.php?option=com_comprofiler&view=userprofile&Itemid=486' (length=60)
  1 => 
    object(stdClass)[499]
      public 'name' => string 'Sign up' (length=7)
      public 'link' => null

From my understanding the issue looks like that for some reason the ->link is not set right?

Looks like that's correct. But must the last item in a breadcrumb even have a link ?

[zero-24]

Looks like that's correct. But must the last item in a breadcrumb even have a link ?

Interesting question I would say yes as it should be the current page. If anything it should also not be "null" as thats not a valid link in the first place. Looks like we have a option to show or hide the last item. And there we never display the URL even when its set: https://github.com/joomla/joomla-cms/blob/3.10-dev/modules/mod_breadcrumbs/tmpl/default.php#L63-L71 but when speaking abstract about the pathway I would say it makes sense to have the URL of all items of the pathway right?

We could fix it like

$crumbs[$i]->link = !is_null($items[$i]->link) ? JRoute::_($items[$i]->link) : '';

That would do the trick here but generally speaking i would say we should try to find the reason why the link is null in the first place.

[beat]

We could fix it like

$crumbs[$i]->link = !is_null($items[$i]->link) ? JRoute::_($items[$i]->link) : '';

That would do the trick here but generally speaking i would say we should try to find the reason why the link is null in the first place.

Agree. But in 3.10 maintenance mode we also want to be as B/C as possible, and I like your suggested fix. Adapted this PR for it (and tested your suggestion before committing it).

[zero-24]

Merging thanks 👍

[brianteeman]

Looks like that's correct. But must the last item in a breadcrumb even have a link ?

That is an option in the module so please be careful changing this!!!

[zero-24]

Looks like that's correct. But must the last item in a breadcrumb even have a link ?

That is an option in the module so please be careful changing this!!!

Yes I saw that but as posted above we never add the link to the breadcrumb only the text if anything. Do i miss anything here?

Also we have not changed anything yet we only not process an link when it comes as "null".

[beat]

Looks like that's correct. But must the last item in a breadcrumb even have a link ?

That is an option in the module so please be careful changing this!!!

Thank you for sharing your concern. All is ok, no worries:

Nothing changed. The behavior stays exactly same as before, in the functions as well as in the usage.

In a nutshell, the only difference is that PHP <8.1 was accepting NULL as a string parameter in its strpos() function and considering it as an empty string, and now it generates a deprecation warning in that case. So the fix is to return the same result as before in case of NULL (which was an empty string for the function that was calling the function that used strpos() with null).