Contrib.social

features_get_info() poisons original objects from system_rebuild_module_data()

Open on Drupal.org →
Created on 10 August 2019, over 5 years ago
Updated 6 November 2024, 4 months ago

I open this as a "Bug report", even though I cannot currently present any symptoms.

Background

features_get_info() gets info about available modules from system_rebuild_module_data(), delivered as \stdClass objects.
In addition, features_get_info() does:

  • Modify the objects slightly:
    - unset $row->info['mtime']
    - unset $row->info['php']
    - modify $row->info['stylesheets']
    - modify $row->info['scripts']
    - modify $row->info['features']
    - set $row->components
  • Cache the objects (so that only on cache reset the core function is called).
  • Provide a filtered list that only contains the modules with non-empty $row->info['features'].

Problem

In the reset/rebuild case, the modifications mentioned above are applied to the original objects from system_rebuild_module_data().

This can be reproduced in devel/php like so:

dpm(system_rebuild_module_data()['features_test']);
features_get_info('feature', NULL, TRUE);
dpm(system_rebuild_module_data()['features_test']);

Before the features_get_info(), the ->components is not present.
After the features_get_info(), the ->components is present.

Solution

Clone the objects.

Related issue

#1070912: features_get_info() hands back original objects, can get corrupted → is about calling code that might poison the original objects from features_get_info().
A solution was committed, but only for the case of a single module returned.
If a list of modules is returned, they are the original objects.

This can still be a problem, but out of scope for this issue.

Normally I would say the API code should be responsible for preventing calling code from modifying data, e.g. by making objects immutable.
However, in Drupal 7, mutable \stdClass objects are the norm, and most API functions return not-cloned objects.
I would say in Drupal 7 the calling code is responsible for not poisoning the original objects.
This means features_get_info() should clone the original objects when fetching them from system_rebuild_module_data(), but it is not required to clone the objects again when returning them to calling code.

🐛 Bug report
Status

Active

Version

2.0

Component

Code

Created by

🇩🇪Germany donquixote

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 4 months ago →
    🇺🇸United States csmdgl

    This is not a new bug and has long existed without causing problems for us. It has become apparent by moving to PHP 8.

    When enabling a module via drush, this warning is produced:
    Undefined array key "php" environment.inc:790 [warning]

    By itself, we were tolerating and ignoring the bug. But when enabling a module via the UI, this produces thirteen pages of warnings of these two types:
    Warning: Undefined array key "php" in _system_modules_build_row() (line 1027 of /var/www/html/docroot/modules/system/system.admin.inc).
    and
    Deprecated function: version_compare(): Passing null to parameter #2 ($version2) of type string is deprecated in _system_modules_build_row() (line 1027 of /var/www/awards.practicegreenhealth.org/pgh-awards/docroot/modules/system/system.admin.inc).

    Features is corrupting the system module cache with this line in features_get_info():
    unset($row->info['php']);

    Patch attached.

  • Comment 4 months ago →
    🇺🇸United States csmdgl
  • Status changed to RTBC 4 days ago
  • Comment 4 days ago →
    🇳🇴Norway steinmb

    For what it's worth, confirm that this two liner addresses the problem.

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024