#32 ADOConnection->Execute ignores param when it's not an array and evaluates to "false"

Unstable (example)
closed-accepted
dregad
5
2014-12-24
2014-02-20
No

Fix for ADOConnection->Execute when called with just one param (not array) that evaluates to false.

Example:

$strSQL = 'SELECT * FROM products WHERE price = ?';
$var = 0;
$result = $cnnDB->Execute($strSQL, $var);

When calling Execute, it evaluates the second parameter in this line:

if ($inputarr) {...

And as 0 evaluates as false, it ignores that value.

Fix: to compare estrictly with false using

if ($inputarr !== false) {

http://sourceforge.net/p/adodb/git/ci/v5.18a/tree/phplens/adodb5/adodb.inc.php

/**
* Execute SQL
*
* @param sql SQL statement to execute, or possibly an array holding prepared statement ($sql[0] will hold sql text)
* @param [inputarr] holds the input data to bind to. Null elements will be set to null.
* @return RecordSet or false
*/
function Execute($sql,$inputarr=false)
{
    if ($this->fnExecute) {
        $fn = $this->fnExecute;
        $ret = $fn($this,$sql,$inputarr);
        if (isset($ret)) return $ret;
    }
    if ($inputarr !== false) {
        if (!is_array($inputarr)) $inputarr = array($inputarr);

Discussion

  • dregad
    dregad
    2014-02-21

    I didn't actually test this, but I'm wondering if it wouldn't be a cleaner solution instead to change the default value assigned to this parameter from false to array().

    Then we could do something like

    -       if ($inputarr) {
    -           if (!is_array($inputarr)) $inputarr = array($inputarr);
    +       if (!is_array($inputarr)) {
    +           $inputarr = array($inputarr);
    +       }
    +       if (!empty($inputarr)) {
    

    The only problem I can think of is that this change could cause regression for code calling Execute($query, false), as the method would interpret false as a single parameter for $query and fail to execute it.

     
  • I personally use Execute($query, false) a lot... so why introduce a problem changing the default value.

    Just checking for " !== false " is enough, mantaining the rest of the code as is.

    I cannot see any case where a developer need to pass false as a valid param for the SQL Query.

     
  • dregad
    dregad
    2014-02-25

    I cannot see any case where a developer need to pass false as a valid param for the SQL Query.

    What about
    Execute( 'select * from table where boolean_flag = $1', false );

    I'll grant you that this case probably only affects PostgreSQL as other RDBMS that I know of do not support a SQL:1999 boolean data type. With the current approach, one actually has to do
    Execute( 'select * from table where boolean_flag = $1', array(false) );

    which is not consistent, if you compare with e.g.
    Execute( 'select * from table where column = $1', 'value' );.

    But I guess you're right, it's probably not worth breaking backwards compatibility.

     
  • dregad
    dregad
    2014-03-04

    • status: open --> closed-accepted
    • assigned_to: John Lim --> dregad