Contrib.social

Deprecated function: str_replace(): Passing null to parameter #3 in Highlight->createExcerpt() - php 8.1+

Open on Drupal.org →
Created on 16 July 2024, about 2 months ago
Updated 20 July 2024, about 2 months ago

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.

🐛 Bug report
Status

Needs review

Version

1.0

Component

Plugins

Created by

🇺🇸United States mariacha1

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mariacha1
  • First commit to issue fork.
  • Comment about 2 months ago →
    vishwenderbaloria

    Adding patch to solve the issue. Please review.

  • Status changed to RTBC about 2 months ago
  • Comment about 2 months ago →
    🇺🇸United States mariacha1

    Yep, looks great!

  • Comment about 2 months ago →
    🇦🇹Austria drunken monkey Vienna, Austria

    drunken monkey → made their first commit to this issue’s fork.

  • Merge request !156Resolve #3461936: Fix str_replace() called with NULL → (Open) created by drunken monkey
  • Status changed to Needs review about 2 months ago
  • Comment about 2 months ago →
    🇦🇹Austria drunken monkey Vienna, Austria

    Thanks 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 return NULL in that case.
    As an alternative, we could also just use the original $text in 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 the preg_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.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024