Skip to content

Fix GH-16905: Internal iterator functions can't handle UNDEF properties #16907

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

Fix GH-16905: Internal iterator functions can't handle UNDEF properties #16907

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
ca79b10
Fix GH-16905: Internal iterator functions can't handle UNDEF properties
nielsdos Nov 23, 2024
b3cc8a8
Use ZVAL_DEINDIRECT
nielsdos Nov 23, 2024
393199c
Follow foreach behaviour
nielsdos Nov 23, 2024
9a18fc7
Throw in key() and current() when internal iterator points to undef e…
nielsdos Nov 24, 2024
031321a
Rename enum
nielsdos Nov 24, 2024
4542899
Rename more
nielsdos Nov 24, 2024
165b07a
Change strategy again
nielsdos Nov 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
85 changes: 35 additions & 50 deletions ext/standard/array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1029,11 +1029,40 @@ static inline HashTable *get_ht_for_iap(zval *zv, bool separate) {
return zobj->handlers->get_properties(zobj);
}

static void ia_return_current(zval *return_value, HashTable *array, bool forward_direction)
{
zval *entry;

while (true) {
if ((entry = zend_hash_get_current_data(array)) == NULL) {
RETURN_FALSE;
}

ZVAL_DEINDIRECT(entry);

/* Possible with an uninitialized typed property */
if (UNEXPECTED(Z_TYPE_P(entry) == IS_UNDEF)) {
zend_result result;
if (forward_direction) {
result = zend_hash_move_forward(array);
} else {
result = zend_hash_move_backwards(array);
}
if (result != SUCCESS) {
RETURN_FALSE;
}
} else {
break;
}
}

RETURN_COPY_DEREF(entry);
}

/* {{{ Advances array argument's internal pointer to the last element and return it */
PHP_FUNCTION(end)
{
zval *array_zv;
zval *entry;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_ARRAY_OR_OBJECT_EX(array_zv, 0, 1)
Expand All @@ -1047,15 +1076,7 @@ PHP_FUNCTION(end)
zend_hash_internal_pointer_end(array);

if (USED_RET()) {
if ((entry = zend_hash_get_current_data(array)) == NULL) {
RETURN_FALSE;
}

if (Z_TYPE_P(entry) == IS_INDIRECT) {
entry = Z_INDIRECT_P(entry);
}

RETURN_COPY_DEREF(entry);
ia_return_current(return_value, array, false);
}
}
/* }}} */
Expand All @@ -1064,7 +1085,6 @@ PHP_FUNCTION(end)
PHP_FUNCTION(prev)
{
zval *array_zv;
zval *entry;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_ARRAY_OR_OBJECT_EX(array_zv, 0, 1)
Expand All @@ -1078,15 +1098,7 @@ PHP_FUNCTION(prev)
zend_hash_move_backwards(array);

if (USED_RET()) {
if ((entry = zend_hash_get_current_data(array)) == NULL) {
RETURN_FALSE;
}

if (Z_TYPE_P(entry) == IS_INDIRECT) {
entry = Z_INDIRECT_P(entry);
}

RETURN_COPY_DEREF(entry);
ia_return_current(return_value, array, false);
}
}
/* }}} */
Expand All @@ -1095,7 +1107,6 @@ PHP_FUNCTION(prev)
PHP_FUNCTION(next)
{
zval *array_zv;
zval *entry;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_ARRAY_OR_OBJECT_EX(array_zv, 0, 1)
Expand All @@ -1109,15 +1120,7 @@ PHP_FUNCTION(next)
zend_hash_move_forward(array);

if (USED_RET()) {
if ((entry = zend_hash_get_current_data(array)) == NULL) {
RETURN_FALSE;
}

if (Z_TYPE_P(entry) == IS_INDIRECT) {
entry = Z_INDIRECT_P(entry);
}

RETURN_COPY_DEREF(entry);
ia_return_current(return_value, array, true);
}
}
/* }}} */
Expand All @@ -1126,7 +1129,6 @@ PHP_FUNCTION(next)
PHP_FUNCTION(reset)
{
zval *array_zv;
zval *entry;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_ARRAY_OR_OBJECT_EX(array_zv, 0, 1)
Expand All @@ -1140,15 +1142,7 @@ PHP_FUNCTION(reset)
zend_hash_internal_pointer_reset(array);

if (USED_RET()) {
if ((entry = zend_hash_get_current_data(array)) == NULL) {
RETURN_FALSE;
}

if (Z_TYPE_P(entry) == IS_INDIRECT) {
entry = Z_INDIRECT_P(entry);
}

RETURN_COPY_DEREF(entry);
ia_return_current(return_value, array, true);
}
}
/* }}} */
Expand All @@ -1157,22 +1151,13 @@ PHP_FUNCTION(reset)
PHP_FUNCTION(current)
{
zval *array_zv;
zval *entry;

ZEND_PARSE_PARAMETERS_START(1, 1)
Z_PARAM_ARRAY_OR_OBJECT(array_zv)
ZEND_PARSE_PARAMETERS_END();

HashTable *array = get_ht_for_iap(array_zv, /* separate */ false);
if ((entry = zend_hash_get_current_data(array)) == NULL) {
RETURN_FALSE;
}

if (Z_TYPE_P(entry) == IS_INDIRECT) {
entry = Z_INDIRECT_P(entry);
}

RETURN_COPY_DEREF(entry);
ia_return_current(return_value, array, true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will implicitly advance when the current element is unset. E.g. with a current(), unset(), current(). I think it would be reasonable for current() not to advance, but to throw if the element is undef.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's fair, same for key(), let me fix this.

}
/* }}} */

Expand Down
63 changes: 63 additions & 0 deletions ext/standard/tests/array/gh16905.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
--TEST--
GH-16905 (Internal iterator functions can't handle UNDEF properties)
--FILE--
<?php

class TestSomeUndef {
public int $a;
public int $b;
public int $c;
public int $d;
}

class TestAllUndef {
public int $a;
}

$x = new TestSomeUndef;
$x->b = 1;
$x->c = 2;

var_dump(reset($x));
var_dump(current($x));
var_dump(end($x));

var_dump(reset($x));
var_dump(next($x));

var_dump(end($x));
var_dump(prev($x));

var_dump(value: key($x));

$x = new TestAllUndef;
var_dump(current($x));

?>
--EXPECTF--
Deprecated: reset(): Calling reset() on an object is deprecated in %s on line %d
int(1)

Deprecated: current(): Calling current() on an object is deprecated in %s on line %d
int(1)

Deprecated: end(): Calling end() on an object is deprecated in %s on line %d
int(2)

Deprecated: reset(): Calling reset() on an object is deprecated in %s on line %d
int(1)

Deprecated: next(): Calling next() on an object is deprecated in %s on line %d
int(2)

Deprecated: end(): Calling end() on an object is deprecated in %s on line %d
int(2)

Deprecated: prev(): Calling prev() on an object is deprecated in %s on line %d
int(1)

Deprecated: key(): Calling key() on an object is deprecated in %s on line %d
string(1) "b"

Deprecated: current(): Calling current() on an object is deprecated in %s on line %d
bool(false)
Loading