Contrib.social

Runtime PHP TypeError when comparing broken revisions

Open on Drupal.org →
Created on 17 October 2023, almost 2 years ago

Problem/Motivation

When comparing revisions that have unexpectedly missing data, PHP 8.0 and above will return a TypeError:
Argument #1 ($value) must be of type Countable|array, int given in diff_process_state_lines() (line 521 of /code/profiles/openberkeley/modules/contrib/diff/diff.pages.inc).

Steps to reproduce

Unfortunately it's difficult to reproduce the issue; it involves a combination of the Panels IPE and Workbench Moderation. The Panels IPE allows unchecking the Create Revision checkbox when customizing a page. In some cases, unchecking this box when saving a draft of a moderated node results in a broken revision. When you compare revisions, any comparison that encompasses the broken revision will return the TypeError.

Proposed resolution

Here's the function in question:

/**
 * Helper function to get line counts per field.
 */
function diff_process_state_lines(&$diff, $key) {
  foreach ($diff['#states'] as $state => $data) {
    if (isset($data['#old'])) {
      if (is_string($data['#old'])) {
        $diff['#states'][$state]['#old'] = explode("\n", $data['#old']);
      }
      $diff['#states'][$state]['#count_old'] = count($diff['#states'][$state]['#old']);
    }
    else {
      $diff['#states'][$state]['#count_old'] = 0;
    }
    if (isset($data['#new'])) {
      if (is_string($data['#new'])) {
        $diff['#states'][$state]['#new'] = explode("\n", $data['#new']);
      }
      $diff['#states'][$state]['#count_new'] = count($diff['#states'][$state]['#new']);
    }
    else {
      $diff['#states'][$state]['#count_new'] = 0;
    }
  }
}

This function assumes that if the incoming data isn't a string, it's an array. In the broken case, it's neither, it's an integer. We can add a conditional to check whether the non-string is an array. However, in versions of PHP prior to 8.0, in some cases count() would return 1 instead of 0 when trying to count a non-array/non-string, so behavior might change.

It's tempting to just cast the non-array to a string - this would work for booleans, integers, and floats, but not for objects that don't implement toString.

Remaining tasks

Patch and test.

User interface changes

None.

API changes

None.

Data model changes

None.

🐛 Bug report
Status

Active

Version

3.0

Component

Code

Created by

🇺🇸United States cboyden

Live updates comments and jobs are added and updated live.
  • PHP 8.0

    The issue particularly affects sites running on PHP version 8.0.0 or later.

Sign in to follow issues

Comments & Activities

  • Issue created by @cboyden
  • Issue was unassigned.
  • Status changed to Needs review almost 2 years ago
  • Open on Drupal.org →
    Core: 7.x + Environment: PHP 5.3 & MySQL 5.5
    last update almost 2 years ago
    Waiting for branch to pass
  • Comment almost 2 years ago →
    🇺🇸United States cboyden

    Here's a patch that works in my testing. The broken revision comparison no longer causes a type error. It seems pretty unlikely that the data passed to the function will be an object, but if it's important to consider that case, I can update the patch to account for that.

  • Comment almost 2 years ago →
    🇺🇸United States cboyden
  • Status changed to RTBC almost 2 years ago
  • Comment almost 2 years ago →
    🇺🇸United States dsnopek USA

    This makes sense to me! While this may lead to slight change in behavior with weird inputs, it seems to me that the outputs it would generate are a little bit more sensible than previously, so I think this is OK.

  • Status changed to Needs work about 1 year ago
  • Comment about 1 year ago →
    🇦🇺Australia acbramley

    Thank you for your contribution. This issue currently does not meet the Contribution guidelines which are required to get this change committed.

  • Status changed to Closed: outdated 8 months ago
  • Comment 8 months ago →
    heddn Nicaragua

    Given the proximity of EOL for Drupal 7, marking this as won't fix. The likely final release of diff has landed in https://www.drupal.org/project/diff/releases/7.x-3.5 →

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024