From cd1aed8eddd2dccbb55fb1bc3d67b23a8455248c Mon Sep 17 00:00:00 2001 From: Denis Vaksman Date: Thu, 11 Aug 2022 17:12:13 +0200 Subject: [PATCH] Fix GH-9296: `ksort` behaves incorrectly on arrays with mixed keys The comparator function used at ksort in SORT_REGULAR mode need to be consistent with basic comparison rules. These rules were changed in PHP-8.0 for numeric strings, but comparator used at ksort kept the old behaviour. It leads to inconsistent situations, when after ksort the first key is GREATER than some of the next ones by according to the basic comparison operators. Closes GH-9293. --- NEWS | 2 + ext/standard/array.c | 53 +++++++----------- ext/standard/tests/array/002.phpt | 20 +++---- ext/standard/tests/array/gh9296.phpt | 17 ++++++ .../tests/array/krsort_variation8.phpt | Bin 3121 -> 3121 bytes .../tests/array/krsort_variation9.phpt | 8 +-- .../tests/array/ksort_variation8.phpt | Bin 3107 -> 3107 bytes .../tests/array/ksort_variation9.phpt | 8 +-- 8 files changed, 56 insertions(+), 52 deletions(-) create mode 100644 ext/standard/tests/array/gh9296.phpt diff --git a/NEWS b/NEWS index d71a5c718c3f3..2f5bd4626bdf4 100644 --- a/NEWS +++ b/NEWS @@ -23,6 +23,8 @@ PHP NEWS - Standard: . Fixed bug GH-9017 (php_stream_sock_open_from_socket could return NULL). (Heiko Weber) + . Fixed bug GH-9296 (`ksort` behaves incorrectly on arrays with mixed keys). + (Denis Vaksman) - Streams: . Fixed bug GH-8472 (The resource returned by stream_socket_accept may have diff --git a/ext/standard/array.c b/ext/standard/array.c index 32ad1b0eaa6ed..d145661e3782e 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -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); } /* }}} */ diff --git a/ext/standard/tests/array/002.phpt b/ext/standard/tests/array/002.phpt index fdb838deb19dd..5cf377e2a3538 100644 --- a/ext/standard/tests/array/002.phpt +++ b/ext/standard/tests/array/002.phpt @@ -276,6 +276,8 @@ array(8) { -- Testing krsort() -- No second argument: array(8) { + ["test"]=> + int(27) [16777216]=> float(-0.3333333333333333) [1001]=> @@ -288,8 +290,6 @@ array(8) { string(4) "Test" [0]=> string(3) "PHP" - ["test"]=> - int(27) [-1000]=> array(2) { [0]=> @@ -300,6 +300,8 @@ array(8) { } Using SORT_REGULAR: array(8) { + ["test"]=> + int(27) [16777216]=> float(-0.3333333333333333) [1001]=> @@ -312,8 +314,6 @@ array(8) { string(4) "Test" [0]=> string(3) "PHP" - ["test"]=> - int(27) [-1000]=> array(2) { [0]=> @@ -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]=> @@ -383,8 +383,6 @@ array(8) { } [0]=> string(3) "PHP" - ["test"]=> - int(27) [5]=> string(4) "Test" [17]=> @@ -395,6 +393,8 @@ array(8) { string(6) "monkey" [16777216]=> float(-0.3333333333333333) + ["test"]=> + int(27) } Using SORT_REGULAR: array(8) { @@ -407,8 +407,6 @@ array(8) { } [0]=> string(3) "PHP" - ["test"]=> - int(27) [5]=> string(4) "Test" [17]=> @@ -419,6 +417,8 @@ array(8) { string(6) "monkey" [16777216]=> float(-0.3333333333333333) + ["test"]=> + int(27) } Using SORT_NUMERIC: array(8) { diff --git a/ext/standard/tests/array/gh9296.phpt b/ext/standard/tests/array/gh9296.phpt new file mode 100644 index 0000000000000..cfbc5cbbb09ec --- /dev/null +++ b/ext/standard/tests/array/gh9296.phpt @@ -0,0 +1,17 @@ +--TEST-- +GH-9296: incorrect ksort(..., SORT_REGULAR) behaviour on arrays with numeric and string keys +--FILE-- + 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) diff --git a/ext/standard/tests/array/krsort_variation8.phpt b/ext/standard/tests/array/krsort_variation8.phpt index 1483e6fd1bbc18abe1c129d73043254e66ee6dc7..39e72e2eb22211e4f3bb45963fc3ae7c3dab99d7 100644 GIT binary patch delta 107 zcmdleu~A}!DBEUdHhZSYqHKDTk8+z%7UGeRR8WXkQi`>;<5Ey4E-A{)OV=>aR8Ugl Zs-3)w)orpKrxu15d7N6fT=Mf$xd8tI8>j#P delta 113 zcmdleu~A}!DBI)^4!zByZ1zl(k8+z%7UdC@QBa81HHx*h<5Eya%gIkH(a<&0v$Oz0 bO|II>t61G8>v3vf=u*O`i_0ZHFO>@bu$dh& diff --git a/ext/standard/tests/array/krsort_variation9.phpt b/ext/standard/tests/array/krsort_variation9.phpt index 4364911b62127..b0309f373d362 100644 --- a/ext/standard/tests/array/krsort_variation9.phpt +++ b/ext/standard/tests/array/krsort_variation9.phpt @@ -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 -- diff --git a/ext/standard/tests/array/ksort_variation8.phpt b/ext/standard/tests/array/ksort_variation8.phpt index 7b7e1c003e587897379970a72be8a1936fb5fce1..9a15c4dc3e75d7dd76ec29986cc84159096b381c 100644 GIT binary patch delta 91 zcmZ21u~=dQ2ixRSPKn80oHCQ^IAtd<*K`?!raf9G7y$XGjh9;@qQ R1x_u3O4)04FgRDC8f!goJx})aK>#e=32=prl1gQ w08x{clb=|kVW4MVV5m9Sg3EI9CQfa}+R5`+T_-DWYEi>9_F68N{Jc~y069b~q5uE@ diff --git a/ext/standard/tests/array/ksort_variation9.phpt b/ext/standard/tests/array/ksort_variation9.phpt index 9322af70f57fa..0023a9e0602ae 100644 --- a/ext/standard/tests/array/ksort_variation9.phpt +++ b/ext/standard/tests/array/ksort_variation9.phpt @@ -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" }