Nothing Special   »   [go: up one dir, main page]

Skip to content
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

Undefined Array Key with a Key as a Long String #13279

Closed
designextreme opened this issue Jan 30, 2024 · 14 comments
Closed

Undefined Array Key with a Key as a Long String #13279

designextreme opened this issue Jan 30, 2024 · 14 comments

Comments

@designextreme
Copy link

Description

The following code:

<?php
class array_test
{
	public $array = array();

	public function __construct()
	{
		$this->array = array(
			'1696007099_5_d386ca263816163166823ef00979d944' => array(
				'order' => 1
			),
			'1663066269_5_8c82ae69264d73f7ae7ce3a619501231' => array(
				'order' => 0
			)
		);

		uksort($this->array, function ($a, $b)
			{
				return $this->array[$a]['order'] - $this->array[$b]['order'];
			}
		);
	}
}

new array_test;

Resulted in this output:

Warning:  Undefined array key "1696007099_5_d386ca263816163166823ef00979d944" in .../array_test.php on line 19

But I expected this output instead:

No warnings/errors, just a freshly sorted array.

I have tested the following:

echo $a; // 1696007099_5_d386ca263816163166823ef00979d944
echo $b; // 1663066269_5_8c82ae69264d73f7ae7ce3a619501231
echo array_key_exists($a, $this->array); // FALSE
echo isset($this->array[$a]); // FALSE
echo array_key_first($this->array); // 1696007099_5_d386ca263816163166823ef00979d944
echo array_keys($this->array)[0]; // 1696007099_5_d386ca263816163166823ef00979d944

PHP Version

PHP 8.3

Operating System

CentOS 7.9

@iluuu1994
Copy link
Member

I don't see this warning. How are you running this code? Just in a CLI script?

@designextreme
Copy link
Author

I've attempted to simplify the PHP contained within a large script and may have failed in the process. I think I missed something with the array...

@designextreme
Copy link
Author

The example code is a simplified version of a WordPress plugin. It's really odd how array values can't be accessed by the keys here, yet they do exist. It is covered in a forum post at: WordPress.org. This is only in PHP 8.3, not 8.2,

In the meantime, I will try to create a simple version so you can replicate it.

@SVGAnimate
Copy link
Contributor

The mbstring extension has changed. I bet that's it.

@designextreme
Copy link
Author

@SVGAnimate I think you're on to something. The actual array does contain emoticons and more besides. I am still attempting to create the "simple" version using the review data from the WordPress plugin.

@SVGAnimate
Copy link
Contributor
SVGAnimate commented Jan 30, 2024

In the index.php i see suspicius code :

				uksort($this->reviews_filtered, function ($b, $a)
					{
						if ($this->review_sort_option == 'review_characters_asc' || $this->review_sort_option == 'review_characters_desc')
						{
							return mb_strlen($this->reviews_filtered[$a][$this->review_sort_options[$this->review_sort_option]['field']]) - mb_strlen($this->reviews_filtered[$b][$this->review_sort_options[$this->review_sort_option]['field']]);
						}
						

The problem is that in the PHP-8.3 version the mbstring extension filters UTF8 string.
So sequence of characters like \xFF \xFE fallback to ? before no.

Post-Edit:

... @alexdowad can you enlighten us ?

I also suspect this bug report. But without more information it is difficult to say more.
#13229

@designextreme
Copy link
Author

If this is a multibyte issue, I will need more time to isolate the issue and produce a fix in my code. However, whatever is in the array, it really should not be failing at this point - the array key check should be unaffected.

@iluuu1994
Copy link
Member

ext-mbstring doesn't affect how array keys are handled. If this is related to mbstring (to which there indeed have been many changes between 8.2 and 8.3) the bug most likely occurs in a different place.

@designextreme
Copy link
Author

It is iterating through an array with keys that don't "exist". Even if the fault occurred before in the code, how is this possible? This is why I reported it as a bug - it's failing either too late or, in based on the code, why is it even failing at all?

I will try to recreate this using a stripped back example - but the version in the WordPress plugin does return warnings (version 4.27 as I will add a work-around soon; it will probably need the completed setup).

@iluuu1994
Copy link
Member

It is iterating through an array with keys that don't "exist". Even if the fault occurred before in the code, how is this possible?

I think this would be possible if the string is placed in the wrong bucket, i.e. if it was originally saved in one bucket and then the string+hash change. I suppose this could happen if there was a refcounting bug where the string is modified in-place.

But let's wait for your reproducer.

@designextreme
Copy link
Author

Got it! Here is the simplified example:

<?php

class array_test
{
	public $array = array();

	public function __construct()
	{
		$this->array = array(
			'1696007099_5_d386ca263816163166823ef00979d944' => array(
				'id' => 1,
				'order' => 1
			),
			'1663066269_5_8c82ae69264d73f7ae7ce3a619501231' => array(
				'id' => 2,
				'order' => 0
			)
		);

        $relevance = 0;
        $checked_ids = array();

        foreach (array_keys($this->array) as $key)
        {
            $a = $this->array[$key];
            
            if (!in_array($a['id'], $checked_ids, TRUE))
            {
				/* 
					This part introduces the key error
 				*/
                $this->array[$key]['removable'] = TRUE;
                $this->array[$key]['order'] = $relevance;
                $relevance++;
            }
        }

		uksort($this->array, function ($a, $b)
			{
				/*
					A warning is produced:
					Undefined array key "1696007099_5_d386ca263816163166823ef00979d944" in array_test.php on line 44
				*/
				return $this->array[$a]['order'] - $this->array[$b]['order'];
			}
		);

        echo '<pre>' . print_r($this->array, TRUE) . '</pre>';
	}
}

new array_test;

I did try this with some multi-byte characters and emoticons, but this didn't affect the outcome.

@SVGAnimate
Copy link
Contributor

On PHP 8.3.1 i do not have Notice/Warning

@iluuu1994
Copy link
Member
iluuu1994 commented Jan 31, 2024

Hmm yes. This is caused by #11060. The issue is that during the user call, the array is essentially unstable. I think this should be reverted for php_usort. /cc @nielsdos (I'm creating a PR)

@designextreme
Copy link
Author

With PHP 8.3.2, here are the log entries:

[31-Jan-2024 12:13:58 UTC] PHP Warning:  Undefined array key "1696007099_5_d386ca263816163166823ef00979d944" in array_test.php on line 44
[31-Jan-2024 12:13:58 UTC] PHP Warning:  Trying to access array offset on null in array_test.php on line 44
[31-Jan-2024 12:13:58 UTC] PHP Warning:  Undefined array key "1663066269_5_8c82ae69264d73f7ae7ce3a619501231" in array_test.php on line 44
[31-Jan-2024 12:13:58 UTC] PHP Warning:  Trying to access array offset on null in array_test.php on line 44

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Jan 31, 2024
The array isn't just observable if the array has RCn, but also if it is inside a
reference that is RCn. By-ref parameters are always RCn and as such always
observable.

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

No branches or pull requests

3 participants