Skip to content

Fix GH-9296: incorrect ksort(..., SORT_REGULAR) behaviour on arrays with numeric and string keys #9293

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.

Sign up for GitHub

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
wants to merge 4 commits into from
Closed

Fix GH-9296: incorrect ksort(..., SORT_REGULAR) behaviour on arrays with numeric and string keys #9293

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
10c2160
Fix GH-9296: `ksort` behaves incorrectly on arrays with string and nu…
DrDet Aug 8, 2022
5c4ef9a
Fix ksort tests to make expected results consistent with php8 compari…
DrDet Aug 9, 2022
aa125c8
Review fixes
DrDet Aug 11, 2022
23fc6d4
Add the test case for GH-9296
DrDet Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 19 additions & 34 deletions ext/standard/array.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,40 +166,25 @@ static zend_never_inline ZEND_COLD int stable_sort_fallback(Bucket *a, Bucket *b

static zend_always_inline int php_array_key_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */
{
zend_uchar t;
zend_long l1, l2;
double d;

if (f->key == NULL) {
if (s->key == NULL) {
return (zend_long)f->h > (zend_long)s->h ? 1 : -1;
} else {
l1 = (zend_long)f->h;
t = is_numeric_string(s->key->val, s->key->len, &l2, &d, 1);
if (t == IS_LONG) {
/* pass */
} else if (t == IS_DOUBLE) {
return ZEND_NORMALIZE_BOOL((double)l1 - d);
} else {
l2 = 0;
}
}
} else {
if (s->key) {
return zendi_smart_strcmp(f->key, s->key);
} else {
l2 = (zend_long)s->h;
t = is_numeric_string(f->key->val, f->key->len, &l1, &d, 1);
if (t == IS_LONG) {
/* pass */
} else if (t == IS_DOUBLE) {
return ZEND_NORMALIZE_BOOL(d - (double)l2);
} else {
l1 = 0;
}
}
}
return ZEND_NORMALIZE_BOOL(l1 - l2);
zval first;
zval second;

if (f->key == NULL && s->key == NULL) {
return (zend_long)f->h > (zend_long)s->h ? 1 : -1;
} else if (f->key && s->key) {
return zendi_smart_strcmp(f->key, s->key);
}
if (f->key) {
ZVAL_STR(&first, f->key);
} else {
ZVAL_LONG(&first, f->h);
}
if (s->key) {
ZVAL_STR(&second, s->key);
} else {
ZVAL_LONG(&second, s->h);
}
return zend_compare(&first, &second);
}
/* }}} */

Expand Down
20 changes: 10 additions & 10 deletions ext/standard/tests/array/002.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ array(8) {
-- Testing krsort() --
No second argument:
array(8) {
["test"]=>
int(27)
[16777216]=>
float(-0.3333333333333333)
[1001]=>
Expand All @@ -288,8 +290,6 @@ array(8) {
string(4) "Test"
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[-1000]=>
array(2) {
[0]=>
Expand All @@ -300,6 +300,8 @@ array(8) {
}
Using SORT_REGULAR:
array(8) {
["test"]=>
int(27)
[16777216]=>
float(-0.3333333333333333)
[1001]=>
Expand All @@ -312,8 +314,6 @@ array(8) {
string(4) "Test"
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[-1000]=>
array(2) {
[0]=>
Expand All @@ -334,10 +334,10 @@ array(8) {
string(27) "PHP: Hypertext Preprocessor"
[5]=>
string(4) "Test"
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[0]=>
string(3) "PHP"
[-1000]=>
array(2) {
[0]=>
Expand Down Expand Up @@ -383,8 +383,6 @@ array(8) {
}
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[5]=>
string(4) "Test"
[17]=>
Expand All @@ -395,6 +393,8 @@ array(8) {
string(6) "monkey"
[16777216]=>
float(-0.3333333333333333)
["test"]=>
int(27)
}
Using SORT_REGULAR:
array(8) {
Expand All @@ -407,8 +407,6 @@ array(8) {
}
[0]=>
string(3) "PHP"
["test"]=>
int(27)
[5]=>
string(4) "Test"
[17]=>
Expand All @@ -419,6 +417,8 @@ array(8) {
string(6) "monkey"
[16777216]=>
float(-0.3333333333333333)
["test"]=>
int(27)
}
Using SORT_NUMERIC:
array(8) {
Expand Down
17 changes: 17 additions & 0 deletions ext/standard/tests/array/gh9296.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
--TEST--
GH-9296: incorrect ksort(..., SORT_REGULAR) behaviour on arrays with numeric and string keys
--FILE--
<?php
$array = ["aaa" => 0, 600 => 1];
ksort($array, SORT_REGULAR);
var_dump($array);
var_dump(array_key_first($array) <=> array_key_last($array));
?>
--EXPECT--
array(2) {
[600]=>
int(1)
["aaa"]=>
int(0)
}
int(-1)
Binary file modified ext/standard/tests/array/krsort_variation8.phpt
View file
Open in desktop
Binary file not shown.
8 changes: 4 additions & 4 deletions ext/standard/tests/array/krsort_variation9.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,22 +82,22 @@ array(5) {
- With default sort flag -
bool(true)
array(3) {
["c"]=>
string(5) "apple"
["a"]=>
string(6) "orange"
[0]=>
string(6) "banana"
["c"]=>
string(5) "apple"
}
- Sort flag = SORT_REGULAR -
bool(true)
array(3) {
["c"]=>
string(5) "apple"
["a"]=>
string(6) "orange"
[0]=>
string(6) "banana"
["c"]=>
string(5) "apple"
}

-- Iteration 3 --
Expand Down
Binary file modified ext/standard/tests/array/ksort_variation8.phpt
View file
Open in desktop
Binary file not shown.
8 changes: 4 additions & 4 deletions ext/standard/tests/array/ksort_variation9.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,20 @@ array(5) {
- With default sort flag -
bool(true)
array(3) {
["a"]=>
string(6) "orange"
[0]=>
string(6) "banana"
["a"]=>
string(6) "orange"
["c"]=>
string(5) "apple"
}
- Sort flag = SORT_REGULAR -
bool(true)
array(3) {
["a"]=>
string(6) "orange"
[0]=>
string(6) "banana"
["a"]=>
string(6) "orange"
["c"]=>
string(5) "apple"
}
Expand Down