Logic Error in Tables.php

Created on 7 April 2023, about 1 year 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 7 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.69.0 2024