Skip to content

Commit

Permalink
More helpful exception message on NULL values.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
paragonie-security committed Oct 28, 2023
1 parent 575dda1 commit b368965
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 0 deletions.
51 changes: 51 additions & 0 deletions src/EncryptedRow.php
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 . '.'
);
}
}
19 changes: 19 additions & 0 deletions tests/EncryptedRowTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

0 comments on commit b368965

Please sign in to comment.