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(), + ); + } }