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.

CommentFileSizeAuthor
#18 views-n3336764-18.patch5.49 KBjoseph.olstad
#18 views-n3336764-18-interdiff_13_to_18.txt682 bytesjoseph.olstad
#13 views-n3336764-13-interdiff_06_to_13.txt1.71 KBjoseph.olstad
#13 views-n3336764-13.patch5.42 KBjoseph.olstad
#10 views-n3336764-10.patch6.1 KBDamienMcKenna
#10 views-n3336764-10.interdiff.txt712 bytesDamienMcKenna
#8 views-n3336764-8.patch6.17 KBDamienMcKenna
#8 views-n3336764-8.interdiff.txt3.76 KBDamienMcKenna
#6 3332486-06.patch5.19 KBjoseph.olstad
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cboyden created an issue. See original summary.

DamienMcKenna’s picture

Are 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().

cboyden’s picture

Title: PHP 8.x deprecated function warnings at runtime » PHP 8.x deprecated function warning at runtime
Issue summary: View changes

My 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:

  /**
   * Take a value and apply token replacement logic to it.
   */
  public function tokenize_value($value, $row_index) {
    if (strpos($value, '[') !== FALSE || strpos($value, '!') !== FALSE || strpos($value, '%') !== FALSE) {
      $fake_item = array(
        'alter_text' => TRUE,
        'text' => $value,
      );

      // Row tokens might be empty, for example for node row style.
      $tokens = isset($this->row_tokens[$row_index]) ? $this->row_tokens[$row_index] : array();
      if (!empty($this->view->build_info['substitutions'])) {
        $tokens += $this->view->build_info['substitutions'];
      }

      if ($tokens) {
        $value = strtr($value, $tokens);
      }
    }

    return $value;
  }

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?

DamienMcKenna’s picture

Yeah, 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.

cboyden’s picture

I 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.

$ grep -B 2 -r -e 'strpos' .

