Skip to content

Deprecate using null as an array offset and when calling array_key_exists() #19511

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.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

alexandre-daubois
Copy link
Member

@alexandre-daubois alexandre-daubois commented Aug 18, 2025

@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 3 times, most recently from 472c3c6 to baeda1f Compare August 20, 2025 08:30
@@ -6981,6 +6981,7 @@ PHP_FUNCTION(array_key_exists)
RETVAL_BOOL(zend_hash_index_exists(ht, Z_LVAL_P(key)));
break;
case IS_NULL:
zend_error(E_DEPRECATED, "Using null as an array offset is deprecated, use an empty string instead");
Copy link
Member

Choose a reason for hiding this comment

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

should we have different messages for using array offset ($foo[null]) and array key exists (array_key_exists(null, $foo))?

Copy link
Member Author

@alexandre-daubois alexandre-daubois Aug 20, 2025

Choose a reason for hiding this comment

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

I pushed "Using null as the key parameter for array_key_exists() is deprecated, use an empty string instead". What do you think?

@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 2 times, most recently from d5fd9e5 to a602268 Compare August 20, 2025 15:02
@alexandre-daubois
Copy link
Member Author

Still a few tests to fix but we're getting closer

@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 2 times, most recently from 6f50c89 to e508fcc Compare August 21, 2025 06:54
@alexandre-daubois alexandre-daubois force-pushed the array-key-exists-null branch 3 times, most recently from fd39b6d to 8f22921 Compare August 21, 2025 08:14
@@ -2629,6 +2629,7 @@ static zend_never_inline uint8_t slow_index_convert(HashTable *ht, const zval *d
ZEND_FALLTHROUGH;
}
case IS_NULL:
zend_error(E_DEPRECATED, "Using null as an array offset is deprecated, use an empty string instead");
Copy link
Member

Choose a reason for hiding this comment

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

You will need to addref/delref ht around this, because ht may be freed by an error handler:

set_error_handler(function($errno, $errstr) {
    global $ary;
    $ary = null;
    echo $errstr;
});

$ary[null] = 1; // AddressSanitizer: heap-use-after-free

See case IS_DOUBLE as an example

@@ -2699,6 +2700,7 @@ static zend_never_inline uint8_t slow_index_convert_w(HashTable *ht, const zval
ZEND_FALLTHROUGH;
}
case IS_NULL:
zend_error(E_DEPRECATED, "Using null as an array offset is deprecated, use an empty string instead");
Copy link
Member

Choose a reason for hiding this comment

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

Here as well

@@ -3324,6 +3326,9 @@ static zend_never_inline bool ZEND_FASTCALL zend_array_key_exists_fast(HashTable
} else if (Z_TYPE_P(key) <= IS_NULL) {
if (UNEXPECTED(Z_TYPE_P(key) == IS_UNDEF)) {
ZVAL_UNDEFINED_OP1();
} else {
ZEND_ASSERT(Z_TYPE_P(key) == IS_NULL);
zend_error(E_DEPRECATED, "Using null as the key parameter for array_key_exists() is deprecated, use an empty string instead");
Copy link
Member

Choose a reason for hiding this comment

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

Probably here as well (I'm not sure)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants