From de7fbdff83cbf0d9f6e2f52dd7cb38b68147578c Mon Sep 17 00:00:00 2001 From: Rudolph Gottesheim Date: Tue, 4 Aug 2026 14:10:55 +0200 Subject: [PATCH] Reject values that don't match the declared type Builtin scalar constructor arguments were passed through unchecked. The instance is created via the Reflection API, which always binds arguments in weak mode regardless of any declare(strict_types=1), so PHP silently coerced them: {"pinRequired":"not a boolean"} decoded to true, {"code":42} to '42', and {"articleType":50.9} to 50 plus a deprecation notice. Decoding reported success while handing the caller a value the payload never contained. Check the value against the declared ReflectionNamedType before constructing instead, mirroring strict-mode parameter binding: int widens to float, nothing else converts. Properties are checked the same way before assignment, so a promoted constructor parameter and a plain property no longer give two different answers for one input. BREAKING CHANGE: payloads that decode today may start throwing JsonError. --- CHANGELOG.md | 69 ++++++ src/Json.php | 55 +++++ tests/unit/Fixtures/HasBoolProperty.php | 10 + tests/unit/Fixtures/TakesBool.php | 12 + tests/unit/Fixtures/TakesCallable.php | 15 ++ tests/unit/Fixtures/TakesFalse.php | 12 + tests/unit/Fixtures/TakesFloat.php | 12 + tests/unit/Fixtures/TakesInt.php | 12 + tests/unit/Fixtures/TakesIterable.php | 15 ++ tests/unit/Fixtures/TakesMixed.php | 12 + tests/unit/Fixtures/TakesNull.php | 12 + tests/unit/Fixtures/TakesObject.php | 12 + tests/unit/Fixtures/TakesString.php | 12 + tests/unit/Fixtures/TakesTrue.php | 12 + tests/unit/JsonTest.php | 279 ++++++++++++++++++++++++ 15 files changed, 551 insertions(+) create mode 100644 CHANGELOG.md create mode 100644 tests/unit/Fixtures/HasBoolProperty.php create mode 100644 tests/unit/Fixtures/TakesBool.php create mode 100644 tests/unit/Fixtures/TakesCallable.php create mode 100644 tests/unit/Fixtures/TakesFalse.php create mode 100644 tests/unit/Fixtures/TakesFloat.php create mode 100644 tests/unit/Fixtures/TakesInt.php create mode 100644 tests/unit/Fixtures/TakesIterable.php create mode 100644 tests/unit/Fixtures/TakesMixed.php create mode 100644 tests/unit/Fixtures/TakesNull.php create mode 100644 tests/unit/Fixtures/TakesObject.php create mode 100644 tests/unit/Fixtures/TakesString.php create mode 100644 tests/unit/Fixtures/TakesTrue.php diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..9f49cb7 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,69 @@ +# Changelog + +All notable changes to this project will be documented in this file. + +The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to +[Semantic Versioning](https://semver.org/spec/v2.0.0.html). + +## [0.2.0] - 2026-08-04 + +### Changed + +- **BREAKING**: `Json::decode()` now rejects values whose JSON type does not match the declared PHP type instead of + coercing them. Both promoted constructor parameters and plain properties throw a `JsonError` naming the parameter or + property, its class, the expected type and what arrived: + + ``` + Expected bool for parameter "pinRequired" of class Body, got string + ``` + + Previously, values bound to constructor parameters were passed through unchecked. The instance is created through the + Reflection API, which always binds arguments in weak mode regardless of any `declare(strict_types=1)`, so PHP silently + coerced them: `{"pinRequired":"not a boolean"}` decoded to `true`, `{"code":42}` to `'42'`, and `{"articleType":50.9}` + to `50` plus a deprecation notice. Decoding reported success while handing the caller a value the payload never + contained. + + The check mirrors strict-mode parameter binding: + + | Declared type | Accepted | Rejected | + | --- | --- | --- | + | `bool` | `true`, `false` | everything else, including `"true"`, `"false"`, `0`, `1` | + | `int` | int | float (including `50.0`), numeric string, bool, everything else | + | `float` | float, **and int** | numeric string, bool, everything else | + | `string` | string | int, float, bool, everything else | + | `true` / `false` / `null` | exactly that value | everything else | + | `mixed` | anything | – | + + Widening an int to a float keeps working, because that is the one conversion strict mode itself performs: a payload + sending a whole amount as `100` still decodes into `float $amount` as `100.0`. + + Nullable parameters and properties still accept `null`, non-nullable ones no longer do, and omitted optional + parameters still take their default without being checked. + +- **BREAKING**: Constructor parameters and properties typed `iterable`, `object` or `callable` are now rejected with + `Unsupported type "iterable" for parameter "value" of class …` instead of being passed through. `json_decode()` cannot + produce a value that meaningfully satisfies them. + +### Upgrading + +Payloads that decode today may start throwing `JsonError` at runtime rather than failing analysis. Codebases are likely +to depend on the old coercion without knowing it, because the library was already strict about the very same JSON when +the target field happened to be a plain property rather than a promoted constructor parameter: + +```php +final class ViaCtor { public function __construct(public bool $flag = false) {} } +final class ViaProp { public bool $flag = false; } + +Json::decode('{"flag":"not a boolean"}', ViaCtor::class)->flag; +// before: true — after: JsonError +Json::decode('{"flag":"not a boolean"}', ViaProp::class)->flag; +// before: TypeError — after: JsonError +``` + +One input produced two answers, decided by a detail of the target class that has nothing to do with the JSON. Both paths +now fail the same way, so a codebase may have been relying on the lax behavior in one place and not in another. + +If a producer legitimately sends a scalar in a different shape than the consumer declares — numbers as strings, for +instance — declare the parameter with the type that is actually sent and convert it in your own code. + +[0.2.0]: https://github.com/eventjet/eventjet-json/compare/v0.1.3...v0.2.0 diff --git a/src/Json.php b/src/Json.php index f8f7a7d..ff028fc 100644 --- a/src/Json.php +++ b/src/Json.php @@ -202,9 +202,26 @@ private static function populateProperty(object $object, string $jsonKey, mixed $value = $newValue; } } + self::assertPropertyType($object, $property, $value); $object->$property = $value; // @phpstan-ignore-line } + private static function assertPropertyType(object $object, string $property, mixed $value): void + { + $type = (new ReflectionProperty($object, $property))->getType(); + if (!$type instanceof ReflectionNamedType || !$type->isBuiltin()) { + return; + } + if ($value === null && $type->allowsNull()) { + return; + } + self::assertBuiltinType( + $type->getName(), + $value, + sprintf('property "%s" of class %s', $property, $object::class), + ); + } + private static function getPropertyNameForJsonKey(object $object, string $key): string { foreach ((new ReflectionObject($object))->getProperties() as $property) { @@ -316,6 +333,13 @@ private static function createConstructorArgumentForNamedType(ReflectionParamete return self::createConstructorArgumentForArrayType($parameter, $value); } if ($type->isBuiltin()) { + $class = $parameter->getDeclaringClass(); + assert($class !== null); + self::assertBuiltinType( + $typeName, + $value, + sprintf('parameter "%s" of class %s', $paramName, $class->getName()), + ); return $value; } if (enum_exists($typeName)) { @@ -354,6 +378,37 @@ private static function createConstructorArgumentForNamedType(ReflectionParamete return self::instantiateClass($typeName, $value); } + /** + * Constructor arguments are bound by the Reflection API, which always uses weak mode and would silently coerce + * them, and property values are assigned dynamically. Check both against the declared type ourselves so that the + * two paths reject the same payloads with the same error. + * + * @param string $subject Describes the target the value is checked against, e.g. `parameter "age" of class Person`. + */ + private static function assertBuiltinType(string $typeName, mixed $value, string $subject): void + { + $isValid = match ($typeName) { + 'mixed' => true, + 'bool' => is_bool($value), + 'int' => is_int($value), + // Widening an int to a float is the one conversion strict mode itself performs. + 'float' => is_float($value) || is_int($value), + 'string' => is_string($value), + 'array' => is_array($value), + 'true' => $value === true, + 'false' => $value === false, + // Nulls are returned before we get here if the type allows them, so nothing left can satisfy this type. + 'null' => false, + default => throw JsonError::decodeFailed( + sprintf('Unsupported type "%s" for %s', $typeName, $subject), + ), + }; + if ($isValid) { + return; + } + throw JsonError::decodeFailed(sprintf('Expected %s for %s, got %s', $typeName, $subject, gettype($value))); + } + private static function createConstructorArgumentForUnionType(): mixed { throw JsonError::decodeFailed('Union types are not supported'); diff --git a/tests/unit/Fixtures/HasBoolProperty.php b/tests/unit/Fixtures/HasBoolProperty.php new file mode 100644 index 0000000..ce7a319 --- /dev/null +++ b/tests/unit/Fixtures/HasBoolProperty.php @@ -0,0 +1,10 @@ +value = $value; + } +} diff --git a/tests/unit/Fixtures/TakesFalse.php b/tests/unit/Fixtures/TakesFalse.php new file mode 100644 index 0000000..8b7cd9d --- /dev/null +++ b/tests/unit/Fixtures/TakesFalse.php @@ -0,0 +1,12 @@ + $value + */ + public function __construct(public iterable $value = []) + { + } +} diff --git a/tests/unit/Fixtures/TakesMixed.php b/tests/unit/Fixtures/TakesMixed.php new file mode 100644 index 0000000..5c2b0c4 --- /dev/null +++ b/tests/unit/Fixtures/TakesMixed.php @@ -0,0 +1,12 @@ +map); }, ]; + yield 'Bool constructor argument' => [ + '{"value":true}', + TakesBool::class, + static function (object $object): void { + self::assertInstanceOf(TakesBool::class, $object); + self::assertTrue($object->value); + }, + ]; + yield 'Int constructor argument' => [ + '{"value":42}', + TakesInt::class, + static function (object $object): void { + self::assertInstanceOf(TakesInt::class, $object); + self::assertSame(42, $object->value); + }, + ]; + yield 'Float constructor argument' => [ + '{"value":1.5}', + TakesFloat::class, + static function (object $object): void { + self::assertInstanceOf(TakesFloat::class, $object); + self::assertSame(1.5, $object->value); + }, + ]; + yield 'String constructor argument' => [ + '{"value":"foo"}', + TakesString::class, + static function (object $object): void { + self::assertInstanceOf(TakesString::class, $object); + self::assertSame('foo', $object->value); + }, + ]; + yield 'Mixed constructor argument takes a string' => [ + '{"value":"foo"}', + TakesMixed::class, + static function (object $object): void { + self::assertInstanceOf(TakesMixed::class, $object); + self::assertSame('foo', $object->value); + }, + ]; + yield 'Mixed constructor argument takes a JSON object' => [ + '{"value":{"foo":"bar"}}', + TakesMixed::class, + static function (object $object): void { + self::assertInstanceOf(TakesMixed::class, $object); + self::assertSame(['foo' => 'bar'], $object->value); + }, + ]; + yield 'True constructor argument' => [ + '{"value":true}', + TakesTrue::class, + static function (object $object): void { + self::assertInstanceOf(TakesTrue::class, $object); + }, + ]; + yield 'False constructor argument' => [ + '{"value":false}', + TakesFalse::class, + static function (object $object): void { + self::assertInstanceOf(TakesFalse::class, $object); + }, + ]; + yield 'Null constructor argument' => [ + '{"value":null}', + TakesNull::class, + static function (object $object): void { + self::assertInstanceOf(TakesNull::class, $object); + }, + ]; + yield 'Null for nullable constructor argument' => [ + '{"name":null}', + NullableStringField::class, + static function (object $object): void { + self::assertInstanceOf(NullableStringField::class, $object); + self::assertNull($object->name); + }, + ]; + yield 'Omitted optional argument keeps its default and is not type checked' => [ + '{}', + TakesIterable::class, + static function (object $object): void { + self::assertInstanceOf(TakesIterable::class, $object); + self::assertSame([], $object->value); + }, + ]; + yield 'Bool property' => [ + '{"value":true}', + HasBoolProperty::class, + static function (object $object): void { + self::assertInstanceOf(HasBoolProperty::class, $object); + self::assertTrue($object->value); + }, + ]; } /** @@ -518,6 +624,153 @@ public function __construct(public DoesNotExist|null $nested = null) 'The type of the constructor parameter "map" for class Eventjet\Test\Unit\Json\Fixtures\UndocumentedMap is ' . '"array", but its shape is not documented', ]; + yield 'String for bool constructor argument' => [ + '{"value":"not a boolean"}', + TakesBool::class, + 'Expected bool for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesBool, got string', + ]; + yield '"false" for bool constructor argument' => [ + '{"value":"false"}', + TakesBool::class, + 'Expected bool for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesBool, got string', + ]; + yield 'Zero for bool constructor argument' => [ + '{"value":0}', + TakesBool::class, + 'Expected bool for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesBool, got integer', + ]; + yield 'One for bool constructor argument' => [ + '{"value":1}', + TakesBool::class, + 'Expected bool for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesBool, got integer', + ]; + yield 'Fractional float for int constructor argument' => [ + '{"value":50.9}', + TakesInt::class, + 'Expected int for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesInt, got double', + ]; + yield 'Whole float for int constructor argument' => [ + '{"value":50.0}', + TakesInt::class, + 'Expected int for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesInt, got double', + ]; + yield 'Numeric string for int constructor argument' => [ + '{"value":"42"}', + TakesInt::class, + 'Expected int for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesInt, got string', + ]; + yield 'Bool for int constructor argument' => [ + '{"value":true}', + TakesInt::class, + 'Expected int for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesInt, got boolean', + ]; + yield 'Numeric string for float constructor argument' => [ + '{"value":"1.5"}', + TakesFloat::class, + 'Expected float for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesFloat, got string', + ]; + yield 'Bool for float constructor argument' => [ + '{"value":true}', + TakesFloat::class, + 'Expected float for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesFloat, got boolean', + ]; + yield 'Int for string constructor argument' => [ + '{"value":42}', + TakesString::class, + 'Expected string for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesString, got integer', + ]; + yield 'Float for string constructor argument' => [ + '{"value":1.5}', + TakesString::class, + 'Expected string for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesString, got double', + ]; + yield 'Bool for string constructor argument' => [ + '{"value":true}', + TakesString::class, + 'Expected string for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesString, got boolean', + ]; + yield 'False for true constructor argument' => [ + '{"value":false}', + TakesTrue::class, + 'Expected true for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesTrue, got boolean', + ]; + yield 'One for true constructor argument' => [ + '{"value":1}', + TakesTrue::class, + 'Expected true for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesTrue, got integer', + ]; + yield 'True for false constructor argument' => [ + '{"value":true}', + TakesFalse::class, + 'Expected false for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesFalse, got boolean', + ]; + yield 'Zero for false constructor argument' => [ + '{"value":0}', + TakesFalse::class, + 'Expected false for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesFalse, got integer', + ]; + yield 'String for null constructor argument' => [ + '{"value":"foo"}', + TakesNull::class, + 'Expected null for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesNull, got string', + ]; + yield 'Null for non-nullable constructor argument' => [ + '{"name":null}', + StringField::class, + 'Expected string for parameter "name" of class Eventjet\Test\Unit\Json\Fixtures\StringField, got NULL', + ]; + yield 'Iterable constructor argument' => [ + '{"value":[]}', + TakesIterable::class, + 'Unsupported type "iterable" for parameter "value" of class ' + . 'Eventjet\Test\Unit\Json\Fixtures\TakesIterable', + ]; + yield 'Object constructor argument' => [ + '{"value":{"foo":"bar"}}', + TakesObject::class, + 'Unsupported type "object" for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesObject', + ]; + yield 'Callable constructor argument' => [ + '{"value":"strlen"}', + TakesCallable::class, + 'Unsupported type "callable" for parameter "value" of class ' + . 'Eventjet\Test\Unit\Json\Fixtures\TakesCallable', + ]; + yield 'Type mismatch in a nested object' => [ + '{"nested":{"name":42}}', + HasNestedClass::class, + 'Expected string for parameter "name" of class Eventjet\Test\Unit\Json\Fixtures\StringField, got integer', + ]; + yield 'String for bool property' => [ + '{"value":"not a boolean"}', + HasBoolProperty::class, + 'Expected bool for property "value" of class Eventjet\Test\Unit\Json\Fixtures\HasBoolProperty, got string', + ]; + yield 'Int for nullable string property' => [ + '{"name":42}', + new NullableStringField(), + 'Expected string for property "name" of class Eventjet\Test\Unit\Json\Fixtures\NullableStringField, got ' + . 'integer', + ]; + yield 'String for array property' => [ + '{"topics":"foo"}', + new Repository(), + 'Expected array for property "topics" of class Eventjet\Test\Unit\Json\Fixtures\GitHub\Repository, got ' + . 'string', + ]; + } + + /** + * @param class-string $class + */ + private static function captureDecodeError(string $json, string $class): JsonError + { + try { + Json::decode($json, $class); + } catch (JsonError $error) { + return $error; + } + self::fail(sprintf('Expected decoding %s into %s to fail, but it succeeded', $json, $class)); } #[DataProvider('encodeCases')] @@ -573,4 +826,30 @@ public function testFailingDecode(string $json, object|string $object, string|nu Json::decode($json, $object); } + + /** + * Widening an int to a float is the one conversion strict-mode parameter binding permits, and APIs routinely send + * whole amounts without a fractional part. + */ + public function testIntIsWidenedToFloatConstructorArgument(): void + { + $decoded = Json::decode('{"value":100}', TakesFloat::class); + + self::assertSame(100.0, $decoded->value); + } + + public function testPromotedConstructorParametersAndPlainPropertiesRejectTheSameValue(): void + { + $viaConstructor = self::captureDecodeError('{"value":"not a boolean"}', TakesBool::class); + $viaProperty = self::captureDecodeError('{"value":"not a boolean"}', HasBoolProperty::class); + + self::assertSame( + 'Expected bool for parameter "value" of class Eventjet\Test\Unit\Json\Fixtures\TakesBool, got string', + $viaConstructor->getMessage(), + ); + self::assertSame( + 'Expected bool for property "value" of class Eventjet\Test\Unit\Json\Fixtures\HasBoolProperty, got string', + $viaProperty->getMessage(), + ); + } }