./views_ui.module-        $one_path = $display->handler->get_option('path');
./views_ui.module-
./views_ui.module:        if (empty($view->disabled) && strpos($one_path, '%') === FALSE) {
--
./plugins/views_wizard/views_ui_base_views_wizard.class.php-      else {
./plugins/views_wizard/views_ui_base_views_wizard.class.php-        foreach ($fields as $field_name => $value) {
./plugins/views_wizard/views_ui_base_views_wizard.class.php:          if ($pos = strpos($field_name, '.' . $bundle_key)) {
--
./plugins/views_plugin_style_jump_menu.inc-        // so users don't shoot themselves in the foot.
./plugins/views_plugin_style_jump_menu.inc-        $base_path = base_path();
./plugins/views_plugin_style_jump_menu.inc:        if (strpos($path, $base_path) === 0) {
--
./plugins/views_plugin_style.inc-    if ($this->uses_row_class()) {
./plugins/views_plugin_style.inc-      $class = $this->options['row_class'];
./plugins/views_plugin_style.inc:      if (strpos($class, '[') !== FALSE || strpos($class, '!') !== FALSE || strpos($class, '%') !== FALSE) {
--
./plugins/views_plugin_style.inc-   */
./plugins/views_plugin_style.inc-  public function tokenize_value($value, $row_index) {
./plugins/views_plugin_style.inc:    if (strpos($value, '[') !== FALSE || strpos($value, '!') !== FALSE || strpos($value, '%') !== FALSE) {
--
./plugins/views_plugin_pager_full.inc-    $error = FALSE;
./plugins/views_plugin_pager_full.inc-    $exposed_options = $form_state['values']['pager_options']['expose']['items_per_page_options'];
./plugins/views_plugin_pager_full.inc:    if (strpos($exposed_options, '.') !== FALSE) {
--
./plugins/views_plugin_display_page.inc-    switch ($form_state['section']) {
./plugins/views_plugin_display_page.inc-      case 'path':
./plugins/views_plugin_display_page.inc:        if (strpos($form_state['values']['path'], '$arg') !== FALSE) {
--
./plugins/views_plugin_display_page.inc-        }
./plugins/views_plugin_display_page.inc-
./plugins/views_plugin_display_page.inc:        if (strpos($form_state['values']['path'], '%') === 0) {
--
./plugins/views_plugin_display_page.inc-      case 'menu':
./plugins/views_plugin_display_page.inc-        $path = $this->get_option('path');
./plugins/views_plugin_display_page.inc:        if ($form_state['values']['menu']['type'] == 'normal' && strpos($path, '%') !== FALSE) {
--
./plugins/export_ui/views_ui.class.php-      else {
./plugins/export_ui/views_ui.class.php-        // Check whether the tag can be found in the views tag.
./plugins/export_ui/views_ui.class.php:        return strpos($view->tag, $form_state['values']['tag']) === FALSE;
--
./includes/ajax.inc-    $prefix = count($array) ? implode(', ', $array) . ', ' : '';
./includes/ajax.inc-
./includes/ajax.inc:    if (strpos('anonymous', strtolower($last_string)) !== FALSE) {
--
./includes/ajax.inc-      $n = $account;
./includes/ajax.inc-      // Commas and quotes in terms are special cases, so encode 'em.
./includes/ajax.inc:      if (strpos($account, ',') !== FALSE || strpos($account, '"') !== FALSE) {
--
./includes/ajax.inc-      $n = $name;
./includes/ajax.inc-      // Term names containing commas or quotes must be wrapped in quotes.
./includes/ajax.inc:      if (strpos($name, ',') !== FALSE || strpos($name, '"') !== FALSE) {
--
./includes/admin.inc-    if ($display->handler->has_path()) {
./includes/admin.inc-      $one_path = $display->handler->get_option('path');
./includes/admin.inc:      if (strpos($one_path, '%') === FALSE) {
--
./includes/admin.inc-      elseif ($display->handler->has_path()) {
./includes/admin.inc-        $path = $display->handler->get_path();
./includes/admin.inc:        if (strpos($path, '%') === FALSE) {
--
./includes/admin.inc-      list($table, $field) = explode('.', $field, 2);
./includes/admin.inc-
./includes/admin.inc:      if ($cut = strpos($field, '$')) {
--
./includes/admin.inc-  $views = views_get_all_views();
./includes/admin.inc-  foreach ($views as $view) {
./includes/admin.inc:    if (!empty($view->tag) && strpos($view->tag, $string) === 0) {
--
./includes/view.inc-        if ($this->display_handler->uses_breadcrumb() && $argument->uses_breadcrumb()) {
./includes/view.inc-          $path = $this->get_url($breadcrumb_args);
./includes/view.inc:          if (strpos($path, '%') === FALSE) {
--
./includes/view.inc-    }
./includes/view.inc-    // Don't bother working if there's nothing to do.
./includes/view.inc:    if (empty($path) || (empty($args) && strpos($path, '%') === FALSE)) {
--
./handlers/views_handler_field_numeric.inc-    }
./handlers/views_handler_field_numeric.inc-    else {
./handlers/views_handler_field_numeric.inc:      $point_position = strpos($value, '.');
--
./handlers/views_handler_field.inc-   */
./handlers/views_handler_field.inc-  public function tokenize_value($value, $row_index = NULL) {
./handlers/views_handler_field.inc:    if (strpos($value, '[') !== FALSE || strpos($value, '!') !== FALSE || strpos($value, '%') !== FALSE) {
--
./handlers/views_handler_field.inc-        $base_path = base_path();
./handlers/views_handler_field.inc-        // Checks whether the path starts with the base_path.
./handlers/views_handler_field.inc:        if (strpos($more_link_path, $base_path) === 0) {
--
./help/alter-exposed-filter.html-<?php
./help/alter-exposed-filter.html-function MODULENAME_form_views_exposed_form_alter(&$form, $form_state) {
./help/alter-exposed-filter.html:  if (strpos($form['#id'], 'volunteer-directory') !== FALSE) {
--
./views.module- */
./views.module-function views_forms($form_id, $args) {
./views.module:  if (strpos($form_id, 'views_form_') === 0) {
--
./views.module-    // has the rightmost extension removed, but there might still be more,
./views.module-    // such as with .tpl.php, which still has .tpl in $template at this point.
./views.module:    if (($pos = strpos($template, '.')) !== FALSE) {
--
./views.module-      if ($matches) {
./views.module-        foreach ($matches as $match) {
./views.module:          $file = substr($match, 0, strpos($match, '.'));
--
./views.module-    $output = $var ? 'TRUE' : 'FALSE';
./views.module-  }
./views.module:  elseif (is_string($var) && strpos($var, "\n") !== FALSE) {
joseph.olstad’s picture

Status: Active » Needs review

Passing PHP 8.2 tests, please review.

DamienMcKenna’s picture

I don't like some of the logic used, so how about this? Hopefully I didn't bork anything.

Status: Needs review » Needs work

The last submitted patch, 8: views-n3336764-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

DamienMcKenna’s picture

DamienMcKenna’s picture

Status: Needs work » Needs review
joseph.olstad’s picture

@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

joseph.olstad’s picture

I'm expecting patch 13 to pass PHP 8.2 tests

joseph.olstad’s picture

@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!

joseph.olstad’s picture

Issue tags: +PHP 8.2
DamienMcKenna’s picture

Agreed - 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.

joseph.olstad’s picture

joseph.olstad’s picture

very 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.

DamienMcKenna’s picture

The tests pass for 8.2 but not for 8.1?!? What's even going on here?!?

joseph.olstad’s picture

ya very strange

joseph.olstad’s picture

ok 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

joseph.olstad’s picture

I've re-queued PHP 8.2 on patch 13 and patch 18, php 8.1 tests ran a few times, same result.

joseph.olstad’s picture

Status: Needs review » Reviewed & tested by the community

RTBC, deal with 8.1 later.

I'm using PHP 8.0 and PHP 8.2 currently so this is good for me

joseph.olstad’s picture

there 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

joseph.olstad’s picture

Included in todays PHP 8.1.16 release

Core:

...

  • Fixed bug GH-10072 (PHP crashes when execute_ex is overridden and a __call trampoline is used from internal code).
  • Fix GH-10251 (Assertion `(flag & (1<<3)) == 0' failed).
  • Fix wrong comparison in block optimisation pass after opcode update.

...
plus more

DamienMcKenna’s picture

Status: Reviewed & tested by the community » Fixed

Good insights, hopefully drupalci will be upgraded soon and we'll have error-free test runs.

Committed. Thanks everyone!

joseph.olstad’s picture

w00t! :) yay !!! :)

Another victory for PHP 8.2

joseph.olstad’s picture

@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.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.