Prepared statements seem not to trigger an error if parameter typ is wrong

[HLeithner]

While rewriting the cms queryies, I tested the affect of wrong input type and it seam that mysql don't care.

Is this normal? Or maybe I did something wrong...

If this expected i would like to suggest to test the type in our db layer and throw an exception if it's wrong.

[mbabker]

It instructs PDO or the mysqli API how to handle the data, it isn’t doing a type validation that what you passed is a certain PHP scalar. Nothing to fix here IMO.

On Wed, May 29, 2019 at 5:13 PM Harald Leithner ***@***.***> wrote: While rewriting the cms queryies, I tested the affect of wrong input type and it seam that mysql don't care. Is this normal? Or maybe I did something wrong... If this expected i would like to suggest to test the type in our db layer and throw an exception if it's wrong. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#159?email_source=notifications&email_token=AACZ7IIF666GYL32ITSYRK3PX35XLA5CNFSM4HQWSYG2YY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4GWTD2VQ>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACZ7INK4SX6YCSDGA4ILHDPX35XLANCNFSM4HQWSYGQ> .

-- - Michael Please pardon any errors, this message was sent from my iPhone.

[HLeithner]

Hmm but if I give an string to mysql and say its and integer it should give me a warning at least? Isn't this the whole point telling mysql what type the variable is?

[alikon]

mysql is a little bit lighthearted on this (pdo or not), the opposite of pgsql that is much more strict

[HLeithner]

@alikon Can you test the behavior on pgsql?

[alikon]

i've already experienced that see joomla/joomla-cms#24149

[HLeithner]

Ok but this error only tells us that the target field is not an boolean but pdo still except an integer as input. Is this valid for string and integer too?

[mbabker]

I can't say I've run into these type of type checking problems you're seeing, but this isn't a behavior that belongs to a DBAL IMO (and if it does you need to convince Doctrine and Laravel to do the same).

Databases don't have a scalar boolean type, true/false is generally stored in a TINYINT column as 1/0. Unless I've completely misunderstood something, specifying the parameter type signals to PDO and the mysqli APIs the data should be internally processed to convert whatever you've given it to a representation that matches the type you've given (so those APIs handle converting boolean true to integer 1 when writing the query, because obviously you aren't getting a boolean as your query result without an ORM). Specifying a parameter type does not equate to requiring that data pass a is_(array|bool|float|int|string) check, and that would probably be a performance bottleneck if it started to be executed for every parameter on every query.

[HLeithner]

atm I try to implement prepared statements into the sqlsrv driver and sqlsrv has a bool for example but still I didn't found out if all drivers silently convert input variables to it needs.

btw. would be really awesome to have declare strict but thats another topic

[mbabker]

Before I gave up putting too much of my time into things, I wanted to just rip apart and completely rebuild the database test suites. Because right now there are a lot of janky and goofy tests and not a lot of good functional/integration tests. Plus, with all the changes I've made to the Travis and AppVeyor configs, everything's in a better spot to be able to test behaviors on specific versions of specific database platforms whereas in the CMS repo everything's testing on one MySQL version.

[HLeithner]

mssqlsrv doesn't like different datatypes:
https://ci.appveyor.com/project/joomla/database/builds/24926396/job/7mq3farm0a2krxaq

[mbabker]

*sigh* I forgot that wasn't a PDO driver.

So if SQL Server is that picky about it then add type checks in that query/statement class. Either way it doesn't come across to me as something that belongs to DatabaseQuery::bind() as a default behavior to either be type checking or typecasting data given to it (none of the Framework apps in production have as high query usage as the CMS, the CMS easily gets into 25-30 queries per request and most queries have at least one parameter, stuff like modules and plugins can get into 4 or 5 params easily, this sounds like it could easily start to create a performance bottleneck). To me it feels like what is happening now is exactly what should happen; let the PHP extension internally decide what to do with the data when converting it into the structure needed to communicate with the database as it will know best and raise an error if it has a problem, otherwise our DBAL is getting into the business of making arbitrary decisions because of the behavior of one adapter.

[HLeithner]

In this case it's my fault I think, I miss interpreted the 4 sqltype parameter. it seams it expects the target column type and not the variable type. Our bind system doesn't provide this information so I removed it.

I think for the bind function I would only check if int is expected and is given or type cast to int if set... but I would prefer to throw an exception in this case.