Logic Error in Tables.php

Created on 7 April 2023, almost 2 years ago

Note

This is related to https://www.drupal.org/project/drupal/issues/2893747 πŸ› Call to a member function getColumns() on boolean Needs work . But, I think it is a different issue. I think that bug is asking "Why $field_storage is FALSE?" This bug is asking, "Why are we trying to call methods when we know $field_storage == FALSE?"

Problem/Motivation

I think I spotted a logic error in Tables.php. I'm working on a query. My query may be flawed. There may be a problem with addressfield. But that should result in an exception. I think there's a logic error in Tables.php.

Using line numbers from the 9.5.5 version of .../core/lib/Drupal/Core/Entity/Query/Sql/Tables.php:

  • $field_storage gets set on line 105 or 109
  • Basic conditional logic on line 134
  • Else condition on line 191
  • $field_storage is used like an object inside this else condition ie: even when we KNOW $field_storage == FALSE

Steps to reproduce

I'm running Drupal 9.5.5 with Address field module on PHP 8.1.14. I have a taxonomy (Cities) with addressfield field_address and text field field_allowed_zip_codes.

I tried to do an entity query and get this error:

Error: Call to a member function getColumns() on bool in Drupal\Core\Entity\Query\Sql\Tables->addField() (line 263 of /app/web/core/lib/Drupal/Core/Entity/Query/Sql/Tables.php)

#0 /app/web/core/lib/Drupal/Core/Entity/Query/Sql/Condition.php(58): Drupal\Core\Entity\Query\Sql\Tables->addField('field_address.a...', 'INNER', NULL)
#1 /app/web/core/lib/Drupal/Core/Entity/Query/Sql/Query.php(177): Drupal\Core\Entity\Query\Sql\Condition->compile(Object(Drupal\mysql\Driver\Database\mysql\Select))

The query:

      $storage = $this->entityTypeManager->getStorage('taxonomy_term');

      $query = $storage->getQuery()
        ->condition('vid', self::VOCABULARY_VID)
        ->condition('field_address.administrative_area', $input);

The error is partially tied to the address field. When I change the last line of the query to either of these, it works:

        // This works.
        // ->condition('field_allowed_zip_codes.value', $input, 'CONTAINS');

        // This works.
        // ->condition('field_allowed_zip_codes.%delta.value', $input, 'CONTAINS');

I don't know why conditions on an address field cause a problem for $field_storage. But, we should not be calling methods without validating that we have an object.

Proposed resolution

I think this needs more strict validation. There should probably be something like around line 129:

  if (!($field_storage instanceof FieldWhateverYouShouldBeInterface)) :
    // log an error or throw an exception.

    // exit current function.
  endif;

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

πŸ› Bug report
Status

Active

Version

9.5

Component
EntityΒ  β†’

Last updated about 11 hours ago

Created by

πŸ‡ΊπŸ‡ΈUnited States JasonSafro

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

Comments & Activities

Production build 0.71.5 2024