From b4e272c56ad5d4130754e9a024f9e4304dd60da2 Mon Sep 17 00:00:00 2001 From: Gina Peter Banyard Date: Tue, 27 Feb 2024 15:25:00 +0000 Subject: [PATCH] ext/pdo: Fix various PDORow bugs - Add tests - NULL derefencing in read_dimension handler - Fix isset() - Fix empty() with column numbers as offsets - Refactoring to use common functions --- NEWS | 3 + ext/pdo/pdo_stmt.c | 198 +++++++++++----------- ext/pdo/tests/pdo_035.phpt | 262 ++++++++++++++++++++++++++++++ ext/pdo_sqlite/tests/pdo_035.phpt | 46 ------ 4 files changed, 365 insertions(+), 144 deletions(-) create mode 100644 ext/pdo/tests/pdo_035.phpt delete mode 100644 ext/pdo_sqlite/tests/pdo_035.phpt diff --git a/NEWS b/NEWS index d8ffce0018d..1564f031f0b 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.2.18 +- PDO: + . Fix various PDORow bugs. (Girgias) + 14 Mar 2024, PHP 8.2.17 diff --git a/ext/pdo/pdo_stmt.c b/ext/pdo/pdo_stmt.c index 453933607f3..e11d0f4288c 100644 --- a/ext/pdo/pdo_stmt.c +++ b/ext/pdo/pdo_stmt.c @@ -2257,74 +2257,84 @@ zend_object_iterator *pdo_stmt_iter_get(zend_class_entry *ce, zval *object, int /* }}} */ /* {{{ overloaded handlers for PDORow class (used by PDO_FETCH_LAZY) */ +static zval *row_read_column_name(pdo_stmt_t *stmt, zend_string *name, zval *rv) +{ + /* TODO: replace this with a hash of available column names to column numbers */ + for (int colno = 0; colno < stmt->column_count; colno++) { + if (zend_string_equals(stmt->columns[colno].name, name)) { + fetch_value(stmt, rv, colno, NULL); + return rv; + } + } + return NULL; +} + +static zval *row_read_column_number(pdo_stmt_t *stmt, zend_long column, zval *rv) +{ + if (column >= 0 && column < stmt->column_count) { + fetch_value(stmt, rv, column, NULL); + return rv; + } + return NULL; +} static zval *row_prop_read(zend_object *object, zend_string *name, int type, void **cache_slot, zval *rv) { pdo_row_t *row = (pdo_row_t *)object; pdo_stmt_t *stmt = row->stmt; - int colno = -1; zend_long lval; + zval *retval; ZEND_ASSERT(stmt); ZVAL_NULL(rv); if (zend_string_equals_literal(name, "queryString")) { return zend_std_read_property(&stmt->std, name, type, cache_slot, rv); - } else if (is_numeric_string(ZSTR_VAL(name), ZSTR_LEN(name), &lval, NULL, 0) == IS_LONG) { - if (lval >= 0 && lval < stmt->column_count) { - fetch_value(stmt, rv, lval, NULL); - } + } else if (is_numeric_str_function(name, &lval, /* dval */ NULL) == IS_LONG) { + retval = row_read_column_number(stmt, lval, rv); } else { - /* TODO: replace this with a hash of available column names to column - * numbers */ - for (colno = 0; colno < stmt->column_count; colno++) { - if (zend_string_equals(stmt->columns[colno].name, name)) { - fetch_value(stmt, rv, colno, NULL); - return rv; - } - } + retval = row_read_column_name(stmt, name, rv); } - - return rv; + if (UNEXPECTED(!retval)) { + // TODO throw an error on master + //if (type != BP_VAR_IS) { + // if (is_numeric) { + // zend_value_error("Invalid column index"); + // } else { + // zend_throw_error(NULL, "No column named \"%s\" exists", ZSTR_VAL(name)); + // } + //} + //return &EG(uninitialized_zval); + ZVAL_NULL(rv); + return rv; + } + return retval; } -static zval *row_dim_read(zend_object *object, zval *member, int type, zval *rv) +static zval *row_dim_read(zend_object *object, zval *offset, int type, zval *rv) { - pdo_row_t *row = (pdo_row_t *)object; - pdo_stmt_t *stmt = row->stmt; - int colno = -1; - zend_long lval; - ZEND_ASSERT(stmt); - - ZVAL_NULL(rv); - if (Z_TYPE_P(member) == IS_LONG) { - if (Z_LVAL_P(member) >= 0 && Z_LVAL_P(member) < stmt->column_count) { - fetch_value(stmt, rv, Z_LVAL_P(member), NULL); - } - } else if (Z_TYPE_P(member) == IS_STRING - && is_numeric_string(Z_STRVAL_P(member), Z_STRLEN_P(member), &lval, NULL, 0) == IS_LONG) { - if (lval >= 0 && lval < stmt->column_count) { - fetch_value(stmt, rv, lval, NULL); - } - } else { - if (!try_convert_to_string(member)) { - return &EG(uninitialized_zval); - } - - if (zend_string_equals_literal(Z_STR_P(member), "queryString")) { - return zend_std_read_property(&stmt->std, Z_STR_P(member), type, NULL, rv); - } - - /* TODO: replace this with a hash of available column names to column - * numbers */ - for (colno = 0; colno < stmt->column_count; colno++) { - if (zend_string_equals(stmt->columns[colno].name, Z_STR_P(member))) { - fetch_value(stmt, rv, colno, NULL); - return rv; - } - } + if (UNEXPECTED(!offset)) { + zend_throw_error(NULL, "Cannot append to PDORow offset"); + return NULL; } + if (Z_TYPE_P(offset) == IS_LONG) { + pdo_row_t *row = (pdo_row_t *)object; + pdo_stmt_t *stmt = row->stmt; + ZEND_ASSERT(stmt); - return rv; + ZVAL_NULL(rv); + if (Z_LVAL_P(offset) >= 0 && Z_LVAL_P(offset) < stmt->column_count) { + fetch_value(stmt, rv, Z_LVAL_P(offset), NULL); + } + return rv; + } else { + zend_string *member = zval_try_get_string(offset); + if (!member) { + return NULL; + } + zval *result = row_prop_read(object, member, type, NULL, rv); + zend_string_release_ex(member, false); + return result; + } } static zval *row_prop_write(zend_object *object, zend_string *name, zval *value, void **cache_slot) @@ -2335,75 +2345,67 @@ static zval *row_prop_write(zend_object *object, zend_string *name, zval *value, static void row_dim_write(zend_object *object, zval *member, zval *value) { - zend_throw_error(NULL, "Cannot write to PDORow offset"); + if (!member) { + zend_throw_error(NULL, "Cannot append to PDORow offset"); + } else { + zend_throw_error(NULL, "Cannot write to PDORow offset"); + } } static int row_prop_exists(zend_object *object, zend_string *name, int check_empty, void **cache_slot) { pdo_row_t *row = (pdo_row_t *)object; pdo_stmt_t *stmt = row->stmt; - int colno = -1; zend_long lval; + zval tmp_val; + zval *retval = NULL; ZEND_ASSERT(stmt); - if (is_numeric_string(ZSTR_VAL(name), ZSTR_LEN(name), &lval, NULL, 0) == IS_LONG) { - return lval >=0 && lval < stmt->column_count; + if (is_numeric_str_function(name, &lval, /* dval */ NULL) == IS_LONG) { + retval = row_read_column_number(stmt, lval, &tmp_val); + } else { + retval = row_read_column_name(stmt, name, &tmp_val); } - /* TODO: replace this with a hash of available column names to column - * numbers */ - for (colno = 0; colno < stmt->column_count; colno++) { - if (zend_string_equals(stmt->columns[colno].name, name)) { - int res; - zval val; - - fetch_value(stmt, &val, colno, NULL); - res = check_empty ? i_zend_is_true(&val) : Z_TYPE(val) != IS_NULL; - zval_ptr_dtor_nogc(&val); - - return res; - } + if (!retval) { + return false; } - - return 0; + ZEND_ASSERT(retval == &tmp_val); + int res = check_empty ? i_zend_is_true(retval) : Z_TYPE(tmp_val) != IS_NULL; + zval_ptr_dtor_nogc(retval); + return res; } -static int row_dim_exists(zend_object *object, zval *member, int check_empty) +static int row_dim_exists(zend_object *object, zval *offset, int check_empty) { - pdo_row_t *row = (pdo_row_t *)object; - pdo_stmt_t *stmt = row->stmt; - int colno = -1; - zend_long lval; - ZEND_ASSERT(stmt); + if (Z_TYPE_P(offset) == IS_LONG) { + pdo_row_t *row = (pdo_row_t *)object; + pdo_stmt_t *stmt = row->stmt; + ZEND_ASSERT(stmt); + zend_long column = Z_LVAL_P(offset); - if (Z_TYPE_P(member) == IS_LONG) { - return Z_LVAL_P(member) >= 0 && Z_LVAL_P(member) < stmt->column_count; - } else if (Z_TYPE_P(member) == IS_STRING) { - if (is_numeric_string(Z_STRVAL_P(member), Z_STRLEN_P(member), &lval, NULL, 0) == IS_LONG) { - return lval >=0 && lval < stmt->column_count; + if (!check_empty) { + return column >= 0 && column < stmt->column_count; } + + zval tmp_val; + zval *retval = row_read_column_number(stmt, column, &tmp_val); + if (!retval) { + return false; + } + ZEND_ASSERT(retval == &tmp_val); + int res = check_empty ? i_zend_is_true(retval) : Z_TYPE(tmp_val) != IS_NULL; + zval_ptr_dtor_nogc(retval); + return res; } else { - if (!try_convert_to_string(member)) { + zend_string *member = zval_try_get_string(offset); + if (!member) { return 0; } + int result = row_prop_exists(object, member, check_empty, NULL); + zend_string_release_ex(member, false); + return result; } - - /* TODO: replace this with a hash of available column names to column - * numbers */ - for (colno = 0; colno < stmt->column_count; colno++) { - if (zend_string_equals(stmt->columns[colno].name, Z_STR_P(member))) { - int res; - zval val; - - fetch_value(stmt, &val, colno, NULL); - res = check_empty ? i_zend_is_true(&val) : Z_TYPE(val) != IS_NULL; - zval_ptr_dtor_nogc(&val); - - return res; - } - } - - return 0; } static void row_prop_delete(zend_object *object, zend_string *offset, void **cache_slot) diff --git a/ext/pdo/tests/pdo_035.phpt b/ext/pdo/tests/pdo_035.phpt new file mode 100644 index 00000000000..05bf32ba9af --- /dev/null +++ b/ext/pdo/tests/pdo_035.phpt @@ -0,0 +1,262 @@ +--TEST-- +PDO Common: PDORow + get_parent_class() +--EXTENSIONS-- +pdo +--SKIPIF-- + +--FILE-- +exec('CREATE TABLE ' . TABLE_NAME .' (id int, name varchar(10))'); +$db->exec('INSERT INTO ' . TABLE_NAME .' VALUES (23, \'0\')'); + +$stmt = $db->prepare('SELECT id, name FROM ' . TABLE_NAME); +$stmt->execute(); +$result = $stmt->fetch(PDO::FETCH_LAZY); + +var_dump($result); +var_dump(get_parent_class($result)); + +foreach ([0, "0", "id", "name", 1] as $offset) { + echo 'Offset: ', var_export($offset), PHP_EOL; + $offsetRef = &$offset; + + echo 'Dimension:', PHP_EOL; + echo 'Isset:', PHP_EOL; + var_dump(isset($result[$offset])); + var_dump(isset($result[$offsetRef])); + echo 'Empty:', PHP_EOL; + var_dump(empty($result[$offset])); + var_dump(empty($result[$offsetRef])); + echo 'Null coalesce:', PHP_EOL; + var_dump($result[$offset] ?? "default"); + var_dump($result[$offsetRef] ?? "default"); + echo 'Read:', PHP_EOL; + var_dump($result[$offset]); + var_dump($result[$offsetRef]); + echo 'Property:', PHP_EOL; + echo 'Isset:', PHP_EOL; + var_dump(isset($result->{$offset})); + var_dump(isset($result->{$offsetRef})); + echo 'Empty:', PHP_EOL; + var_dump(empty($result->{$offset})); + var_dump(empty($result->{$offsetRef})); + echo 'Null coalesce:', PHP_EOL; + var_dump($result->{$offset} ?? "default"); + var_dump($result->{$offsetRef} ?? "default"); + echo 'Read:', PHP_EOL; + var_dump($result->{$offset}); + var_dump($result->{$offsetRef}); +} + +echo 'Errors:', PHP_EOL; +try { + $result[0] = 1; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $result[] = 1; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $refResult = &$result[0]; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $refResult = &$result[]; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + unset($result[0]); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + $result->foo = 1; +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} +try { + unset($result->foo); +} catch (Error $e) { + echo $e->getMessage(), "\n"; +} + +?> +--CLEAN-- +setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_SILENT); + +const TABLE_NAME = 'test_pdo_35_pdo_row'; +$db->exec("DROP TABLE " . TABLE_NAME); +?> +--EXPECTF-- +object(PDORow)#3 (3) { + ["queryString"]=> + string(40) "SELECT id, name FROM test_pdo_35_pdo_row" + ["id"]=> + string(2) "23" + ["name"]=> + string(1) "0" +} +bool(false) +Offset: 0 +Dimension: +Isset: +bool(true) +bool(true) +Empty: +bool(false) +bool(false) +Null coalesce: +string(2) "23" +string(2) "23" +Read: +string(2) "23" +string(2) "23" +Property: +Isset: +bool(true) +bool(true) +Empty: +bool(false) +bool(false) +Null coalesce: +string(2) "23" +string(2) "23" +Read: +string(2) "23" +string(2) "23" +Offset: '0' +Dimension: +Isset: +bool(true) +bool(true) +Empty: +bool(false) +bool(false) +Null coalesce: +string(2) "23" +string(2) "23" +Read: +string(2) "23" +string(2) "23" +Property: +Isset: +bool(true) +bool(true) +Empty: +bool(false) +bool(false) +Null coalesce: +string(2) "23" +string(2) "23" +Read: +string(2) "23" +string(2) "23" +Offset: 'id' +Dimension: +Isset: +bool(true) +bool(true) +Empty: +bool(false) +bool(false) +Null coalesce: +string(2) "23" +string(2) "23" +Read: +string(2) "23" +string(2) "23" +Property: +Isset: +bool(true) +bool(true) +Empty: +bool(false) +bool(false) +Null coalesce: +string(2) "23" +string(2) "23" +Read: +string(2) "23" +string(2) "23" +Offset: 'name' +Dimension: +Isset: +bool(true) +bool(true) +Empty: +bool(true) +bool(true) +Null coalesce: +string(1) "0" +string(1) "0" +Read: +string(1) "0" +string(1) "0" +Property: +Isset: +bool(true) +bool(true) +Empty: +bool(true) +bool(true) +Null coalesce: +string(1) "0" +string(1) "0" +Read: +string(1) "0" +string(1) "0" +Offset: 1 +Dimension: +Isset: +bool(true) +bool(true) +Empty: +bool(true) +bool(true) +Null coalesce: +string(1) "0" +string(1) "0" +Read: +string(1) "0" +string(1) "0" +Property: +Isset: +bool(true) +bool(true) +Empty: +bool(true) +bool(true) +Null coalesce: +string(1) "0" +string(1) "0" +Read: +string(1) "0" +string(1) "0" +Errors: +Cannot write to PDORow offset +Cannot append to PDORow offset + +Notice: Indirect modification of overloaded element of PDORow has no effect in %s on line %d +Cannot append to PDORow offset +Cannot unset PDORow offset +Cannot write to PDORow property +Cannot unset PDORow property diff --git a/ext/pdo_sqlite/tests/pdo_035.phpt b/ext/pdo_sqlite/tests/pdo_035.phpt deleted file mode 100644 index ec723842f03..00000000000 --- a/ext/pdo_sqlite/tests/pdo_035.phpt +++ /dev/null @@ -1,46 +0,0 @@ ---TEST-- -PDO Common: PDORow + get_parent_class() ---EXTENSIONS-- -pdo_sqlite ---FILE-- -exec('CREATE TABLE test (id int)'); -$db->exec('INSERT INTO test VALUES (23)'); - -$stmt = $db->prepare('SELECT id FROM test'); -$stmt->execute(); -$result = $stmt->fetch(PDO::FETCH_LAZY); - -echo get_class($result), "\n"; -var_dump(get_parent_class($result)); - -try { - $result->foo = 1; -} catch (Error $e) { - echo $e->getMessage(), "\n"; -} -try { - $result[0] = 1; -} catch (Error $e) { - echo $e->getMessage(), "\n"; -} -try { - unset($result->foo); -} catch (Error $e) { - echo $e->getMessage(), "\n"; -} -try { - unset($result[0]); -} catch (Error $e) { - echo $e->getMessage(), "\n"; -} - -?> ---EXPECT-- -PDORow -bool(false) -Cannot write to PDORow property -Cannot write to PDORow offset -Cannot unset PDORow property -Cannot unset PDORow offset