From b3689655a8726fd156d0eb31fdb4f0592a30d123 Mon Sep 17 00:00:00 2001
From: "Paragon Initiative Enterprises, LLC" <security@paragonie.com>
Date: Sat, 28 Oct 2023 06:16:13 -0400
Subject: [PATCH] More helpful exception message on NULL values.

If you do not declare a field optional, it generally will not accept NULL as a value on encrypt. Boolean is the exception to this rule (for backwards compat).

However, non-optional fields (even booleans) must have a ciphertext on the decrypt path.

```
Encrypt:
  TYPE_BOOLEAN + (null) -> ciphertext
  TYPE_OPTIONAL_BOOLEAN + (null) -> ciphertext

Decrypt:
  TYPE_BOOLEAN + (null) -> TypeError
  TYPE_OPTIONAL_BOOLEAN + (null) -> null
```

Booleans are the weird ones, though.

```
Encrypt:
  TYPE_TEXT + (null) -> TypeError
  TYPE_OPTIONAL_TEXT + (null) -> null

Decrypt:
  TYPE_TEXT + (null) -> TypeError
  TYPE_OPTIONAL_BOOLEAN + (null) -> null
```

Every other type doesn't tolerate null implicitly. This behavior is because of a very early design decision with boolean types.
---
 src/EncryptedRow.php       | 51 ++++++++++++++++++++++++++++++++++++++
 tests/EncryptedRowTest.php | 19 ++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/src/EncryptedRow.php b/src/EncryptedRow.php
index a88844c..adcb317 100644
--- a/src/EncryptedRow.php
+++ b/src/EncryptedRow.php
@@ -527,6 +527,9 @@ public function decryptRow(
             }
             // Encrypted booleans will be scalar values as ciphertext
             if (!is_scalar($row[$field])) {
+                if (is_null($row[$field])) {
+                    $this->fieldNotOptional($field, $type);
+                }
                 throw new TypeError('Invalid type for ' . $field);
             }
             if (
@@ -627,6 +630,9 @@ public function encryptRow(
             // All others must be scalar
             if (!is_scalar($row[$field])) {
                 // NULL is not permitted.
+                if (is_null($row[$field])) {
+                    $this->fieldNotOptional($field, $type);
+                }
                 throw new TypeError('Invalid type for ' . $field);
             }
             $plaintext = $this->convertToString($row[$field], $type);
@@ -1014,4 +1020,49 @@ protected function coaxAadToString(mixed $input): string
         /** psalm-suppress PossiblyInvalidCast */
         return (string) $input;
     }
+
+    /**
+     * New exception message to make it clear this is a deliberate behavior, not a bug.
+     *
+     * Instead of, like, Constants::TYPE_TEXT, if you want to accept null parameters, you need to
+     * use something like Constants::TYPE_OPTIONAL_TEXT.
+     *
+     * If you don't tell CipherSweet that NULL is an acceptable return type, it doesn't tolerate
+     * NULL. To do this, you must change the declaration.
+     *
+     * This switch block tries to point the user of this library towards the correct constant to use
+     * for this field, in order to populate the correct error message.
+     */
+    protected function fieldNotOptional(string $field, string $type): void
+    {
+        switch ($type) {
+            case Constants::TYPE_JSON:
+                $oldConst = 'Constants::TYPE_JSON';
+                $newConst = 'Constants::TYPE_OPTIONAL_JSON';
+                break;
+            case Constants::TYPE_BOOLEAN:
+                $oldConst = 'Constants::TYPE_BOOLEAN';
+                $newConst = 'Constants::TYPE_OPTIONAL_BOOLEAN';
+                break;
+            case Constants::TYPE_TEXT:
+                $oldConst = 'Constants::TYPE_TEXT';
+                $newConst = 'Constants::TYPE_OPTIONAL_TEXT';
+                break;
+            case Constants::TYPE_FLOAT:
+                $oldConst = 'Constants::TYPE_FLOAT';
+                $newConst = 'Constants::TYPE_OPTIONAL_FLOAT';
+                break;
+            case Constants::TYPE_INT:
+                $oldConst = 'Constants::TYPE_INT';
+                $newConst = 'Constants::TYPE_OPTIONAL_INT';
+                break;
+            default:
+                $oldConst = $type;
+                $newConst = '?' . $type;
+        }
+        throw new TypeError(
+            'Received a NULL value for ' . $field . ', which has a non-optional type. ' .
+            'To fix this, try changing the type declaration from ' . $oldConst . ' to ' . $newConst . '.'
+        );
+    }
 }
diff --git a/tests/EncryptedRowTest.php b/tests/EncryptedRowTest.php
index 114af19..cf91fa8 100644
--- a/tests/EncryptedRowTest.php
+++ b/tests/EncryptedRowTest.php
@@ -541,4 +541,23 @@ public function tesOptionalFields(CipherSweet $engine): void
         $encrypted = $eR->encryptRow($null);
         $this->assertNotSame($null, $encrypted);
     }
+
+    /**
+     * @dataProvider engineProvider
+     */
+    public function testOptionalFieldsMisconfigured(CipherSweet $engine): void
+    {
+        $eR = new EncryptedRow($engine, 'foo');
+        $eR
+            ->addOptionalTextField('bar')
+            ->addTextField('baz');
+
+        $null = ['bar' => 'bar', 'baz' => null];
+
+        $this->expectExceptionMessage(
+            'Received a NULL value for baz, which has a non-optional type. To fix this, try changing the type' .
+            ' declaration from Constants::TYPE_TEXT to Constants::TYPE_OPTIONAL_TEXT.'
+        );
+        $eR->encryptRow($null);
+    }
 }