Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
On PHP 8.1, Views is showing a runtime error caused by changes in allowed function arguments.
- Deprecated function: strpos(): Passing null to parameter #1 ($string) of type string is deprecated in views_plugin_style->tokenize_value() (line 156 of /plugins/views_plugin_style.inc).
Steps to reproduce
This is happening in multiple places on a Panopoly child distribution; not sure what the exact steps are.
Proposed resolution
Update to use an empty string instead of leaving the variable undefined, or check the value before calling the strpos function.
Remaining tasks
Patch and test.
User interface changes
None.
API changes
None.
Data model changes
None.
Comment | File | Size | Author |
---|---|---|---|
#18 | views-n3336764-18.patch | 5.49 KB | joseph.olstad |
#18 | views-n3336764-18-interdiff_13_to_18.txt | 682 bytes | joseph.olstad |
Comments
Comment #2
DamienMcKennaAre you sure you've updated to the latest codebase? sanitize_value() is already protected against that sort of error, it runs !is_null() before passing the $value to strlen().
Comment #3
cboyden CreditAttribution: cboyden at UC Berkeley Web Platform Services commentedMy apologies, two of the errors are fixed. I've updated the issue summary. I am still seeing this code in the latest dev version of plugins/views_plugin_style.inc starting on line 151:
which is where the error
Deprecated function: strpos(): Passing null to parameter #1 ($string) of type string is deprecated in views_plugin_style->tokenize_value() (line 156 of /plugins/views_plugin_style.inc)
might be coming from, unless $value is checked before calling the function?Comment #4
DamienMcKennaYeah, well spotted.
It'd be worth prefixing every call to strpos() or stripos() with a quick !is_null(), because evidently we missed some, or some snook back in after the last fix.
Comment #5
cboyden CreditAttribution: cboyden at UC Berkeley Web Platform Services commentedI haven't dived in to see how many of these instances represent places where the first argument to strpos could be null at runtime, but there are a bunch. I excluded ones found in tests for now.
Comment #6
joseph.olstadComment #7
joseph.olstadPassing PHP 8.2 tests, please review.
Comment #8
DamienMcKennaI don't like some of the logic used, so how about this? Hopefully I didn't bork anything.
Comment #10
DamienMcKennaLet's see if this fixes the mistake in #8.
Comment #11
DamienMcKennaComment #12
joseph.olstad@DamianMcKenna, patch 6 passes 100% of PHP 8.2 tests, patch 10 has 1 failure on PHP 8.2
hmm, I'll have another look
Comment #13
joseph.olstadComment #14
joseph.olstadI'm expecting patch 13 to pass PHP 8.2 tests
Comment #15
joseph.olstad@DamienMcKenna, I took some of your suggested changes, but not the ones that I suspected to change the logic flow. Patch 13 passes PHP 8.2 however patch 10 and patch 8 do not.
I suggest we move forward with patch 13.
PHP 8.2 is green btw, this should be considered a win for us!
Comment #16
joseph.olstadComment #17
DamienMcKennaAgreed - I obviously fumbled something.
The only additional tweak I would suggest is $class = $this->options['row_class']; - that should verify $this->options['row_class'] exists before using it.
Comment #18
joseph.olstadOk try this patch and interdiff.
Comment #19
joseph.olstadvery strange, with that said, I think it's safe for us to use patch 13 instead of patch 18. Previously there was never a need to do an isset on the array for the options.
Comment #20
DamienMcKennaThe tests pass for 8.2 but not for 8.1?!? What's even going on here?!?
Comment #21
joseph.olstadya very strange
Comment #22
joseph.olstadok I vote to go with Patch 18, we'll figure out the other issue later
at least we're getting a pass on PHP 8.2 and the other versions of PHP like 8.0, 7.4, 5.3
Comment #23
joseph.olstadI've re-queued PHP 8.2 on patch 13 and patch 18, php 8.1 tests ran a few times, same result.
Comment #24
joseph.olstadRTBC, deal with 8.1 later.
I'm using PHP 8.0 and PHP 8.2 currently so this is good for me
Comment #25
joseph.olstadthere must be a bug in PHP 8.1 it'self. PHP 8.1.16 was released today, perhaps drupal ci needs to upgrade.
https://www.php.net/ChangeLog-8.php#8.1.16
Comment #26
joseph.olstadIncluded in todays PHP 8.1.16 release
Core:
...
...
plus more
Comment #28
DamienMcKennaGood insights, hopefully drupalci will be upgraded soon and we'll have error-free test runs.
Committed. Thanks everyone!
Comment #29
joseph.olstadw00t! :) yay !!! :)
Another victory for PHP 8.2
Comment #30
joseph.olstad@DamianMcKenna, I'd say this one change warrants a tag and release? 7.x-3.29 ??
I was looking for a release but I guess I can switch to dev for now.