Cannot access protected property SelectQuery::$alterTags in EntityReference_SelectionHandler_Generic->reAlterQuery()

Linking the related D7 core issue https://www.drupal.org/node/3304886

As a workaround, you could edit the core select.inc file to make the two properties public.

Thanks for reporting this.

I have briefly checked the problematic code in the entityreference, but I think I am missing some information here. The code utilizing those variables was added in #1261856: Implement special handling for some entity type (and factor out the main business logic), as a workaround for the D7 core bugs.

/**
 * Override for the Taxonomy term type.
 *
 * This only exists to workaround core bugs.
 */

I assume, that if there are/were core bugs causing the need of this workaround, these bugs should be mentioned in some issues in the core issue queue (or ideally in the comment of that function). I was unable to find such issues yet - can someone reference these core bugs here?

Also I do not fully understand the intent here:

  // The Taxonomy module doesn't implement any proper taxonomy term access,
  // and as a consequence doesn't make sure that taxonomy terms cannot be viewed
  // when the user doesn't have access to the vocabulary.
  $base_table = $this->ensureBaseTable($query);
  $vocabulary_alias = $query->innerJoin('taxonomy_vocabulary', 'n', '%alias.vid = ' . $base_table . '.vid');
  $query->addMetadata('base_table', $vocabulary_alias);
  // Pass the query to the taxonomy access control.
  $this->reAlterQuery($query, 'taxonomy_vocabulary_access', $vocabulary_alias);

Taxonomy terms are publicly accessible in D7 core by default, without any default access control present (see: #3159905: Taxonomy module should implement hook_query_TAG_alter for taxonomy_access tag). The same applies, if I am not mistaken, to the taxonomy vocabularies view access as well. Therefore I am not sure who is acting on the tag taxonomy_vocabulary_access. It seems to me that this code is extending the core functionality instead of fixing bugs in it (but I may be mistaken).

Can someone with the knowledge of the entityreference module explain this a bit? Thanks!

hi @poker10, thanks for stepping in.

For the record, the aforementioned change in core was a breaking change, as properties visibility has changed from public to protected. Thus the issue.

However, I think the issue is that Drupal core is missing public methods that allows to get/set all tags/metadata.

D10 made it public:

  /**
   * The query metadata for alter purposes.
   */
  public array $alterMetaData;

  /**
   * The query tags.
   */
  public array $alterTags;

see https://git.drupalcode.org/project/drupal/-/blob/10.0.x/core/lib/Drupal/...

Created a core issue #3326249: Make alterTags and alterMetadata public for Select query

@Taran2L Thanks for your findings!

Yes, I understand that and don't dispute that the visibility was changed (although not intentionally), I was just curious about the intent of the entityreference module code adding additional access checks on top of the what D7 core provides.. That seems to be the main reason the problematic code is there and according to the comments, it is a hacky solution to fix D7 core issues (especially the part with taxonomy terms access checking is quite interesting for me). But I wrote this to better understand the whole situation. Now it is not so important (since in D7 core we should stick to what is in D10), albeit I think that the main reason for D10 to change this to public was that it absorbed the entityreference module and it was needed, because of missing getters and setters - otherwise I think they would have changed the visibility to protected, as we have the other properties.

While this doesn't fix the core issue, this code change fixed our issue:
$query->alterMetaData['options']
to
$query->getMetaData('options')

I searched the code for $query->alterMetaData['options'] and didn't find anything. Where did you make that change?

I did this to avoid changing the core.

protected function reAlterQuery(SelectQueryInterface $query, $tag, $base_table) {
     // Save the old tags and metadata.
     // For some reason, those are public.
     if(isset($query->alterTags)) {
       $old_tags = $query->alterTags;
       $old_metadata = $query->alterMetaData;
       $query->alterTags = array($tag => TRUE);
       $query->alterMetaData['base_table'] = $base_table;
       drupal_alter(array('query', 'query_' . $tag), $query);
 
       // Restore the tags and metadata.
       $query->alterTags = $old_tags;
       $query->alterMetaData = $old_metadata;
     }
   }

I tried to produce an empty or NULL value for $old_tags and $old_metadata but it does not help.
Just to mention, I am using the tac_lite very old module to manage permission by taxonomy terms and everything seems to be ok after this quite dirty patch.
Relevant?

I'm usually pretty hesitant to roll out core patches. https://www.drupal.org/project/drupal/issues/3326249 seems just fine, but I'm not keen on patching a large number of D7 sites or swapping them to dev D7. The conditional in #15 seems to do the trick and not cause any harm. Hoping it's a good fix until core can be sorted. Adding a patch using the changes suggested in #15.

Looks like the core issue was fixed in the latest release, #3326249: Make alterTags and alterMetadata public for Select query. So maybe this patch is unnecessary when using 7.94 and up?

Yes, I think that's the case. I don't need the patch, because I modified core according to what they changed (prior to 7.94) to make them public.

If it's alright with everyone here, I'm going to closed this as outdated, since upgrading to Drupal 7.94 should fix it.

Indeed, no problem so far, with new 7.94 and patch removed.