Problem/Motivation
Coming out of https://www.drupal.org/project/search_api/issues/3253738 there's another str_replace being thrown:
Deprecated function: str_replace(): Passing null to parameter #3 ($subject) of type array|string is deprecated in Drupal\search_api\Plugin\search_api\processor\Highlight->createExcerpt() (line 476 of modules/contrib/search_api/src/Plugin/search_api/processor/Highlight.php).
Drupal\search_api\Plugin\search_api\processor\Highlight->createExcerpt(NULL, Array) (Line: 314)
Drupal\search_api\Plugin\search_api\processor\Highlight->addExcerpts(Array, Array, Array) (Line: 268)
Drupal\search_api\Plugin\search_api\processor\Highlight->postprocessSearchResults(Object) (Line: 675)
This happens because preg_replace, which runs right before this call to str_replace can return NULL if there's an error.
Steps to reproduce
In my case, I had a field with some malformed html -- a <style> tag that never closed, like this:
<style>.background { color: red; }<p>Hello</p>
Proposed resolution
Maybe something like
$text = strip_tags(str_replace(['<', '>'], [' <', '> '], $text ?? ''));
Remaining tasks
Provide patch.
Comments
Comment #3
vishwenderbaloria commentedAdding patch to solve the issue. Please review.
Comment #4
mariacha1 commentedYep, looks great!
Comment #7
drunken monkeyThanks for reporting this issue, and thanks for posting a patch for fixing it.
However, since Drupal CI isn’t working anymore, please use MRs instead of patches for proposing changes. I now created an MR with the changes from your patch.
However, instead of just adding
?? ''I think we’d better explicitly check for an error condition and returnNULLin that case.As an alternative, we could also just use the original
$textin this case, skipping the stripping of<style>/<script>element contents – since the tags themselves will still be stripped, there should be no security problem, it might just lead to weird excerpts that contain highlighted Javascript or CSS – though only in the rare cases where thepreg_replace()fails. (E.g., when a visitor searches for “function” they might get an excerpt with something like “… eslint-disable-next-line func-names, prefer-arrow-callback, no-shadow function (preloadFontPaths) { const basePath = drupalSettings.path.baseUrl; …”.)What would you say to that suggestion? Otherwise, those items would end up with an empty excerpt, and I think avoiding those might be worth risking unhelpful/confusing excerpts in rare cases.