Fix GH-13309: array hashes comparison, wrong buffer len calculation.

[devnexen]

php_array_key_compare_string_case_unstable_i has a typo for the second operand resulting in a wrong buffer size calculation.

Issue reported by @AlexRudyuk

[nielsdos]

LGTM.
This is essentially the same bug as #13310, so might as well fix that too here?

[nielsdos]

@devnexen forgot to update this? https://github.com/php/php-src/blob/master/ext/standard/array.c#L241

[devnexen]

ah true sorry..

[bukka]

It should be Standard.

[bukka]

@devnexen Have you tried to analyse the issue and see if we can produce some test for this and what the impact is? It might point to some missing coverage if this has not be found for so long...

[bukka]

Ok I just had a quick look and it seems to me that this is not even used: php_array_key_compare_string_case_unstable_i so it's not really a bug...

[bukka]

So there are follow ups and the part that matter and needs some further analysis is merged in b06d6db . There is actually one more behind as well for master which is the same so it took me a little while to get my head around all those commits. I would recommend to wait with merging a little bit next time to make sure everything is done in a single commit.

[bukka]

Also please create a PR for those sort of changes even if it's a follow up so we can be sure that it runs fine all the tests - this PR is actually pointless because it is for unused code but for all used code there should be always a PR.

[devnexen]

Also please create a PR for those sort of changes even if it's a follow up so we can be sure that it runs fine all the tests - this PR is actually pointless because it is for unused code but for all used code there should be always a PR.

In fact it should have belonged in the same PR, I forgot to add this change. But agreed for the rest.

[bukka]

I have just done some analysis and don't think this is actually a big problem (except the int overflow). The code that triggers that place is following:

<?php

$a = [
    '11111111111111111111111111111111111111111' => 1,
    12 => 2
];

var_dump(ksort($a, SORT_NATURAL));

The first element does not really matter - I just tried larger string to be sure that the comparison does not continue. The thing is that number gets written to the buffer that is null terminated so the l2 length (which is effectively the int overflow). But due to the fact that l1 is correct and the string is null terminated, it is not going to overflow the buffer. At least that how it looked to me from debugging.