Skip to content

Commit 6e73f5a

Browse files
jmarblejmarble
committed
Fix SORT_REGULAR transitivity for mixed types
Fixes GH-20262: SORT_REGULAR now uses transitive comparison for consistent sorting of mixed numeric/non-numeric strings and types. Added EG(transitive_compare_mode) flag with save/restore pattern to enforce ordering: non-numeric < numeric-strings < numeric-types. This fixes sort() and array_unique() inconsistencies with mixed types, nested arrays, and objects. Comparison operators unchanged.
1 parent be1993b commit 6e73f5a

File tree

5 files changed

+205
-2
lines changed

5 files changed

+205
-2
lines changed

‎Zend/zend_globals.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,10 @@ struct _zend_executor_globals {
301301
uint32_t num_errors;
302302
zend_error_info **errors;
303303

304+
/* If transitive_compare_mode is enabled, string comparisons in zendi_smart_strcmp
305+
* will enforce transitivity by consistently ordering numeric vs non-numeric strings. */
306+
bool transitive_compare_mode;
307+
304308
/* Override filename or line number of thrown errors and exceptions */
305309
zend_string *filename_override;
306310
zend_long lineno_override;

‎Zend/zend_operators.c‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2271,6 +2271,12 @@ static int compare_long_to_string(zend_long lval, zend_string *str) /* {{{ */
22712271
return ZEND_THREEWAY_COMPARE((double) lval, str_dval);
22722272
}
22732273

2274+
/* String is non-numeric. In transitive mode, enforce: non-numeric < numeric.
2275+
* Since lval is numeric and str is non-numeric, lval > str, so return 1. */
2276+
if (UNEXPECTED(EG(transitive_compare_mode))) {
2277+
return 1;
2278+
}
2279+
22742280
zend_string *lval_as_str = zend_long_to_str(lval);
22752281
int cmp_result = zend_binary_strcmp(
22762282
ZSTR_VAL(lval_as_str), ZSTR_LEN(lval_as_str), ZSTR_VAL(str), ZSTR_LEN(str));
@@ -2295,6 +2301,12 @@ static int compare_double_to_string(double dval, zend_string *str) /* {{{ */
22952301
return ZEND_THREEWAY_COMPARE(dval, str_dval);
22962302
}
22972303

2304+
/* String is non-numeric. In transitive mode, enforce: non-numeric < numeric.
2305+
* Since dval is numeric and str is non-numeric, dval > str, so return 1. */
2306+
if (UNEXPECTED(EG(transitive_compare_mode))) {
2307+
return 1;
2308+
}
2309+
22982310
zend_string *dval_as_str = zend_double_to_str(dval);
22992311
int cmp_result = zend_binary_strcmp(
23002312
ZSTR_VAL(dval_as_str), ZSTR_LEN(dval_as_str), ZSTR_VAL(str), ZSTR_LEN(str));
@@ -3425,8 +3437,17 @@ ZEND_API int ZEND_FASTCALL zendi_smart_strcmp(zend_string *s1, zend_string *s2)
34253437
zend_long lval1 = 0, lval2 = 0;
34263438
double dval1 = 0.0, dval2 = 0.0;
34273439

