Skip to content

Reject values that don't match the declared type - #8

Merged
MidnightDesign merged 1 commit into
masterfrom
reject-type-mismatches
Aug 4, 2026
Merged

Reject values that don't match the declared type#8
MidnightDesign merged 1 commit into
masterfrom
reject-type-mismatches

Conversation

@MidnightDesign

@MidnightDesign MidnightDesign commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Json::decode() silently type-coerced scalar constructor arguments. It now rejects them with a JsonError.

Json::decode('{"pinRequired":"not a boolean"}', Body::class)->pinRequired; // was: true
Json::decode('{"pinRequired":"false"}', Body::class)->pinRequired;         // was: true
Json::decode('{"code":42}', Body::class)->code;                            // was: '42'
Json::decode('{"articleType":50.9}', Body::class)->articleType;            // was: 50, + a deprecation notice

Nothing was thrown and nothing was logged — the caller received a well-formed object carrying a value the JSON never contained, and downstream code was then correct to trust it.

Why it happened

createConstructorArgumentForNamedType() passed builtin scalars straight through:

if ($type->isBuiltin()) {
    return $value;
}

Objects, enums and arrays were validated; scalars were not, leaving PHP's parameter binding as the only remaining check. That binding runs in weak mode, because instantiateClass() constructs through ReflectionClass::newInstanceArgs(). strict_types is a property of the call-site opcode, and the Reflection API builds the call from C, so there is no user-land call site to read the flag from:

new Box('nope');                                              // TypeError
(new ReflectionClass(Box::class))->newInstanceArgs(['nope']); // bool(true)   <-- no error

The declare(strict_types=1) at the top of Json.php bought nothing on this path. So the fix is to validate before constructing, not to change how the instance is constructed.

The fix

assertBuiltinType() checks the value against the declared ReflectionNamedType, mirroring 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

int → float keeps working, because that is the one widening strict mode itself permits — real APIs send a whole amount as 100, not 100.0, and a consumer declaring float $amount relies on it. It has its own named test.

iterable, object and callable are now rejected with Unsupported type "iterable" for parameter "value" of class … rather than passed through; json_decode() cannot produce a value that meaningfully satisfies them. never and void can't be parameter types, so they fall into the same arm for free.

Errors name the parameter, the class, the expected type and what arrived:

Expected bool for parameter "pinRequired" of class Body, got string

Properties too

The same JSON was already rejected when the target happened to be a plain property rather than a promoted constructor parameter — property assignment in populateProperty() is strict, so it raised a TypeError:

Json::decode('{"flag":"not a boolean"}', ViaCtor::class)->flag; // was: true
Json::decode('{"flag":"not a boolean"}', ViaProp::class)->flag; // was: TypeError

Two answers for one input, decided by a detail of the target class that has nothing to do with the JSON. Both paths now end in the same JsonError, so assertPropertyType() runs the same check before the dynamic assignment. It only fires for builtin named types — class-, enum- and union-typed properties keep their existing behavior.

Notes on the approach

  • src/Type/ is not reused for the check. That layer models JSON types, not PHP parameter types: Type\Number::validateValue() accepts int and float alike, so it cannot tell int $x from float $x — exactly the distinction this turns on. The check is written against ReflectionNamedType directly.
  • No error paths yet. assertBuiltinType() takes the subject as one pre-formatted string (parameter "age" of class Person), so a nested path can be prefixed there later without touching the check. The existing messages don't carry one either, so this doesn't make adding it harder.

Tests

One case per table row, accept and reject; int → float widening as its own named test; 50.9 into int; a nested object, so the failure survives instantiateClass() recursion; null into a nullable scalar vs. a non-nullable one; an omitted optional parameter whose default would itself fail the check, proving defaults aren't run through it; and ViaCtor / ViaProp asserted to produce parallel messages.

composer check is green — cs-check, psalm, phpstan with strict-rules, 150 tests, infection at 100% MSI and 100% covered MSI.

Backward compatibility

Breaking, and the point of the change. Payloads that decode today will start throwing at runtime rather than failing analysis. Codebases are likely to depend on the old coercion without knowing it, precisely because the library was already strict about the same JSON in the property case — so the lax behavior may have been relied on in one place and not another. CHANGELOG.md (new file) carries the table and an upgrade note; to be released as 0.2.0.

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.
@MidnightDesign
MidnightDesign requested a review from rieschl August 4, 2026 12:31
@MidnightDesign
MidnightDesign merged commit 2d04e4d into master Aug 4, 2026
12 checks passed
@MidnightDesign
MidnightDesign deleted the reject-type-mismatches branch August 4, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants