Empty keys (e.g. from environment variable when not set in the current environment) shouldn't break entire site in RealAESEncryptionMethod->decrypt()

Optimistically moving to "needs review"... ¯\_(ツ)_/¯

Commit added to also handle empty keys when trying to _encrypt_, too.

It can't return FALSE, as the only acceptable return type is a string. Or the exception. Please make it return an empty string, and then test if that is still a solution that would work.

This is the documentation for EncryptionMethodInterface from the Encrypt module:

  /**
   * Decrypt text.
   *
   * @param string $text
   *   The text to be decrypted.
   * @param string $key
   *   The key to decrypt the text with.
   *
   * @return string
   *   The decrypted text
   *
   * @throws \Drupal\encrypt\Exception\EncryptException
   *   Thrown when decryption fails.
   * @throws \Drupal\encrypt\Exception\EncryptionMethodCanNotDecryptException
   *   The method should throw this exception when the plugin can not decrypt
   *   (i.e. use a public key).
   */
  public function decrypt($text, $key);

The Real AES module implements this interface, and the behavior as currently implemented is correct according to the interface. So if you want to change this behavior, the Encrypt interface needs to be modified upstream in the Encrypt module. (There are a few issues related to this topic, so changing the interface is worth discussing.)

If you still want to pursue this, please open an issue in the Encrypt module issue queue; the interface will have to be changed in order to change this module's behavior.

At one point, that interface did allow FALSE to be returned, and that created some issues, as there is really no circumstance where that should happen on the call to decrypt(). It either decrypts and the decrypted string is returned or it doesn't decrypt, and an exception should be raised. It was found that sometimes developers wanted a "soft" exception and returned FALSE, and other times threw the exception.

However, calling decrypt("") is really an edge case here, as it might actually be that the correct answer to that is to return back the empty string. That was what I suggested in #4.

Ideally, the tfa module that @alexharries referred to in the issue summary should be modified to somehow handle the exception or simply prevent calling the function with an empty string, which I believe is done (see https://git.drupalcode.org/project/tfa/-/blame/8.x-1.x/src/Plugin/TfaVal...).

Thank you for the additional information; that's very helpful to understand the context here.

If it's been fixed upstream, all the more reason to leave this closed.

I appreciate you sharing this because I've been trying to clean up the Encrypt ecosystem issue queues, so understanding more of the background is useful.