Problem/Motivation

WSOD, making Drupal site unusable.

Steps to reproduce

Update to latest -dev

Proposed resolution

Back to stable version to stable version, and the site is online again.

Remaining tasks

Haven't investigate what is the problem, maybe will try to do it later.

For now, here is the complete error and top10 stack:

TypeError: Drupal\field_permissions\FieldPermissionsService::fieldGetPermissionType(): Argument #1 ($field) must be of type Drupal\field\FieldStorageConfigInterface, Drupal\Core\Field\BaseFieldDefinition given, called in /(...)/modules/contrib/field_permissions/src/FieldPermissionsService.php on line 163 na Drupal\field_permissions\FieldPermissionsService->fieldGetPermissionType() (linha 138 de /(...)/modules/contrib/field_permissions/src/FieldPermissionsService.php)
#0 /(...)/modules/contrib/field_permissions/src/FieldPermissionsService.php(163): Drupal\field_permissions\FieldPermissionsService->fieldGetPermissionType()
#1 /(...)/modules/contrib/field_permissions/field_permissions.module(39): Drupal\field_permissions\FieldPermissionsService->getFieldAccess()
#2 /(...)/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php(350): field_permissions_entity_field_access()
#3 /(...)/core/lib/Drupal/Core/Extension/ModuleHandler.php(405): Drupal\Core\Entity\EntityAccessControlHandler->Drupal\Core\Entity\{closure}()
#4 /(...)/core/lib/Drupal/Core/Entity/EntityAccessControlHandler.php(351): Drupal\Core\Extension\ModuleHandler->invokeAllWith()
#5 /var/www/clients/client1/web1/web/repos/igot/web/core/lib/Drupal/Core/Field/FieldItemList.php(154): Drupal\Core\Entity\EntityAccessControlHandler->fieldAccess()
#6 /(...)/core/lib/Drupal/Core/Entity/Entity/EntityViewDisplay.php(253): Drupal\Core\Field\FieldItemList->access()
#7 /(...)/core/modules/layout_builder/src/Entity/LayoutBuilderEntityViewDisplay.php(266): Drupal\Core\Entity\Entity\EntityViewDisplay->buildMultiple()
#8 /(...)/core/lib/Drupal/Core/Entity/EntityViewBuilder.php(340): Drupal\layout_builder\Entity\LayoutBuilderEntityViewDisplay->buildMultiple()
#9 /(...)/core/modules/node/src/NodeViewBuilder.php(24): Drupal\Core\Entity\EntityViewBuilder->buildComponents()
#10 /(...)/core/lib/Drupal/Core/Entity/EntityViewBuilder.php(282): Drupal\node\NodeViewBuilder->buildComponents()
Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

jrochate created an issue. See original summary.

ayush.pandey’s picture

I have explored the issue, but the issue seems not to be reproducible on the field_permission module version 8.x-1.x-dev and PHP version 8.1.
Please add proper steps to reproduce incase it is reproducible at your end. jrochate

mariacha1’s picture

Agreed, I'm going to assume you've got a patch applied that's conflicting with the latest push to 8.x-1.x. I had this trouble related to a patch in https://www.drupal.org/node/2881776. I'll update that issue with an updated patch.

mariacha1’s picture

Status: Active » Needs review

Patch over at https://www.drupal.org/files/issues/2023-07-10/2881776-39.patch should fix it. If you're using the previous patch (32), you should be able to roll that one instead.

jrochate’s picture

Sorry, it didn't work.

We will try to find where is this coming from, and will keep you posted. thanks Maria

lolgm’s picture

It seems to me that the problem is being caused by the change implemented in issue #3371144: Invalid context for call to FieldDefinitionInterface->isDisplayConfigurable().

function field_permissions_entity_field_access($operation, FieldDefinitionInterface $field_definition, $account, FieldItemListInterface $items = NULL) {
  $context = ($operation == 'view') ? 'view' : 'edit';
  if (!$field_definition->isDisplayConfigurable($context) || empty($items)) {
    return AccessResult::neutral();
  }
  $access_field = \Drupal::service('field_permissions.permissions_service')->getFieldAccess($operation, $items, $account, $field_definition);
  ...
}

This alteration causes the unexpected behavior of allowing the entity's base fields to be validated by the getFieldAccess function, however this is not prepared to receive this type of fields, hence the error reported above.
My suggestion is to check if it is a base field of the entity. If it is, we should not process it with the module's permissions service.
Example below:

function field_permissions_entity_field_access($operation, FieldDefinitionInterface $field_definition, $account, FieldItemListInterface $items = NULL) {
  $context = ($operation == 'view') ? 'view' : 'edit';
  if (!$field_definition->isDisplayConfigurable($context) || empty($items) || $field_definition->getFieldStorageDefinition()->isBaseField()) {
    return AccessResult::neutral();
  }
  $access_field = \Drupal::service('field_permissions.permissions_service')->getFieldAccess($operation, $items, $account, $field_definition);
  ...
}

Or maybe the solution should be implemented at the root of the problem?
Drupal\field_permissions\FieldPermissionsService::fieldGetPermissionType()

I would like to know your opinions about this.

mariacha1’s picture

Ahh, good catch. More the fool me for reviewing on a site with my own bunch of patches applied.

I think this is a fine solution for the problem for now. I'd like base fields to be useable in the future, but that's a different problem.

The most fool-proof solution is to just check to see if $field_definition implements \Drupal\Core\Field\FieldConfigInterface, since that's what fieldGetPermissionType expects.

So this:

  if (!$field_definition->isDisplayConfigurable($context) || empty($items) || !is_a($field_definition, '\Drupal\Core\Field\FieldConfigInterface')) {
    return AccessResult::neutral();
  }

Just on the off chance there's a non-base-field that is also not a config field (this is something a module like https://www.drupal.org/project/entity may define for BundleFieldDefinition)

Ultimately we'll fix that difference so that fieldGetPermissionType handles non-FieldConfigInterface field definitions better, but there are other issues open for that, and this solution should fix the immediate need.

I'll send up a patch for this in a bit.

mariacha1’s picture

StatusFileSize
new885 bytes

Here's that patch. Let me know if it solves the issue!

jhedstrom’s picture

I think this makes sense to commit now if it fixes the issue, since this is critical. It is a bit disconcerting though that neither the issue here, or the change over in #3371144: Invalid context for call to FieldDefinitionInterface->isDisplayConfigurable() were caught by tests. I'd recommend we do a follow-up to add test coverage for this so we don't regress it again in the future.

mariacha1’s picture

100% -- I don't even think it'd be too hard to write that test. I'll merge this the second someone who's experiencing this gets me a thumbs up, since I don't have a clean install running locally just now.

lolgm’s picture

StatusFileSize
new916 bytes

Thanks a lot for the help.
Unfortunately the issue #8 patch didn't work.
It seems to me that even with that condition it is not working for BaseFieldDefinition and BaseFieldOverride fields.
Perhaps the best solution is to directly check the type of FieldStorageDefinition, this way we guarantee that we filter only the expected fields.
Example below:

function field_permissions_entity_field_access($operation, FieldDefinitionInterface $field_definition, $account, FieldItemListInterface $items = NULL) {
  $context = ($operation == 'view') ? 'view' : 'edit';
  if (!$field_definition->isDisplayConfigurable($context) || empty($items) || !is_a($field_definition->getFieldStorageDefinition(), '\Drupal\field\FieldStorageConfigInterface')) {
    return AccessResult::neutral();
  }
  $access_field = \Drupal::service('field_permissions.permissions_service')->getFieldAccess($operation, $items, $account, $field_definition);
  ...
}
mariacha1’s picture

Status: Needs review » Reviewed & tested by the community

Aha, yep, this patch both fixes the problem and is more correct than mine. Nice! I tested on a clean branch this morning, and will merge shortly. Thanks for the teamwork everyone!

  • lolgm authored 1e6ad554 on 8.x-1.x
    Issue #3373500 by mariacha1, lolgm: TypeError: Drupal\field_permissions\...

  • 35ef7b33 committed on 8.x-1.x
    Issue #3373500: TypeError: Drupal\field_permissions\...
mariacha1’s picture

Status: Reviewed & tested by the community » Fixed

Patch is applied to the latest 8.x-1.x branch, and I've opened a followup ticket at https://www.drupal.org/project/field_permissions/issues/3374151 to test the hook for non-FieldConfig elements in the future.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

stephenplatz’s picture

I'm seeing something similar in

TypeError: Drupal\field_permissions\FieldPermissionsService::fieldGetPermissionType(): Argument #1 ($field) must be of type Drupal\field\FieldStorageConfigInterface, Drupal\Core\Field\BaseFieldDefinition given, called in /app/web/modules/contrib/field_permissions/src/FieldPermissionsService.php on line 198 in Drupal\field_permissions\FieldPermissionsService->fieldGetPermissionType() (line 138 of /app/web/modules/contrib/field_permissions/src/FieldPermissionsService.php).

using 8.x-1.3 and Drupal 10.2.4.

Making the same change to field_permissions_jsonapi_entity_field_filter_access fixes the error, but does anyone see any difficulties with that update? Maybe I had better make an issue branch so it can be tested.

mariacha1’s picture

Status: Closed (fixed) » Needs work

@stephenplatz I'm re-opening with "Needs work" so you can submit a patch or merge request here -- I think your proposal to add !is_a($field_definition->getFieldStorageDefinition(), '\Drupal\field\FieldStorageConfigInterface') to the`field_permissions_jsonapi_entity_field_filter_access` is great.

stephenplatz’s picture

@mariacha1 I went ahead and created a child issue for this here https://www.drupal.org/project/field_permissions/issues/3443837, I thought that may make sense, MR also there.

mariacha1’s picture

Status: Needs work » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.