Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Hi all Resuming this once again after Anthony's blog post... On 16/10/2014 18:10, Ferenc Kovacs wrote: On Thu, Oct 16, 2014 at 5:47 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: I do agree that the default should probably be server-side since it is the least surprising. We just need to make it very very clear in the upgrade doc that this change will likely slow down peoples' apps and show them how to turn client-side prepares back on. -Rasmus I don't think we should remove the option, just change the defaults, and most people would be fine switching back to the emulation, but it should be their conscious decision imo. Yes. For the record, pdo_pgsql and pdo_mysql seem to be the only PDO drivers that can effectively emulate prepares. As far as I could tell, the others ignore the attribute. For pgsql the default is to use server-side prepares, so I'd be all for making both work consistently by default. However, I think PDO is fundamentally flawed in enforcing the use of prepared statements even when 99% of the times the queries issued by a PHP script need only to be executed once. It surely is a waste of resources having to allocate a prepared query, execute it and deallocate it each and every time. I certainly wish there was something in the mysql client/protocol similar to PQexecParams, which would be the best of both worlds. Cheers -- Matteo Beccati Development Consulting - http://www.beccati.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 3 November 2014 09:18:08 GMT, Matteo Beccati p...@beccati.com wrote: Hi all Resuming this once again after Anthony's blog post... On 16/10/2014 18:10, Ferenc Kovacs wrote: On Thu, Oct 16, 2014 at 5:47 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: I do agree that the default should probably be server-side since it is the least surprising. We just need to make it very very clear in the upgrade doc that this change will likely slow down peoples' apps and show them how to turn client-side prepares back on. -Rasmus I don't think we should remove the option, just change the defaults, and most people would be fine switching back to the emulation, but it should be their conscious decision imo. Yes. For the record, pdo_pgsql and pdo_mysql seem to be the only PDO drivers that can effectively emulate prepares. As far as I could tell, the others ignore the attribute. For pgsql the default is to use server-side prepares, so I'd be all for making both work consistently by default. However, I think PDO is fundamentally flawed in enforcing the use of prepared statements even when 99% of the times the queries issued by a PHP script need only to be executed once. It surely is a waste of resources having to allocate a prepared query, execute it and deallocate it each and every time. I certainly wish there was something in the mysql client/protocol similar to PQexecParams, which would be the best of both worlds. I think rhis Twitter exchange is very relevant here: https://twitter.com/dracony_gimp/status/528240781996605440?s=09 When using a one-shot call to query() with multiple statements (e.g. create temp table; populate temp table; select results) it's actually necessary with Postgres to switch on emulation, and for that emulation to allow multiple queries even though the DBMS wouldn't. We discovered this in moving from ext/postgres, where pg_query() doesn't attempt to prepare the query, so allows strings containing multiple statements. We therefore needed a compatible mode in order to switch to PDO. -- Rowan Collins [IMSoP] -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 03/11/2014 12:40, Rowan Collins wrote: When using a one-shot call to query() with multiple statements (e.g. create temp table; populate temp table; select results) it's actually necessary with Postgres to switch on emulation, and for that emulation to allow multiple queries even though the DBMS wouldn't. We discovered this in moving from ext/postgres, where pg_query() doesn't attempt to prepare the query, so allows strings containing multiple statements. We therefore needed a compatible mode in order to switch to PDO. Both pg_query and EMULATE_PREPARES in pdo_pgsql use PQexec, that allows multiple statements being sent at once. Going from memory it is also used with PGSQL_DISABLE_PREPARES (5.6+) if there are no bound parameters. Cheers -- Matteo Beccati Development Consulting - http://www.beccati.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 03/11/2014 12:55, Matteo Beccati wrote: [re: PQexec] Going from memory it is also used with PGSQL_DISABLE_PREPARES (5.6+) if there are no bound parameters. Actually, that was true only in the earlier versions of the feature. I've decided not to do that to avoid behaviour changes and unpleasant surprises. Cheers -- Matteo Beccati Development Consulting - http://www.beccati.com/ -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
2014.10.17. 6:51 ezt írta (Rasmus Lerdorf ras...@lerdorf.com): On 10/16/2014 09:10 AM, Ferenc Kovacs wrote: I don't think we should remove the option, just change the defaults, and most people would be fine switching back to the emulation, but it should be their conscious decision imo. Currently many people aren't aware that they are using client side prepares, and they are pretty much ignore the fact, that they can be exposed to sql injections (for example via using mismatching client and server encodings or not properly quoting the identifiers: http://www.codeyellow.nl/identifier-sqli.html because they think that server side prepared statements would be immune to this kind of problems). I think you have the wrong idea here. That link you pointed to talks about SQLi in identifiers. Server-side prepares are just as vulnerable to this, so switching from client-side to server-side does nothing to make this safer. Server side prepares does not support parameter binding for identifiers so while you can still be vulnerable if you concatenate variables into your query, but you wouldn't think that you are immune to sql injection that way. Emulated prepares make it look like that you are. As far as a charset mismatch between the client and the server when it comes to preparing query values, PDO's implementation handles that. You need a connection handle to do a prepare so we know the charset and take that into account. Same thing here, while (since 5.3.6 afair) you are able to pass the encoding information to the pdo driver (before that you could only use set names which pdo doesn't care about) the emulation still make it look like that you are immune, because many people aren't aware of the emulation and assumes that they are immune to sqli because the params travel as separate entity from the query to the server.
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 16/10/14 18:59, christopher jones wrote: The MySQL team has been improving their server-side prepare code: http://mysqlserverteam.com/re-factoring-some-internals-of-prepared-statements-in-5-7/ It may be worth clarifying some terms here since people are talking about restrictions that MySQL creates for itself and which are not normal limitations in other engines. This article identifies the simple fact that MySQL is still playing catchup when it comes to PARAMETRIZED statements. Something which is the norm in other engines. One of the best ways of maintaining security against SQLi IS to pass data as parameters and so anybody trying to be clever by replacing a text value by SQL fails at the first hurdle. Once using parameters then prepare is a required step anyway. It is not optional. It's facets like this which are one of the reasons that PDO is NOT a good base to build on top of as it assumes the lowest common denominator is the norm? Which it has to. Ulf stated early on in this thread re MySQL - statement and parameter are send to the server independently - the server builds the final statement string Is this ACTUALLY how it works? Since other engines prepare the statement and bind buckets to put the parameters in. They never 'build the final statement string' and there is no 'client side prepare' because the concept can't exist. Some other engines may support named parameters via client side juggling, but that just hides the underlying fixed order requirements it does not replace the bind process. PDO has a number of restrictions which prevent facilities available on other engines from being used. Is this just another area where the attempts to make every one seem the same is actually more of a hindrance than a help? Passing a fully assembled SQL statement over the wire should always be a last resort rather than the norm once it's content is provided via external routes. I don't need to call 'prepare' in Firebird, it will be handled as required so having that step carried out by the client side is the waste of traffic. The next version of Firebird will actually start caching prepared statements on persistent connections which I'm just waiting to play with! -- Lester Caine - G8HFL - Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Am 17.10.2014 um 11:51 schrieb Lester Caine: On 16/10/14 18:59, christopher jones wrote: Ulf stated early on in this thread re MySQL - statement and parameter are send to the server independently - the server builds the final statement string Is this ACTUALLY how it works? Since other engines prepare the statement I thought this was a mailing list about PHP. I even believed from the headline the question would be whether PHP users of MySQL would like to change an API default setting. But no, its about explaining the MySQL source code to Firebird lovers. Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 17/10/14 11:17, Ulf Wendel wrote: Am 17.10.2014 um 11:51 schrieb Lester Caine: On 16/10/14 18:59, christopher jones wrote: Ulf stated early on in this thread re MySQL - statement and parameter are send to the server independently - the server builds the final statement string Is this ACTUALLY how it works? Since other engines prepare the statement I thought this was a mailing list about PHP. I even believed from the headline the question would be whether PHP users of MySQL would like to change an API default setting. But no, its about explaining the MySQL source code to Firebird lovers. Since it is the object of PDO to create a level playing field then just how each engine handles the process is what is important so that PHP users know what they are getting and where the real security holes are. ATTR_EMULATE_PREPARES may well be a potential security hole and having to live with sites that have adopted PDO_Mysql I'd like to understand just what the process between PDO and MySQL is so I know if I have to worry or not. Yes it may affect if I take the time to switch those sites from MySQL, and maintaining them is complicated by the level of 'attack' instigated trying to find the weaknesses, so if you switch this off do I need simply to switch it back on, or take other action. -- Lester Caine - G8HFL - Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Am 17.10.2014 um 13:47 schrieb Lester Caine: users know what they are getting and where the real security holes are. Hmm, maybe, you could make this world a better one by contributing to improve http://php.net/manual/en/pdo.prepared-statements.php ? For the rest: the MySQL manual and the MySQL source code have the details. No need for speculation towards the implementation of server-side PS in MySQL, no need playing my DB vs. your DB, no need for playing the users have a right card. Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 17/10/14 13:20, Ulf Wendel wrote: users know what they are getting and where the real security holes are. Hmm, maybe, you could make this world a better one by contributing to improve http://php.net/manual/en/pdo.prepared-statements.php ? PDO does not support management of SQL differences between databases. This page is a good example of where users run into problems because they have no idea if what they are copying actually works on their particular database. Does MySQL need ATTR_EMULATE_PREPARES in order to convert client side the SQL that it feeds over to the server? If I am converting from one database to another just what is actually supported and how? I don't use PDO with Firebird if I can help it but I am having to work with this where mysql hosting is the norm and PDO_mysql is an alternative that gets provided instead of mysqli. *I* have trouble sorting this stuff out so how do users who currently have working sites cope when things under the hood change perhaps without them even knowing. I can quite happily add notes as to what Firebird does with the various abstractions on that page, but what about every other PDO driver. Which emulate aspects of the prepares and which do it natively? Just what does get emulated? -- Lester Caine - G8HFL - Contact - http://lsces.co.uk/wiki/?page=contact L.S.Caine Electronic Services - http://lsces.co.uk EnquirySolve - http://enquirysolve.com/ Model Engineers Digital Workshop - http://medw.co.uk Rainbow Digital Media - http://rainbowdigitalmedia.co.uk -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Am 17.10.2014 um 15:09 schrieb Lester Caine: On 17/10/14 13:20, Ulf Wendel wrote: users know what they are getting and where the real security holes are. Hmm, maybe, you could make this world a better one by contributing to improve http://php.net/manual/en/pdo.prepared-statements.php ? PDO does not support management of SQL differences between databases. This page is a good example of where users run into problems because they have no idea if what they are copying actually works on their To me, you are making noise on the wrong list and the wrong discussion thread: file a documentation bug report. Wait, I did that for you: https://bugs.php.net/bug.php?id=68254 . Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On Fri, Jun 15, 2012 at 3:01 AM, Anthony Ferrara ircmax...@gmail.com wrote: Hello all, I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. However, Rasmus did mention that it was a possibility for 5.4 ( http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that ship has sailed, I submitted a pull request for trunk to change the default value of prepared statement emulation for MySQL. https://github.com/php/php-src/pull/108 https://bugs.php.net/bug.php?id=54638 Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? Thanks, Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php hi, do we want to change the default for this in PHP7? -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 10/16/2014 04:27 AM, Ferenc Kovacs wrote: On Fri, Jun 15, 2012 at 3:01 AM, Anthony Ferrara ircmax...@gmail.com wrote: Hello all, I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. However, Rasmus did mention that it was a possibility for 5.4 ( http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that ship has sailed, I submitted a pull request for trunk to change the default value of prepared statement emulation for MySQL. https://github.com/php/php-src/pull/108 https://bugs.php.net/bug.php?id=54638 Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? Thanks, Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php hi, do we want to change the default for this in PHP7? Honestly, I am not sure about this anymore. There is a significant performance benefit in doing client-side prepares. Last year I attempted to switch to server-side prepares on Etsy's production servers but it added 30-40ms of page latency because of the extra round trips. And yes, we were doing too many queries, but I fear if we change this default people won't understand where this slowdown is coming from. Of course, in some rare cases using server-side prepares might speed things up because of prepared statement caching in the server, but I have yet to see a case where that caching outweighs the extra tcp roundtrip overhead. I do agree that the default should probably be server-side since it is the least surprising. We just need to make it very very clear in the upgrade doc that this change will likely slow down peoples' apps and show them how to turn client-side prepares back on. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On Thu, Oct 16, 2014 at 5:47 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: On 10/16/2014 04:27 AM, Ferenc Kovacs wrote: On Fri, Jun 15, 2012 at 3:01 AM, Anthony Ferrara ircmax...@gmail.com wrote: Hello all, I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. However, Rasmus did mention that it was a possibility for 5.4 ( http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that ship has sailed, I submitted a pull request for trunk to change the default value of prepared statement emulation for MySQL. https://github.com/php/php-src/pull/108 https://bugs.php.net/bug.php?id=54638 Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? Thanks, Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php hi, do we want to change the default for this in PHP7? Honestly, I am not sure about this anymore. There is a significant performance benefit in doing client-side prepares. Last year I attempted to switch to server-side prepares on Etsy's production servers but it added 30-40ms of page latency because of the extra round trips. And yes, we were doing too many queries, but I fear if we change this default people won't understand where this slowdown is coming from. Of course, in some rare cases using server-side prepares might speed things up because of prepared statement caching in the server, but I have yet to see a case where that caching outweighs the extra tcp roundtrip overhead. I do agree that the default should probably be server-side since it is the least surprising. We just need to make it very very clear in the upgrade doc that this change will likely slow down peoples' apps and show them how to turn client-side prepares back on. -Rasmus I don't think we should remove the option, just change the defaults, and most people would be fine switching back to the emulation, but it should be their conscious decision imo. Currently many people aren't aware that they are using client side prepares, and they are pretty much ignore the fact, that they can be exposed to sql injections (for example via using mismatching client and server encodings or not properly quoting the identifiers: http://www.codeyellow.nl/identifier-sqli.html because they think that server side prepared statements would be immune to this kind of problems). I think it would be better to change the default in a major version, if the tradeoff is performance vs security, and if they think that they are okay with the emulation, they can change it back. ps: don't forget that some/many of our users are still using php on a single server setup, where the mysql connection is done through a unix socket instead of the network stack, so the roundtrip there will be even less noticable, and usually those are the kind of users, who need safe defaults because they can't afford to be aware or change settings/code for their apps. -- Ferenc Kovács @Tyr43l - http://tyrael.hu
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Le jeudi 16 octobre 2014 à 18:10 +0200, Ferenc Kovacs a écrit :
On Thu, Oct 16, 2014 at 5:47 PM, Rasmus Lerdorf ras...@lerdorf.com wrote:
On 10/16/2014 04:27 AM, Ferenc Kovacs wrote:
On Fri, Jun 15, 2012 at 3:01 AM, Anthony Ferrara ircmax...@gmail.com
wrote:
Hello all,
I raised this topic on list over a year ago (
http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was
determined that it wasn't time yet to disable prepared statement
emulation for MySQL yet. However, Rasmus did mention that it was a
possibility for 5.4 (
http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that
ship has sailed, I submitted a pull request for trunk to change the
default value of prepared statement emulation for MySQL.
https://github.com/php/php-src/pull/108
https://bugs.php.net/bug.php?id=54638
Does this need to be an RFC (should I draft one)? Or can it just be
pulled as-is?
Thanks,
Anthony
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
hi,
do we want to change the default for this in PHP7?
Honestly, I am not sure about this anymore. There is a significant
performance benefit in doing client-side prepares. Last year I attempted
to switch to server-side prepares on Etsy's production servers but it
added 30-40ms of page latency because of the extra round trips. And yes,
we were doing too many queries, but I fear if we change this default
people won't understand where this slowdown is coming from.
Of course, in some rare cases using server-side prepares might speed
things up because of prepared statement caching in the server, but I
have yet to see a case where that caching outweighs the extra tcp
roundtrip overhead.
I do agree that the default should probably be server-side since it is
the least surprising. We just need to make it very very clear in the
upgrade doc that this change will likely slow down peoples' apps and
show them how to turn client-side prepares back on.
-Rasmus
I don't think we should remove the option, just change the defaults, and
most people would be fine switching back to the emulation, but it should be
their conscious decision imo.
Currently many people aren't aware that they are using client side
prepares, and they are pretty much ignore the fact, that they can be
exposed to sql injections (for example via using mismatching client and
server encodings or not properly quoting the identifiers:
http://www.codeyellow.nl/identifier-sqli.html because they think that
server side prepared statements would be immune to this kind of problems).
I think it would be better to change the default in a major version, if the
tradeoff is performance vs security, and if they think that they are okay
with the emulation, they can change it back.
ps:
don't forget that some/many of our users are still using php on a single
server setup, where the mysql connection is done through a unix socket
instead of the network stack, so the roundtrip there will be even less
noticable, and usually those are the kind of users, who need safe defaults
because they can't afford to be aware or change settings/code for their
apps.
Hi,
for me there is two usage of prepared statements :
#1 : safely handle variables
#2 : performance in batch traitments
The first one want emulated prepared statments and the second need true
prepared.
It's possible to add a simplier method for the first one case, and let
prepare() do a true prepare by default.
For example here we extends PDO to add the q() shortcut.
This shortcut basicaly do that :
public function q($statement, array $params)
{
$stmt = $this-prepare($statement, [PDO::MYSQL_ATTR_DIRECT_QUERY = true]);
return $stmt-execute($params);
}
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 10/16/14, 8:47 AM, Rasmus Lerdorf wrote: On 10/16/2014 04:27 AM, Ferenc Kovacs wrote: On Fri, Jun 15, 2012 at 3:01 AM, Anthony Ferrara ircmax...@gmail.com wrote: Hello all, I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. However, Rasmus did mention that it was a possibility for 5.4 ( http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that ship has sailed, I submitted a pull request for trunk to change the default value of prepared statement emulation for MySQL. https://github.com/php/php-src/pull/108 https://bugs.php.net/bug.php?id=54638 Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? Thanks, Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php hi, do we want to change the default for this in PHP7? Honestly, I am not sure about this anymore. There is a significant performance benefit in doing client-side prepares. Last year I attempted to switch to server-side prepares on Etsy's production servers but it added 30-40ms of page latency because of the extra round trips. And yes, we were doing too many queries, but I fear if we change this default people won't understand where this slowdown is coming from. Of course, in some rare cases using server-side prepares might speed things up because of prepared statement caching in the server, but I have yet to see a case where that caching outweighs the extra tcp roundtrip overhead. I do agree that the default should probably be server-side since it is the least surprising. We just need to make it very very clear in the upgrade doc that this change will likely slow down peoples' apps and show them how to turn client-side prepares back on. -Rasmus The MySQL team has been improving their server-side prepare code: http://mysqlserverteam.com/re-factoring-some-internals-of-prepared-statements-in-5-7/ Chris -- christopher.jo...@oracle.com http://twitter.com/ghrd -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 16 October 2014 18:39:57 GMT+01:00, Olivier Bonvalet php-dev.l...@daevel.net wrote: Le jeudi 16 octobre 2014 à 18:10 +0200, Ferenc Kovacs a écrit : On Thu, Oct 16, 2014 at 5:47 PM, Rasmus Lerdorf ras...@lerdorf.com wrote: On 10/16/2014 04:27 AM, Ferenc Kovacs wrote: On Fri, Jun 15, 2012 at 3:01 AM, Anthony Ferrara ircmax...@gmail.com wrote: Hello all, I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. However, Rasmus did mention that it was a possibility for 5.4 ( http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that ship has sailed, I submitted a pull request for trunk to change the default value of prepared statement emulation for MySQL. https://github.com/php/php-src/pull/108 https://bugs.php.net/bug.php?id=54638 Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? Thanks, Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php hi, do we want to change the default for this in PHP7? Honestly, I am not sure about this anymore. There is a significant performance benefit in doing client-side prepares. Last year I attempted to switch to server-side prepares on Etsy's production servers but it added 30-40ms of page latency because of the extra round trips. And yes, we were doing too many queries, but I fear if we change this default people won't understand where this slowdown is coming from. Of course, in some rare cases using server-side prepares might speed things up because of prepared statement caching in the server, but I have yet to see a case where that caching outweighs the extra tcp roundtrip overhead. I do agree that the default should probably be server-side since it is the least surprising. We just need to make it very very clear in the upgrade doc that this change will likely slow down peoples' apps and show them how to turn client-side prepares back on. -Rasmus I don't think we should remove the option, just change the defaults, and most people would be fine switching back to the emulation, but it should be their conscious decision imo. Currently many people aren't aware that they are using client side prepares, and they are pretty much ignore the fact, that they can be exposed to sql injections (for example via using mismatching client and server encodings or not properly quoting the identifiers: http://www.codeyellow.nl/identifier-sqli.html because they think that server side prepared statements would be immune to this kind of problems). I think it would be better to change the default in a major version, if the tradeoff is performance vs security, and if they think that they are okay with the emulation, they can change it back. ps: don't forget that some/many of our users are still using php on a single server setup, where the mysql connection is done through a unix socket instead of the network stack, so the roundtrip there will be even less noticable, and usually those are the kind of users, who need safe defaults because they can't afford to be aware or change settings/code for their apps. Hi, for me there is two usage of prepared statements : #1 : safely handle variables #2 : performance in batch traitments The first one want emulated prepared statments and the second need true prepared. Surely this is the other way around? Emulation has imperfect security, because it is basically a smarter version of magic quotes, e.g. the character encoding mismatch already mentioned. It's only advantage over real prepares is performance, if network latency is a significant factor. -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 10/16/2014 02:38 PM, Morgan Tocker wrote: The more clear cases would be: - The prepared statement is reused enough times that the initial extra round trip can be amortized. - If large place-holder values can be sent direct to the server without having to be escaped or parsed. For example reading a file directly into the contents of a column to insert. Other than large batch backfill jobs it is not very common to issue the same query enough times to overcome the connection overhead. At least not in code I have worked with. But I am glad to see that there is work being done on it. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 10/16/2014 09:10 AM, Ferenc Kovacs wrote: I don't think we should remove the option, just change the defaults, and most people would be fine switching back to the emulation, but it should be their conscious decision imo. Currently many people aren't aware that they are using client side prepares, and they are pretty much ignore the fact, that they can be exposed to sql injections (for example via using mismatching client and server encodings or not properly quoting the identifiers: http://www.codeyellow.nl/identifier-sqli.html because they think that server side prepared statements would be immune to this kind of problems). I think you have the wrong idea here. That link you pointed to talks about SQLi in identifiers. Server-side prepares are just as vulnerable to this, so switching from client-side to server-side does nothing to make this safer. As far as a charset mismatch between the client and the server when it comes to preparing query values, PDO's implementation handles that. You need a connection handle to do a prepare so we know the charset and take that into account. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Hi, 2012/6/28 Rasmus Lerdorf ras...@lerdorf.com: On 06/27/2012 08:45 PM, Yasuo Ohgaki wrote: Hi, I forgot to mention MySQL's query cache. This change may have negative performance impact, since prepared query is not cached and native prepared query may not be used by other requests. That's not really true anymore. There are some limitations, yes, but in general prepared statements are cached since MySQL version 5.1.21 Good to know! Thank you. -- Yasuo Ohgaki -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Hi, I forgot to mention MySQL's query cache. This change may have negative performance impact, since prepared query is not cached and native prepared query may not be used by other requests. It would be nice to have an option keeping prepared statement when PDOStatement destroyed. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net 2012/6/20 Yasuo Ohgaki yohg...@ohgaki.net: Hi, I'm not opposed to this change, but I would like to give some ideas. If I use PDO postgresql and prepared statement, it executes native prepared query. However, prepared statement is released(removed) when PDOStatement object is destroyed. If PDO MySQL made like this, performance will not increase much unless app issues the same SQL query over and over during request. It would be nice to have option for not to release prepared query for better performance. BTW, PDO should have method for setting client/database encoding. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net 2012/6/15 Anthony Ferrara ircmax...@gmail.com: Hello all, I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. However, Rasmus did mention that it was a possibility for 5.4 ( http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that ship has sailed, I submitted a pull request for trunk to change the default value of prepared statement emulation for MySQL. https://github.com/php/php-src/pull/108 https://bugs.php.net/bug.php?id=54638 Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? Thanks, Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 06/27/2012 08:45 PM, Yasuo Ohgaki wrote: Hi, I forgot to mention MySQL's query cache. This change may have negative performance impact, since prepared query is not cached and native prepared query may not be used by other requests. That's not really true anymore. There are some limitations, yes, but in general prepared statements are cached since MySQL version 5.1.21 -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On Thu, 2012-06-21 at 22:35 -0400, Anthony Ferrara wrote:
Actually, the more I think about it, this would result in an
inadvertent API change. Right now, if you have a SQL syntax error, the
error would be thrown by -execute(). But with this change, the error
would be thrown by -prepare(). So code that currently wraps execute()
but not prepare() with a try{}catch(){} would start failing.
Is this a show stopper? Or is restoring the proper execution path worth it?
From what I can see in the few open source drivers I looked at, it
won't be an issue (Drupal, Doctrine, Zend, Lithium, etc)...
Well, one can easily argue that an error during prepare is the correct
behavior, so this could actually be seen as a bug fix.
The interesting question is whether PDO's emulation allows placeholders
in places where the MySQL server doesn't in old servers (5.0?) a case
was
SELECT * FROM t LIMIT ?
which was invalid in MySQL but valid with PDO when binding as integer.
This would be a broken feature. On some quick(!) tests I didn't come up
with a case where this was relevant.
On a related note, as I wrote in a previous mail (which Ulf mentioned)
there are still statements which can't be prepared in MySQL. One example
is ALTER DATABASE. The MySQL documentation unfortunately has only a
positive list[1] which makes it a it hard to find the missing ones ...
anyways, PDO then tries to fallback to emulation, but there seems to be
a bug:
?php
$p = new PDO(mysql:host=localhost,root);
$p-setAttribute(PDO::ATTR_EMULATE_PREPARES, false);
$p-setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
var_dump($p-query(ALTER DATABASE foo CHARACTER SET = utf8));
?
Fatal error: Uncaught exception 'PDOException' with message
'SQLSTATE[HY000]: General error: 2014 Cannot execute queries
while other unbuffered queries are active. Consider using
PDOStatement::fetchAll(). Alternatively, if your code is only
ever going to run against mysql, you may enable query buffering
by setting the PDO::MYSQL_ATTR_USE_BUFFERED_QUERY attribute.'
I will try to look into the bug (unless anybody else is faster) but it
shows that that area needs more testing, too.
If you're still running 5.1 the list of prepareable statements is much
shorter[2]. 5.1 currently is still quite common as many distributions
bundle it, but I assume it's no big deal for php-master as it will take
sometime till php-master is released and reaches the people and by then
most are hopefully on 5.5 or even 5.6.
johannes
[1] http://dev.mysql.com/doc/refman/5.5/en/sql-syntax-prepared-statements.html
[2] http://dev.mysql.com/doc/refman/5.1/en/sql-syntax-prepared-statements.html
--
Johannes Schlüter, MySQL Engineering, ORACLE Deutschland B.V. Co KG
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Am 20.06.2012 08:39, schrieb Pierre Joye: hi Chris, On Tue, Jun 19, 2012 at 11:45 PM, Christopher Jones christopher.jo...@oracle.com wrote: We should take this offline - I can see cases where I'd strongly disagree. I see no reason to move a discussion offline or off list, this is a topic that interests many other readers or developers. Agreed. There was enough trouble around PDO and discussions going on in the hidden. However, this does not impact the original topic. Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
As it turns out, the testing of this is far more difficult than I
originally suspected. Not because it fails, but because emulated
queries were behaving badly to begin with, so a number of tests were
testing bad behavior. For example, bug 44327:
https://github.com/ircmaxell/php-src/blob/master/ext/pdo_mysql/tests/bug44327.phpt
It runs the following code:
$stmt = $db-prepare('foo');
@$stmt-execute();
$row = $stmt-fetch();
var_dump($row-queryString);
And expects:
Notice: Trying to get property of non-object in %s on line %d
NULL
Whereas the proper behavior is for the syntax error to be thrown from
`prepare`. PDO emulation doesn't do that, because it doesn't parse
until `execute`, hence the error delays until that point.
When I run it, I get:
Warning: PDO::prepare(): SQLSTATE[42000]: Syntax error or access
violation: 1064 You have an error in your SQL syntax; check the manual
that corresponds to your MySQL server version for the right syntax to
use near 'foo' at line 1 in %s on line %d
Because it's sent back at prepare() instead of at execute (and prepare
doesn't have an @).
There are at least a few more failures of this nature.
To fix this, is going to take some significant refactoring of a number
of tests. Plus, without this patch, I'm still getting 2 warnings and 6
failures:
=
Number of tests : 166 157
Tests skipped :9 ( 5.4%)
Tests warned:2 ( 1.2%) ( 1.3%)
Tests failed:6 ( 3.6%) ( 3.8%)
Expected fail :1 ( 0.6%) ( 0.6%)
Tests passed: 148 ( 89.2%) ( 94.3%)
-
Time taken : 43 seconds
=
=
EXPECTED FAILED TEST SUMMARY
-
PECL Bug #7976 (Calling stored procedure several times)
[ext/pdo_mysql/tests/bug_pecl_7976.phpt] XFAIL REASON: Works with
mysqlnd. It is not supported by libmysql. For libmysql is good enough
to see no crash.
=
=
FAILED TEST SUMMARY
-
Bug #39858 (Lost connection to MySQL server during query by a repeated
call stored proced) [ext/pdo_mysql/tests/bug_39858.phpt]
PDO MySQL Bug #41997 (stored procedure call returning single rowset
blocks future queries) [ext/pdo_mysql/tests/bug_41997.phpt]
MySQL PDO-__construct() - Generic + DSN
[ext/pdo_mysql/tests/pdo_mysql___construct.phpt]
MySQL PDO-exec(), affected rows
[ext/pdo_mysql/tests/pdo_mysql_exec_load_data.phpt]
MySQL PDOStatement-nextRowSet()
[ext/pdo_mysql/tests/pdo_mysql_stmt_nextrowset.phpt]
MySQL Prepared Statements and different column counts
[ext/pdo_mysql/tests/pdo_mysql_stmt_variable_columncount.phpt]
=
=
WARNED TEST SUMMARY
-
Bug #44454 (Unexpected exception thrown in foreach() statement)
[ext/pdo_mysql/tests/bug_44454.phpt] (warn: XFAIL section but test
passes)
MySQL PDO-prepare(), emulated PS
[ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt] (warn: XFAIL
section but test passes)
=
Whereas with the patch, there are a few more failures:
=
Number of tests : 166 157
Tests skipped :9 ( 5.4%)
Tests warned:2 ( 1.2%) ( 1.3%)
Tests failed: 18 ( 10.8%) ( 11.5%)
Expected fail :1 ( 0.6%) ( 0.6%)
Tests passed: 136 ( 81.9%) ( 86.6%)
-
Time taken : 53 seconds
=
So to do this, I'd need to fix those as well... (at least all but the XFAIL)
So I'm going to keep at it, but it's going to take a while (and would
like some insight into if this is the best approach to fixing the
tests)...
Thanks,
Anthony
On Thu, Jun 21, 2012 at 2:57 AM, Ulf Wendel ulf.wen...@phpdoc.de wrote:
Am 20.06.2012 08:39, schrieb Pierre Joye:
hi Chris,
On Tue, Jun 19, 2012 at 11:45 PM, Christopher Jones
christopher.jo...@oracle.com wrote:
We should take this offline - I can see cases where I'd strongly
disagree.
I see no reason to move a discussion offline or off list, this is a
topic that interests many other readers or developers.
Agreed. There was enough trouble around PDO and discussions going on in the
hidden.
However, this does not impact the original topic.
Ulf
--
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Actually, the more I think about it, this would result in an
inadvertent API change. Right now, if you have a SQL syntax error, the
error would be thrown by -execute(). But with this change, the error
would be thrown by -prepare(). So code that currently wraps execute()
but not prepare() with a try{}catch(){} would start failing.
Is this a show stopper? Or is restoring the proper execution path worth it?
From what I can see in the few open source drivers I looked at, it
won't be an issue (Drupal, Doctrine, Zend, Lithium, etc)...
Thanks,
Anthony
On Thu, Jun 21, 2012 at 10:19 PM, Anthony Ferrara ircmax...@gmail.com wrote:
As it turns out, the testing of this is far more difficult than I
originally suspected. Not because it fails, but because emulated
queries were behaving badly to begin with, so a number of tests were
testing bad behavior. For example, bug 44327:
https://github.com/ircmaxell/php-src/blob/master/ext/pdo_mysql/tests/bug44327.phpt
It runs the following code:
$stmt = $db-prepare('foo');
@$stmt-execute();
$row = $stmt-fetch();
var_dump($row-queryString);
And expects:
Notice: Trying to get property of non-object in %s on line %d
NULL
Whereas the proper behavior is for the syntax error to be thrown from
`prepare`. PDO emulation doesn't do that, because it doesn't parse
until `execute`, hence the error delays until that point.
When I run it, I get:
Warning: PDO::prepare(): SQLSTATE[42000]: Syntax error or access
violation: 1064 You have an error in your SQL syntax; check the manual
that corresponds to your MySQL server version for the right syntax to
use near 'foo' at line 1 in %s on line %d
Because it's sent back at prepare() instead of at execute (and prepare
doesn't have an @).
There are at least a few more failures of this nature.
To fix this, is going to take some significant refactoring of a number
of tests. Plus, without this patch, I'm still getting 2 warnings and 6
failures:
=
Number of tests : 166 157
Tests skipped : 9 ( 5.4%)
Tests warned : 2 ( 1.2%) ( 1.3%)
Tests failed : 6 ( 3.6%) ( 3.8%)
Expected fail : 1 ( 0.6%) ( 0.6%)
Tests passed : 148 ( 89.2%) ( 94.3%)
-
Time taken : 43 seconds
=
=
EXPECTED FAILED TEST SUMMARY
-
PECL Bug #7976 (Calling stored procedure several times)
[ext/pdo_mysql/tests/bug_pecl_7976.phpt] XFAIL REASON: Works with
mysqlnd. It is not supported by libmysql. For libmysql is good enough
to see no crash.
=
=
FAILED TEST SUMMARY
-
Bug #39858 (Lost connection to MySQL server during query by a repeated
call stored proced) [ext/pdo_mysql/tests/bug_39858.phpt]
PDO MySQL Bug #41997 (stored procedure call returning single rowset
blocks future queries) [ext/pdo_mysql/tests/bug_41997.phpt]
MySQL PDO-__construct() - Generic + DSN
[ext/pdo_mysql/tests/pdo_mysql___construct.phpt]
MySQL PDO-exec(), affected rows
[ext/pdo_mysql/tests/pdo_mysql_exec_load_data.phpt]
MySQL PDOStatement-nextRowSet()
[ext/pdo_mysql/tests/pdo_mysql_stmt_nextrowset.phpt]
MySQL Prepared Statements and different column counts
[ext/pdo_mysql/tests/pdo_mysql_stmt_variable_columncount.phpt]
=
=
WARNED TEST SUMMARY
-
Bug #44454 (Unexpected exception thrown in foreach() statement)
[ext/pdo_mysql/tests/bug_44454.phpt] (warn: XFAIL section but test
passes)
MySQL PDO-prepare(), emulated PS
[ext/pdo_mysql/tests/pdo_mysql_prepare_emulated.phpt] (warn: XFAIL
section but test passes)
=
Whereas with the patch, there are a few more failures:
=
Number of tests : 166 157
Tests skipped : 9 ( 5.4%)
Tests warned : 2 ( 1.2%) ( 1.3%)
Tests failed : 18 ( 10.8%) ( 11.5%)
Expected fail : 1 ( 0.6%) ( 0.6%)
Tests passed : 136 ( 81.9%) ( 86.6%)
-
Time taken : 53 seconds
=
So to do this, I'd need to fix those as well... (at least all but the XFAIL)
So I'm going to keep at it,
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
hi Chris, On Tue, Jun 19, 2012 at 11:45 PM, Christopher Jones christopher.jo...@oracle.com wrote: We should take this offline - I can see cases where I'd strongly disagree. I see no reason to move a discussion offline or off list, this is a topic that interests many other readers or developers. Cheers, -- Pierre @pierrejoye | http://blog.thepimp.net | http://www.libgd.org -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 06/16/2012 12:19 AM, Ulf Wendel wrote: Am 15.06.2012 18:28, schrieb Christopher Jones: On 06/15/2012 08:34 AM, Ulf Wendel wrote: As long as client-side escaping is done properly, there is no practical difference between the [client vs server -prepare] approaches. The big problem with this line of reasoning is that the client must know exactly the same dialect of SQL/XQUERY/whatever that the server does. Since we can't predict the future, and so a new DB might Plain wrong. If client does not mess up on type and charsets there is no practical difference between the security of properly done client side escaping and server-side escaping. No matter if the subject of escaping is a fairy tale on goofy or any other string that happens to look like any other human invented format, e.g. SQL. Ulf We should take this offline - I can see cases where I'd strongly disagree. Chris -- christopher.jo...@oracle.com http://twitter.com/#!/ghrd -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Hi, I'm not opposed to this change, but I would like to give some ideas. If I use PDO postgresql and prepared statement, it executes native prepared query. However, prepared statement is released(removed) when PDOStatement object is destroyed. If PDO MySQL made like this, performance will not increase much unless app issues the same SQL query over and over during request. It would be nice to have option for not to release prepared query for better performance. BTW, PDO should have method for setting client/database encoding. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net 2012/6/15 Anthony Ferrara ircmax...@gmail.com: Hello all, I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. However, Rasmus did mention that it was a possibility for 5.4 ( http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that ship has sailed, I submitted a pull request for trunk to change the default value of prepared statement emulation for MySQL. https://github.com/php/php-src/pull/108 https://bugs.php.net/bug.php?id=54638 Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? Thanks, Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Am 15.06.2012 18:28, schrieb Christopher Jones: On 06/15/2012 08:34 AM, Ulf Wendel wrote: As long as client-side escaping is done properly, there is no practical difference between the [client vs server -prepare] approaches. The big problem with this line of reasoning is that the client must know exactly the same dialect of SQL/XQUERY/whatever that the server does. Since we can't predict the future, and so a new DB might Plain wrong. If client does not mess up on type and charsets there is no practical difference between the security of properly done client side escaping and server-side escaping. No matter if the subject of escaping is a fairy tale on goofy or any other string that happens to look like any other human invented format, e.g. SQL. Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Am 15.06.2012 03:01, schrieb Anthony Ferrara: I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. However, Rasmus did mention that it was a possibility for 5.4 ( http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that ship has sailed, I submitted a pull request for trunk to change the default value of prepared statement emulation for MySQL. https://github.com/php/php-src/pull/108 https://bugs.php.net/bug.php?id=54638 Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? Please, be aware of the consequences of this move and don't break any tests. Also, please, do no break it down to prepared statement === security as any such _short_ statement draws an incomplete picture and bares more risks than it does good. Back then, Johannes already pointed out some consequences: http://marc.info/?l=php-internalsm=130423522001744w=2 . Here's another iteration on the topic. There are assorted others in my blog from the days PDO_MySQL has seen its last major update. *Security* It is claimed that native prepared statements are more secure because: - statement and parameter are send to the server independently - the server builds the final statement string Whereas for a non-prepared statement: - the client builds the final statement string - the client has to escape parameter values - the client sends statement and parameter in one As long as client-side escaping is done properly, there is no practical difference between the two approaches. There are, however, arguments that boil down to limiting the possiblity of user errors: - bind-style API - messing up on charsets A bind-style API may be convenient and prevent forgetting to use escaping but does not prevent to shoot yourself doing something foolish such as: PDO::query(... . $_REQUEST['killme']); When using non-prepared statements it must be ensured to use the correct charset for (client-side) escaping. As long as charset changes happen through API calls, the client library is always aware of the current charset and everything is fine. But, the charset can be changes through SQL as well: SET NAMES ... The fact that PDO lacks API calls to set the charset, which enforces the use of SET NAMES, is not helpful. However, even with appropriate API calls users can always fool themselves using: PDO::query(SET NAMES) If building the final statement is done on the server or SET NAMES is disallowed, fooling yourself becomes much harder. *Security in the context of PDO* Actually, the change of the default can give you a false feeling of improved security. This is due to the design of PDO and the existance of a prepared statement emulation in the core of PDO. A statement like this will use the prepared statement emulation: SELECT something FROM table WHERE cond = :placeholder MySQL does not support named bind parameter such as :placeholder. The PDO parser kicks in and does replace :placeholder with the bound value. Which then, is no different from: sprintf(SELECT something FROM table WHERE cond = '%s', escape($param)) Of course, escape() can't be forgotten. It happens inside PHP, inside PDO on the C level. Messing up with charsets is still possible. Thus, users must be educated as ever since. The message prepared statement === security is too short to tell the full story. BTW, there's the classic of: SELECT something FROM table LIMIT :placeholder What will happen if forget to hint a data type for :placeholer? Search the bug database, if you can't answer. As you are at it, try the reverse with ? and another database system that does not support ? as a placeholder... *Performance (and impact on server load)* After switching to native prepared statements users may see an increase of MySQL - PHP round trips: prepare() client -- server execute() client -- server Great, if statements get executed multiple times. Bad, if statements are not executed multiple times, such as: SELECT title, received FROM news WHERE = SELECT name, price, special_price FROM specials Those statements will take two round trips, thus become slower. Things become slower although there is no other win. Note, that none of the example statements has any parameters. A statement such as: INSERT INTO test(col) VALUES (?) May become faster if multiple rows are inserted. The statement string is transferred only once during prepare. Later only the parameter value is transferred. But, of course, you'd use MySQL multi-insert syntax or other tricks here anyway, wouldn't you? Back in the 70's, when prepared statements have been introduced, the world was a different one. Prepared statements make the assumption that its benefitial to cache various things, such as query execution plans, inside the database server.
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Am 15.06.2012 03:01, schrieb Anthony Ferrara: I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? It needs an RFC because it needs to document whether or not the other DB drivers should also be changed. On 06/15/2012 08:34 AM, Ulf Wendel wrote: As long as client-side escaping is done properly, there is no practical difference between the [client vs server -prepare] approaches. The big problem with this line of reasoning is that the client must know exactly the same dialect of SQL/XQUERY/whatever that the server does. Since we can't predict the future, and so a new DB might introduce some funky syntax, then client preparing could break visibly or invisibly. The DB client DB server code architecture needs to head to a model where server side prepares work (and are easy etc). Chris -- christopher.jo...@oracle.com http://twitter.com/#!/ghrd -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Chris, Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? It needs an RFC because it needs to document whether or not the other DB drivers should also be changed. It's a PDO driver-specific change. So to me it's fairly straight forward to see that no other drivers need changing... It doesn't even hit the mysql API level... Not saying an RFC isn't needed, just that justification isn't clear to me... The big problem with this line of reasoning is that the client must know exactly the same dialect of SQL/XQUERY/whatever that the server does. Since we can't predict the future, and so a new DB might introduce some funky syntax, then client preparing could break visibly or invisibly. The DB client DB server code architecture needs to head to a model where server side prepares work (and are easy etc). We're talking about the MySQL specific API, not all APIs. We're not talking about removing emulation support either. Just disabling it by default for MySQL just like it is for almost every other driver supported by PDO... Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
On 06/15/2012 09:36 AM, Anthony Ferrara wrote: Chris, Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? It needs an RFC because it needs to document whether or not the other DB drivers should also be changed. It's a PDO driver-specific change. So to me it's fairly straight forward to see that no other drivers need changing... It doesn't even hit the mysql API level... Not saying an RFC isn't needed, just that justification isn't clear to me... PDO is supposed to be a generic layer. An RFC can clarify the project scope and examine what happens in other drivers. Chris -- christopher.jo...@oracle.com http://twitter.com/#!/ghrd -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Am 15.06.2012 18:36, schrieb Anthony Ferrara: Chris, Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? It needs an RFC because it needs to document whether or not the other DB drivers should also be changed. It's a PDO driver-specific change. So to me it's fairly straight forward to see that no other drivers need changing... It doesn't even hit the mysql API level... Not saying an RFC isn't needed, just that justification isn't clear to me... Go commit if you feel like the change is widely accepted, you are willing to explain the differences to users and no test breaks. After all its a tiny, little PDO driver default setting. Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Ulf, It needs an RFC because it needs to document whether or not the other DB drivers should also be changed. It's a PDO driver-specific change. So to me it's fairly straight forward to see that no other drivers need changing... It doesn't even hit the mysql API level... Not saying an RFC isn't needed, just that justification isn't clear to me... Go commit if you feel like the change is widely accepted, you are willing to explain the differences to users and no test breaks. After all its a tiny, little PDO driver default setting. I think you interpreted me wrong. I was just pointing out that this change would be localized to the PDO_MySQL driver. And that documenting whether other DB drivers should be changed is a moot point, since they already aren't using emulation by default... And where did the passive-agressive thing hit off? I posted it to list as a question and a suggestion. I didn't say let's just do it, who cares. I didn't say anything about any of the points that were raised, I just wanted to clarify something that I thought was miscommunicated. You brought up some very valid points. Points that shouldn't be ignored. I just haven't addressed them because I haven't wrapped my head around them fully yet... So in short, can we please take it down a notch? Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Am 15.06.2012 19:31, schrieb Anthony Ferrara: Ulf, It needs an RFC because it needs to document whether or not the other DB drivers should also be changed. It's a PDO driver-specific change. So to me it's fairly straight forward to see that no other drivers need changing... It doesn't even hit the mysql API level... Not saying an RFC isn't needed, just that justification isn't clear to me... Go commit if you feel like the change is widely accepted, you are willing to explain the differences to users and no test breaks. After all its a tiny, little PDO driver default setting. I think you interpreted me wrong. I was just pointing out that this change would be localized to the PDO_MySQL driver. And that documenting whether other DB drivers should be changed is a moot point, since they already aren't using emulation by default... And where did the passive-agressive thing hit off? I posted it to list There's a bug/feature request. Not just one, many came up over the years. You wrote a patch. You proposed a change. Nobody rejected the change - for a year. Its a tiny change of a driver default setting. It does not require an RFC. One of the maintainers, me, didn't bring up any objections. I said what any maintainer has to say: - educate users, be aware of the consequences - make sure not to break any tests Another maintainer, Johannes, didn't say no a year ago. Nobody says no, !no ~ go - its your call. Stand in for your proposal, take care of the tests and commit. Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Ulf, My apologies. I mis-interpreted that reply. I would commit, but I do not have commit access (no karma at all). I'm going away for a few days, but when I get back I will send a separate request to the list for karma. Thanks, Anthony On Fri, Jun 15, 2012 at 1:55 PM, Ulf Wendel ulf.wen...@oracle.com wrote: Am 15.06.2012 19:31, schrieb Anthony Ferrara: Ulf, It needs an RFC because it needs to document whether or not the other DB drivers should also be changed. It's a PDO driver-specific change. So to me it's fairly straight forward to see that no other drivers need changing... It doesn't even hit the mysql API level... Not saying an RFC isn't needed, just that justification isn't clear to me... Go commit if you feel like the change is widely accepted, you are willing to explain the differences to users and no test breaks. After all its a tiny, little PDO driver default setting. I think you interpreted me wrong. I was just pointing out that this change would be localized to the PDO_MySQL driver. And that documenting whether other DB drivers should be changed is a moot point, since they already aren't using emulation by default... And where did the passive-agressive thing hit off? I posted it to list There's a bug/feature request. Not just one, many came up over the years. You wrote a patch. You proposed a change. Nobody rejected the change - for a year. Its a tiny change of a driver default setting. It does not require an RFC. One of the maintainers, me, didn't bring up any objections. I said what any maintainer has to say: - educate users, be aware of the consequences - make sure not to break any tests Another maintainer, Johannes, didn't say no a year ago. Nobody says no, !no ~ go - its your call. Stand in for your proposal, take care of the tests and commit. Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
No worries, Anthony! Am 15.06.2012 21:15, schrieb Anthony Ferrara: I would commit, but I do not have commit access (no karma at all). I'm going away for a few days, but when I get back I will send a separate request to the list for karma. I see. It is not up to me to grant karma. I am not that active in the PHP project and wouldn't even want to have the power to give you karma. Those that have the power, shall decide. The key for me is: ... you stand in for the change ... you are aware of the technical aspects ... you are willing to help ... you waited for a long time and drove discussion - you must have a reason... Who does the commit is of little interest to me. You, I or Johannes can do it the other day. You can expect us to support the new default and not fall in your back but regarding the change itself, I'm no more than +/- 0. You'll have to continue to stand in if a discussion comes up. Deal? n8 enjoy your weekend, Ulf -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
[PHP-DEV] [PATCH - PR] Disable ATTR_EMULATE_PREPARES by default for PDO_Mysql
Hello all, I raised this topic on list over a year ago ( http://marc.info/?l=php-internalsm=130417646507744w=2 ). It was determined that it wasn't time yet to disable prepared statement emulation for MySQL yet. However, Rasmus did mention that it was a possibility for 5.4 ( http://marc.info/?l=php-internalsm=130418875017027w=2 ). Since that ship has sailed, I submitted a pull request for trunk to change the default value of prepared statement emulation for MySQL. https://github.com/php/php-src/pull/108 https://bugs.php.net/bug.php?id=54638 Does this need to be an RFC (should I draft one)? Or can it just be pulled as-is? Thanks, Anthony -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
