Closed (fixed)
Project:
Views (for Drupal 7)
Version:
7.x-3.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
26 Jan 2023 at 22:23 UTC
Updated:
1 Mar 2023 at 01:39 UTC
Jump to comment: Most recent, Most recent file
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 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 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.