Skip to content

Unable to serialize processed SplFixedArrays in PHP 8.2.4 #10907

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

Closed
codex-m opened this issue Mar 22, 2023 · 4 comments
Closed

Unable to serialize processed SplFixedArrays in PHP 8.2.4 #10907

codex-m opened this issue Mar 22, 2023 · 4 comments

Comments

@codex-m
Copy link

codex-m commented Mar 22, 2023

Description

The following code:

<?php
ini_set('display_errors', 1);
ini_set('display_startup_errors', 1);
error_reporting(E_ALL);

$array = new SplFixedArray(5);

$array[1] = 2;
$array[2] = "test value";
$copy = $array;
$serialized_one = serialize($array);
$object_vars = get_object_vars($array);
foreach ($object_vars as $property_name => $property_value) {    
    $copy[$property_name] = $property_value;
}
$serialized_two = '';
if ($copy === $array) {
    $serialized_two = serialize($copy);
}

if ($serialized_one === $serialized_two) {
    echo "PHP version " . phpversion() . " is ok";
}

Resulted in this output:

500 internal server error or no output

But I expected this output instead:

PHP version 8.2.4 is ok

Notes:

  • The above test code works in all commonly active versions: PHP 8.1, PHP 8.0 and PHP 7.4. However the above code does not work only in PHP 8.2.4.
  • Originally reproduced with Ubuntu 20.04 + PHP 8.2.4.
  • This is also reproducible in PHP Windows environment using PHP 8.2.4 .

I've check the upgrade notes on PHP 8.2.4. There is nothing that mention between serialize() and SplFixedArray. I'm still learning more about PHP 8.2, so please help me confirm if this is a bug or simply a usage issue. Thank you!

PHP Version

PHP 8.2.4

Operating System

Ubuntu 20.04 / Windows 10

@nielsdos
Copy link
Member

Can confirm your findings: Does indeed not reproduce on 8.1, but does on 8.2.
PHP crashes with a segfault.

@nielsdos nielsdos self-assigned this Mar 22, 2023
nielsdos added a commit to nielsdos/php-src that referenced this issue Mar 22, 2023
…8.2.4

The properties table can also contain numeric entries after a rebuild of
the table based on the array. Since the array can only contain numeric
entries, and the properties table can contain a mix of both, we'll add
the numeric entries from the array and only the string entries from the
properties table. To implement this we simply check if the key from the
properties table is a string.
@nielsdos
Copy link
Member

nielsdos commented Mar 22, 2023

I fixed this particular issue in this commit on my fork: nielsdos@042287c

However, the test won't pass yet because I ran into another pre-existing issue...
See the following code: https://3v4l.org/uZH8a of which the output doesn't make sense.
Upon trying to unserialize a property which can be interpreted as a numeric key it actually becomes a numeric key. This causes the property "123" to incorrectly end up in the array instead of the properties. If I manually print out the hashtable entries received in __unserialize, I see that the entry corresponding to what previously was "123" is now the numeric key 123 (i.e. key==NULL && h==123). I can't seem to figure out where and how this happens.

The numeric-like string bug is not an actual problem for this issue I think. I just wanted to test that scenario in my .phpt file to see if I didn't break anything wrt that.

I see that @Girgias is listed as code owner, maybe you can help here (if you want) :) ?

@Girgias
Copy link
Member

Girgias commented Mar 23, 2023

Erg this feels like something related to this change in 7.2: https://www.php.net/manual/en/migration72.incompatible.php#migration72.incompatible.object-array-casts

So I think it is expected behaviour? Looking at the code run on all versions, the behaviour seems consistent since mid PHP 5.4

The fix looks sensible tho

@nielsdos
Copy link
Member

Strange but okay. I'll not touch that behaviour and just amend my test and then create a PR for this. Thanks for having a look.

nielsdos added a commit to nielsdos/php-src that referenced this issue Mar 23, 2023
…8.2.4

The properties table can also contain numeric entries after a rebuild of
the table based on the array. Since the array can only contain numeric
entries, and the properties table can contain a mix of both, we'll add
the numeric entries from the array and only the string entries from the
properties table. To implement this we simply check if the key from the
properties table is a string.
nielsdos added a commit that referenced this issue Mar 24, 2023
* PHP-8.2:
  Fix GH-10907: Unable to serialize processed SplFixedArrays in PHP 8.2.4
  Fix GH-8979: Possible Memory Leak with SSL-enabled MySQL connections
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants