Problem with the compare function of the AbstractCollectionStrategy

[ArchinSoftware]

Recently we've updated this library and experience a problem with the compare function of the AbstractCollectionStrategy. Previously the compare function used the strcmp function, that is changed into the spaceship operator. For some specific hash values this results in an other outcome. For us this leads now into an incorrect list of items to be removed or added.

This is how it was: return strcmp(spl_object_hash($a), spl_object_hash($b));

That is changed in this commit in: return spl_object_hash($a) <=> spl_object_hash($b);

When the hash of $a is "00000000000007e60000000000000000" and the hash of $b is "00000000000053110000000000000000" we get different results. The "old" code (strcmp) results in -5. The "new" code (<=>) results in 1. We think the "old" way is correct. What would be the best way to fix this?

[TomHAnderson]

The docs for strcmp state

Returns -1 if string1 is less than string2; 1 if string1 is greater than string2, and 0 if they are equal.

so strcmp could not return a value -5. Was that a typo?

[ArchinSoftware]

Hi @TomHAnderson, the value -5 is correct, according to the docs that should be fixed in PHP 8.2:

This function now returns -1 or 1, where it previously returned a negative or positive number.

But strcmp still returns -5 in this case. But -1 or -5 doesn't matter, as they are both negative and the spaceship operator returns a positive value.

See the following code:

<?php

$hashA = '00000000000007e60000000000000000';
$hashB = '00000000000053110000000000000000';

var_dump($hashA <=> $hashB); // int(1)
var_dump(strcasecmp($hashA, $hashB)); // int(-5)

https://onlinephp.io?s=s7EvyCjg5eLlUslILM5wVLBVUDdABuapZgZoQN0aqtoJXbWpsaEhNtW8XGWJRfEppbkFGlBrbGztFCBmaFojyRaXFCUnFqcmwxXqwFRpWgMA&v=8.2.1%2C8.1.14%2C7.2.34

[TomHAnderson]

Thanks for the code. I believe you've found a bug in the spaceship operator. Your report, above, is correct but in theory the code we're using is syntactically correct. Take a look at this test:

https://onlinephp.io/c/63ee2 https://onlinephp.io/c/d04e5

As soon as the strings get to 16 characters <=> starts returning an incorrect value while strcasecmp works correctly. I look forward to your views on this.

A patch to revert to use strcasecmp is in order if you agree with my findings. You're welcome to submit that else I'll take care of it.

[ArchinSoftware]

Thanks for the testcase! Hmm, it does indeed look like a bug... I've created a PR to revert the use of the spaceship operator.

[TomHAnderson]

Submitted https://github.com/php/php-src/issues/10513

[driehle]

Bugfix 3.2.1 is now released! 🚢