Suitecrm: Prepared database queries must be used

Created on 28 Mar 2017  路  9Comments  路  Source: salesagility/SuiteCRM

http://php.net/manual/en/mysqli.quickstart.prepared-statements.php

http://php.net/manual/en/pdo.prepared-statements.php

Expected Behavior

prepared queries/statements

Actual Behavior

embedded parameters

Suggestion

Most helpful comment

@gunnicom

  • If you've seen a 50% difference between raw sql and prepared statements, then your database was probably not indexed ideally, or your code maybe not optimally programmed, with the understanding of which calls result in a round trip to the database. When performance is really crucial then stored procedures which use prepared statements can be used for the highest speed. Dealing with this across database types without getting sql sprinkled everywhere in the php code base, is a good reason why to use DBAL.
  • My advice is to always use PDO/prepared statements, or better, DBAL. In cases where they add a marginal performance overhead, they're still worth it for the security - no SQL injection - and readability benefits.
  • The time you may save with one way versus another : 10 milliseconds.
  • The time you may save explaining to your users, how, because you didn't use PDO and missed a few places in your code where you were supposed to escape user input, so your CRM fell victim to hacker bots, and all the users' private personal data ended up on 4chan/wikileaks/some underground russian/chinese hacker crime ring marketplace: PRICELESS.

All 9 comments

Totally agree.

I would not say "must", because there are cases when it is not possible, or only possible with heavy impact on performance, but i would say you are right with "should"

Are you joking about the difference of "must" and "should" ?!
That is the least important thing you "must" worry about in this codebase!
I cannot believe I'm writing about PDO and prepared queries in 2017!

@gunnicom Prepared statements are essential for

  1. security - you must have these for the best anti-hacker defense against SQL injection attacks etc etc.
  2. improve the performance of the CRM - reduced memory usage, and faster time to complete database tasks.
    See also #3325 DBAL is the best light framework around PDO with tremendous added benefits.

@chris001 I know that they are essential for security. But i also know that you are wrong if you say "they are always faster". They are not. There are some cases where i had significant ( about 50% ) slower queries if using prepared statements. So sometimes they are faster, sometimes they are slower.

@gunnicom

  • If you've seen a 50% difference between raw sql and prepared statements, then your database was probably not indexed ideally, or your code maybe not optimally programmed, with the understanding of which calls result in a round trip to the database. When performance is really crucial then stored procedures which use prepared statements can be used for the highest speed. Dealing with this across database types without getting sql sprinkled everywhere in the php code base, is a good reason why to use DBAL.
  • My advice is to always use PDO/prepared statements, or better, DBAL. In cases where they add a marginal performance overhead, they're still worth it for the security - no SQL injection - and readability benefits.
  • The time you may save with one way versus another : 10 milliseconds.
  • The time you may save explaining to your users, how, because you didn't use PDO and missed a few places in your code where you were supposed to escape user input, so your CRM fell victim to hacker bots, and all the users' private personal data ended up on 4chan/wikileaks/some underground russian/chinese hacker crime ring marketplace: PRICELESS.

It's easy enough to include a PDO class and implement it into extended code.

We have now set up a new home for suggestions at Trello. All github issues that were labeled 'suggestion' have been moved and will be closed. Certain ones will be progressed within the new Suggestion Box and may be re-opened.

Announcement of moving Suggestions:
https://suitecrm.com/forum/suggestion-box/13691-moving-suggestions-from-github-to

New SuiteCRM Suggestion Box
https://trello.com/b/Ht7LbMqw/suitecrm-suggestion-box

Was this page helpful?
0 / 5 - 0 ratings