Repository navigation
Validator JSON types, empty whereIn, pgsql emulate_prepares, string traces - #431
Merged
Merged
Conversation
A JSON body is decoded with native types, so a field declared `nullable|boolean` received a real false and the nullable rule passed it to the string-typed Str::isEmpty(), throwing a TypeError (HTTP 500) before the boolean rule ever ran. The same assumption broke nullable with ints, floats and arrays, max/min/size with numbers and booleans, and alpha, alphanum, lower, upper and email with any non-string. - NullableRule: a value is absent only when null, a blank string or an empty array; false, 0 and 0.0 are real values and reach the next rules. - StringRule: max, min and size compare native numbers by value and strings by length, following the convention documented by between. Numeric strings keep length semantics so form inputs are unchanged. Booleans and arrays fail the rule instead of throwing. - alpha, alphanum, lower, upper and email fail cleanly on non-strings. Adds a database-free regression test class covering each case and a full JSON payload.
whereIn() with an empty array built `column in ()`, invalid SQL (SQLSTATE 42601) that `php bow migrate` hit on a project with no migrations yet. An empty list now compiles to `in (null)`, valid everywhere and matching no row. PostgreSQLAdapter honours a per-connection `emulate_prepares` config key (default false). Behind a pooler a server-side prepared statement can outlive a schema change and fail with SQLSTATE 0A000 "cached plan must not change result type"; with emulated prepares PDO quotes the values itself and nothing is cached on the server. Both were previously re-applied as vendor patches by an application; regression tests added for each.
getTrace() returns frames with their arguments, and serializing those can run code: an argument object whose jsonSerialize() issues a query throws again when the transaction is already aborted, so the logger itself failed while reporting the original error. - LoggerConfiguration and QueueAdapter log the exception class, file:line and getTraceAsString() as context instead of the frame array. - BaseErrorHandler puts getTraceAsString() in the non-production JSON response instead of the frame array.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three commits, each independently revertable.
1. Validator: stop crashing on native JSON input types (
a072199c)Bug. A JSON body is decoded with native types, so a field declared
nullable|booleanreceived a realfalse. The nullable rule passed it to the string-typedStr::isEmpty(), which throws a TypeError understrict_types, so the request died with a 500 before the boolean rule ran. The same assumption brokenullablewith ints, floats and arrays,max/min/sizewith numbers and booleans, andalpha,alphanum,lower,upper,emailwith any non-string.Fix.
NullableRule: a value is absent only whennull, a blank string or an empty array.false,0,0.0are real values and reach the following rules.StringRule:max,min,sizeshare aninputSize()helper following the conventionbetweenalready documents: native numbers compare by value, strings by length. Numeric strings keep length semantics, so a form phone number declaredmax:10behaves as before. Booleans and arrays fail the rule instead of throwing.alpha,alphanum,lower,upper,emailfail cleanly on non-strings.Tests. New database-free
tests/Validation/ValidatorTypedInputTest.php(15 tests), written first and failing with 12 TypeErrors. ExistingValidationTest(102 tests) still passes against MySQL.2. Database: empty
whereIn()list and pgsqlemulate_prepares(70a1026a)Both were being re-applied as vendor patches by an application at build time.
whereIn('id', [])builtid in (), invalid SQL (SQLSTATE 42601) thatphp bow migratehits on a project with no migrations yet. It now compiles toid in (null): valid on every driver, matches no row, no bindings. Test on in-memory SQLite checks the SQL, the bindings and the empty result.PostgreSQLAdapterhonours a per-connectionemulate_prepareskey, defaultfalseso nothing changes unless opted in. Behind a pooler a server-side prepared statement can outlive a schema change and fail with SQLSTATE 0A000 "cached plan must not change result type"; with emulated prepares PDO quotes the values and nothing is cached on the server. Two connection tests: off by default, on when configured.3. Log and report exception traces as strings (
54c32163)getTrace()returns frames with their arguments, and serializing those can run code: an argument object whosejsonSerialize()issues a query throws again on an already-aborted transaction, so the logger failed while reporting the original error.LoggerConfigurationandQueueAdapternow log the exception class,file:lineandgetTraceAsString().BaseErrorHandlerputs the string trace in the non-production JSON response.Verification
tests/Validation: 117 tests pass (102 existing against MySQL, 15 new without a database).tests/Database: 778 tests pass against SQLite, MySQL and PostgreSQL in throwaway containers (Redis excluded, no server).phpcs --standard=psr12clean on every changed source file.Not changed
whereNotIn()with an empty array still buildsnot in (). The right empty-list behaviour there is "exclude nothing", the opposite ofwhereIn, so it is left for a separate decision.Companion docs PR: bowphp/docs "Document validator JSON types, empty whereIn and PostgreSQL options".