Using the disable eval setting crashes the module, but the module's use of eval() is redundant anyway

just duplicated using the less specific entity hooks.

This is not correct. The hook are all fired at different times. The reason we have to eval these hooks into existence is because there is no way of decrypting the entity prior to these hooks being fired. This can cause all sorts of problems. The hooks that you think are missing are not there because for these hooks we can decrypt the entity first - so we don't need to eval them into existence.

This is all documented in the module. See

/**
 * Implements hook_module_implements_alter().
 *
 * The Field Encrypt module decrypts and encrypts entity data by implementing
 * regular entity hooks. The order in which the hooks are fired determines
 * whether the entity data is encrypted or decrypted. It is important for that
 * for other implementations of these hooks that data is encrypted and decrypted
 * at the right time.
 * - field_encrypt_entity_storage_load() needs to be the first implementation
 *   of hook_entity_storage_load() so other implementations can use decrypted
 *   data.
 * - field_encrypt_entity_presave() needs to be the last implementation of
 *   hook_entity_presave() so other implementations can use decrypted
 *   data.
 * - field_encrypt_entity_insert() and field_encrypt_entity_update() can not
 *   be triggered in the correct location as hook_ENTITY_TYPE_insert() and
 *   hook_ENTITY_TYPE_update() are triggered first. Field encrypt tries to get
 *   around this by dynamically declaring hook_ENTITY_TYPE_insert() and
 *   hook_ENTITY_TYPE_update() implementations.
 *
 * @see field_encrypt_entity_storage_load()
 * @see field_encrypt_entity_presave()
 * @see field_encrypt_entity_insert()
 * @see field_encrypt_entity_update()
 * @see _field_encrypt_define_entity_hooks()
 * @see \Drupal\Core\Entity\EntityStorageBase::invokeHook()
 */
function field_encrypt_module_implements_alter(&$implementations, $hook) {

So since 2022, I have been using this module with the eval hooks disabled and a custom patch to remove the eval insertion.

I removed all my patches, pulled the latest 3.2.x-dev branch (using Encrypt 3.2 as a dependency), and tried to reproduce this-- and I can't get it to cause an error anymore.

I added $settings['field_encrypt.use_eval_for_entity_hooks'] = FALSE and things work as expected. Please re-open with detailed configuration to reproduce if you are still experiencing this issue with the latest dev of the module.

Ok, I found a way to still reproduce the issue-- if eval is disabled in settings.php AND you don't define the hooks, then field_encrypt will crash with An invalid implementation field_encrypt_{entity_type}_insert or whatever other hook it added.

I think throwing an exception is fine, but the message should be more clear-- for example, "The eval setting is disabled, but custom hooks haven't been added."

ptmkenny → closed merge request !56

Ok, I opened a new MR (57) that simply adds an exception if eval is not enabled and the relevant hooks haven't been created manually. I think this is a more informative error and can help the developer immediately determine what is wrong.

It's a little verbose, but I'd rather be pointed to exactly what I need to do than have to google the exception.

Can we have steps to reproduce the issue. I'm not sure about throwing an exception here. If we can't eval and you don't add the hooks yourself you should get an error on the system report that tells you in detail what to do - see field_encrypt_requirements().

FWIW if you change the value of $settings['field_encrypt.use_eval_for_entity_hooks'] you need to rebuild the module implements cache - so I'm not sure about the steps to reproduce in the issue summary. Maybe we can document that or somehow make the system aware of the need to rebuild when that changes (likely to be v hard to do).

Steps to reproduce

1. Configure a key, encryption file, and an encrypted field on the user profile.
2. Add a custom module with all the field_encrypt hooks for the user entity.

Example:

function field_encrypt_user_storage_load(array $entities): void {
  /** @var \Drupal\field_encrypt\ProcessEntities $field_encrypt_process_entities */
  $field_encrypt_process_entities = \Drupal::service('field_encrypt.process_entities');

  foreach ($entities as $entity) {
    $field_encrypt_process_entities->decryptEntity($entity);
  }
}

3. Add $settings['field_encrypt.use_eval_for_entity_hooks'] = FALSE; to settings.php.
4. drush cr
5. Everything should still work at this point because we have added the custom hooks. Now disable the custom module and run drush cr again.
6. Go to the Status Report page. It will list the hooks to add.
7. Attempt to save a user. WSOD with An invalid implementation field_encrypt_{entity_type}_insert.

How the MR solves this

I think the exception An invalid implementation field_encrypt_{entity_type}_insert is confusing, so I added a new exception that makes it very clear the hook hasn't been added.

The downside about this check is that it is a little redundant:

          if (_field_encrypt_can_eval()) {
            _field_encrypt_define_entity_hooks($entity_type_id);
          }

Because _field_encrypt_define_entity_hooks() immediately calls _field_encrypt_can_eval(). However, to add the new exception, we need to do this check before calling _field_encrypt_define_entity_hooks(), because _field_encrypt_define_entity_hooks() is called in several places, and we only need to throw the exception if we can't eval in this specific case.

This is no longer an issue on the 4.x branch. So I think we should close this and wait for time to pass. once everyone is on 11.1.x and greater this is no longer an issue.