3428-
if ((ret1 = is_numeric_string_ex(s1->val, s1->len, &lval1, &dval1, false, &oflow1, NULL)) &&
3429-
(ret2 = is_numeric_string_ex(s2->val, s2->len, &lval2, &dval2, false, &oflow2, NULL))) {
3440+
ret1 = is_numeric_string_ex(s1->val, s1->len, &lval1, &dval1, false, &oflow1, NULL);
3441+
ret2 = is_numeric_string_ex(s2->val, s2->len, &lval2, &dval2, false, &oflow2, NULL);
3442+
3443+
/* When in transitive comparison mode (used by SORT_REGULAR), enforce transitivity
3444+
* by consistently ordering numeric vs non-numeric strings. */
3445+
if (UNEXPECTED(EG(transitive_compare_mode)) && (ret1 != 0) != (ret2 != 0)) {
3446+
/* One is numeric, one is not. Order: non-numeric < numeric */
3447+
return ret1 ? 1 : -1;
3448+
}
3449+
3450+
if (ret1 && ret2) {
34303451
#if ZEND_ULONG_MAX == 0xFFFFFFFF
34313452
if (oflow1 != 0 && oflow1 == oflow2 && dval1 - dval2 == 0. &&
34323453
((oflow1 == 1 && dval1 > 9007199254740991. /*0x1FFFFFFFFFFFFF*/)

‎ext/standard/array.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,7 +285,16 @@ static zend_always_inline int php_array_key_compare_string_locale_unstable_i(Buc
285285

286286
static zend_always_inline int php_array_data_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */
287287
{
288+
/* Enable transitive comparison mode for this comparison tree.
289+
* Save the previous state to handle reentrancy (e.g., usort with callback that calls sort). */
290+
bool old_transitive_mode = EG(transitive_compare_mode);
291+
EG(transitive_compare_mode) = true;
292+
288293
int result = zend_compare(&f->val, &s->val);
294+
295+
/* Restore previous state */
296+
EG(transitive_compare_mode) = old_transitive_mode;
297+
289298
/* Special enums handling for array_unique. We don't want to add this logic to zend_compare as
290299
* that would be observable via comparison operators. */
291300
zval *rhs = &s->val;

‎ext/standard/tests/array/sort_regular_transitive.phpt‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,88 @@
1+
--TEST--
2+
SORT_REGULAR uses transitive comparison for consistent sorting
3+
--FILE--
4+
<?php
5+
echo "Test 1: Scalar mixed numeric/non-numeric strings\n";
6+
$arr1 = ["5", "10", "3A"];
7+
sort($arr1, SORT_REGULAR);
8+
var_dump($arr1);
9+
10+
echo "\nTest 2: Same values, different order (should match)\n";
11+
$arr2 = ["3A", "10", "5"];
12+
sort($arr2, SORT_REGULAR);
13+
var_dump($arr2);
14+
15+
echo "\nTest 3: Nested arrays with mixed strings\n";
16+
$arr3 = [
17+
['streetNumber' => '5'],
18+
['streetNumber' => '10'],
19+
['streetNumber' => '3A'],
20+
];
21+
sort($arr3, SORT_REGULAR);
22+
foreach ($arr3 as $item) {
23+
echo $item['streetNumber'];
24+
if ($item !== end($arr3)) echo " ";
25+
}
26+
echo "\n";
27+
28+
echo "\nTest 4: Same nested arrays, reversed (should match)\n";
29+
$arr4 = [
30+
['streetNumber' => '3A'],
31+
['streetNumber' => '10'],
32+
['streetNumber' => '5'],
33+
];
34+
sort($arr4, SORT_REGULAR);
35+
foreach ($arr4 as $item) {
36+
echo $item['streetNumber'];
37+
if ($item !== end($arr4)) echo " ";
38+
}
39+
echo "\n";
40+
41+
echo "\nTest 5: array_unique with nested arrays\n";
42+
$arr5 = [
43+
['number' => '5'],
44+
['number' => '10'],
45+
['number' => '5'],
46+
['number' => '3A'],
47+
['number' => '5'],
48+
];
49+
$unique = array_unique($arr5, SORT_REGULAR);
50+
echo "Unique count: " . count($unique) . "\n";
51+
52+
echo "\nTest 6: Comparison operators remain unchanged\n";
53+
echo '"5" <=> "3A": ' . ("5" <=> "3A") . "\n";
54+
echo '"10" <=> "3A": ' . ("10" <=> "3A") . "\n";
55+
?>
56+
--EXPECT--
57+
Test 1: Scalar mixed numeric/non-numeric strings
58+
array(3) {
59+
[0]=>
60+
string(2) "3A"
61+
[1]=>
62+
string(1) "5"
63+
[2]=>
64+
string(2) "10"
65+
}
66+
67+
Test 2: Same values, different order (should match)
68+
array(3) {
69+
[0]=>
70+
string(2) "3A"
71+
[1]=>
72+
string(1) "5"
73+
[2]=>
74+
string(2) "10"
75+
}
76+
77+
Test 3: Nested arrays with mixed strings
78+
3A 5 10
79+
80+
Test 4: Same nested arrays, reversed (should match)
81+
3A 5 10
82+
83+
Test 5: array_unique with nested arrays
84+
Unique count: 3
85+
86+
Test 6: Comparison operators remain unchanged
87+
"5" <=> "3A": 1
88+
"10" <=> "3A": -1

‎ext/standard/tests/array/sort_regular_transitive_objects.phpt‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
--TEST--
2+
SORT_REGULAR with objects containing mixed numeric/non-numeric strings is transitive
3+
--FILE--
4+
<?php
5+
class Address {
6+
public function __construct(
7+
public string $streetNumber,
8+
public string $streetName
9+
) {}
10+
}
11+
12+
echo "Test 1: Objects with mixed numeric/non-numeric string properties\n";
13+
$addresses = [
14+
new Address('5', 'Main St'),
15+
new Address('10', 'Main St'),
16+
new Address('3A', 'Main St'),
17+
];
18+
sort($addresses, SORT_REGULAR);
19+
echo "Result:";
20+
foreach ($addresses as $addr) {
21+
echo " " . $addr->streetNumber;
22+
}
23+
echo "\n";
24+
25+
echo "\nTest 2: Same objects, different order (should match Test 1)\n";
26+
$addresses2 = [
27+
new Address('3A', 'Main St'),
28+
new Address('10', 'Main St'),
29+
new Address('5', 'Main St'),
30+
];
31+
sort($addresses2, SORT_REGULAR);
32+
echo "Result:";
33+
foreach ($addresses2 as $addr) {
34+
echo " " . $addr->streetNumber;
35+
}
36+
echo "\n";
37+
38+
echo "\nTest 3: Another permutation (should match Test 1)\n";
39+
$addresses3 = [
40+
new Address('10', 'Main St'),
41+
new Address('5', 'Main St'),
42+
new Address('3A', 'Main St'),
43+
];
44+
sort($addresses3, SORT_REGULAR);
45+
echo "Result:";
46+
foreach ($addresses3 as $addr) {
47+
echo " " . $addr->streetNumber;
48+
}
49+
echo "\n";
50+
51+
echo "\nTest 4: array_unique() with objects\n";
52+
$addresses_with_dupes = [
53+
new Address('5', 'Main St'),
54+
new Address('10', 'Main St'),
55+
new Address('10', 'Main St'),
56+
new Address('3A', 'Main St'),
57+
new Address('5', 'Main St'),
58+
];
59+
$unique = array_unique($addresses_with_dupes, SORT_REGULAR);
60+
echo "Input: 5 objects (with duplicates)\n";
61+
echo "Output: " . count($unique) . " unique objects\n";
62+
echo "Street numbers:";
63+
foreach ($unique as $addr) {
64+
echo " " . $addr->streetNumber;
65+
}
66+
echo "\n";
67+
?>
68+
--EXPECT--
69+
Test 1: Objects with mixed numeric/non-numeric string properties
70+
Result: 3A 5 10
71+
72+
Test 2: Same objects, different order (should match Test 1)
73+
Result: 3A 5 10
74+
75+
Test 3: Another permutation (should match Test 1)
76+
Result: 3A 5 10
77+
78+
Test 4: array_unique() with objects
79+
Input: 5 objects (with duplicates)
80+
Output: 3 unique objects
81+
Street numbers: 5 10 3A

0 commit comments

Comments
 (0)