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

@@ -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

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

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) {

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--
+<?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)

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 --

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"
 }