orm
orm copied to clipboard
laravel-doctrine
[FIX] Fix substitute binding type conversion
Doctrine documentation says, that PK can be either INT or STRING.
There is not any type conversion. Let's say we have some URL /{document}/update.
If document has INT PK and we enter something like that /1d/update - we will catch an error from database ERROR: invalid input syntax for type integer: \"1d\".
This fix prevents type conversion errors.
Isn't this clearly a 404? I don't like that the library should just auto cast this. Seems wrong.. Do you have a more realistic scenario?
@eigan I agree, that it should be 404, but it produces runtime type error on the database level with 500 error code, you can't handle this exception. In the example above, if client provides 1d and ID type is INT then it will be casted to 1. If client provides d1 it will be casted to 0.
If a PK type is INT and a string is passed it really should be a 404 (possible model not found)? Not casting that string and return an incorrect record, which is what you say your fix will do?
example
uses is inputing a url for handwritten paper
http://example.com/user/1456 :- 1456 is int and correctly returns id 1456
vs
http://example.com/user/14S6 : - 14S6 they miss read 5 as S, this is now cast to int and returns id 14 ??? really that should be 404 model not found
@dpslwk oh, yeah, I see.
So maybe, it should be rewritten to checking if PK is INT and check id value with function is_numeric If it returns false we are not performing any queries to database, we just throw 404? What do you think about that?
yeah posible
Its worth investigating how Laravel with eloquent handles this What does it return in the example I gave above?
@dpslwk just reproduced that on eloquent and received the same result - 500. Eloquent doesn't handle this stuff
@DemianShtepa I think for this behavior on level of routing pattern should be applied. To allow only number for the route.
