Problem/Motivation
When rendering ads with the ad_content module, a PHP deprecation notice is triggered on PHP 8.1+:
Deprecated function: str_replace(): Passing null to parameter #2 ($replace) of type array|string is deprecated in Drupal\ad_content\Plugin\Ad\Bucket\AdContentBucket::postAdRender() (line 252 of /modules/contrib/ad/modules/ad_content/src/Plugin/Ad/Bucket/AdContentBucket.php)
This happens when $build['#ad_impression_id'] is missing or null, which results in str_replace() receiving an invalid argument.
Steps to reproduce
- Install and enable the
adandad_contentmodules. - Create an ad bucket and render ads (for example via Mercury Editor or an ad placement).
- On PHP 8.1+ observe the deprecation warning in logs:
str_replace(): Passing null to parameter #2 ($replace)...
Proposed resolution
Update AdContentBucket::postAdRender() to safely handle nulls by casting to string and defaulting to an empty string when the #ad_impression_id is not set. Example fix:
$search = (string) TrackerInterface::PLACEHOLDER_IMPRESSION; $replace = isset($build['#ad_impression_id']) ? (string) $build['#ad_impression_id'] : ''; $subject = (string) $markup; $output = str_replace($search, $replace, $subject); return $markup instanceof \Drupal\Component\Render\MarkupInterface ? \Drupal\Component\Render\Markup::create($output) : $output;
Remaining tasks
- [ ] Attach patch with fix.
- [ ] Review by maintainers.
- [ ] Add test coverage to ensure
postAdRender()works without#ad_impression_id.
Note: I will attach a patch to this issue, as I cannot currently provide a Merge Request on GitLab for this project.
User interface changes
None.
API changes
None (only internal implementation hardened).
Data model changes
None.
Issue fork ad-3544099
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
anybody@dobe please use MRs not patches.
@LR: Could you please have a look?
Comment #5
lrwebks commentedComment #6
lrwebks commentedIn general, we should perhaps find the cause of why the impression ID was
nullin the first place. @dobe right now, our solutions are equally correct, but let me take a look at what is actually happening under the hood there.Comment #7
anybodyMR is okay but please first try to find out what causes this, would be better to fix the root cause!
Comment #8
lrwebks commentedFound the cause to actually be intended behavior! The impression ID used in the discussed function is returned by the active tracker once it has registered said impression. Though if there is no ad tracker, or the user has the permission “bypass ad tracking” then the impression will of course not be tracked and the function returns
NULL.This MR is still needed for proper handling of that value and avoiding the deprecated functionality, so we should merge this!
Sidenote: This bug very obviously happens once the module is used for the first time, because the default “NULL” tracker is still selected in settings, which returns
NULL(at least now, thanks to me fixing that too), because it simply isn't allowed to track anything. And for an admin, the impression ID is alwaysNULL, because they have the “bypass ad tracking” permission enabled by default.Comment #9
anybodyThanks!!
Comment #11
anybody