Skip to content

Commit d43a043

Browse files
iliaaliliaal
committed
ext/intl: Reject unconstructed Collator in attribute and strength methods
Collator::getAttribute(), setAttribute(), getStrength() and setStrength() dereferenced a NULL ICU collator when called on an object whose constructor skipped parent::__construct(), returning bogus values instead of failing, while compare(), getLocale(), sort() and getSortKey() already throw "Object not initialized". Apply the same guard to the four remaining methods through a collator_check_initialized() helper, which also replaces the four identical copies in collator_compare.c, collator_locale.c and collator_sort.c.
1 parent 9c2acc3 commit d43a043

7 files changed

Lines changed: 93 additions & 24 deletions

File tree

‎NEWS‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ PHP NEWS
2424
. Fixed cloning IntlDateFormatter and MessageFormatter losing PHP-side state
2525
such as dateType, timeType, calendar and the message pattern.
2626
(Ilia Alshanetsky)
27+
. Fixed Collator attribute and strength methods not rejecting an
28+
unconstructed Collator. (Ilia Alshanetsky)
2729

2830
- Lexbor:
2931
. Merge patches lexbor/lexbor@8a14bc0 and lexbor/lexbor@f67ce4b, fixing a

‎ext/intl/collator/collator_attr.c‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ PHP_FUNCTION( collator_get_attribute )
4040
/* Fetch the object. */
4141
COLLATOR_METHOD_FETCH_OBJECT;
4242

43+
if (collator_check_initialized(co) == FAILURE) {
44+
RETURN_THROWS();
45+
}
46+
4347
value = ucol_getAttribute( co->ucoll, attribute, COLLATOR_ERROR_CODE_P( co ) );
4448
COLLATOR_CHECK_STATUS( co, "Error getting attribute value" );
4549

@@ -64,6 +68,10 @@ PHP_FUNCTION( collator_set_attribute )
6468
/* Fetch the object. */
6569
COLLATOR_METHOD_FETCH_OBJECT;
6670

71+
if (collator_check_initialized(co) == FAILURE) {
72+
RETURN_THROWS();
73+
}
74+
6775
/* Set new value for the given attribute. */
6876
ucol_setAttribute( co->ucoll, attribute, value, COLLATOR_ERROR_CODE_P( co ) );
6977
COLLATOR_CHECK_STATUS( co, "Error setting attribute value" );
@@ -87,6 +95,10 @@ PHP_FUNCTION( collator_get_strength )
8795
/* Fetch the object. */
8896
COLLATOR_METHOD_FETCH_OBJECT;
8997

98+
if (collator_check_initialized(co) == FAILURE) {
99+
RETURN_THROWS();
100+
}
101+
90102
/* Get current strength and return it. */
91103
RETURN_LONG( ucol_getStrength( co->ucoll ) );
92104
}
@@ -109,6 +121,10 @@ PHP_FUNCTION( collator_set_strength )
109121
/* Fetch the object. */
110122
COLLATOR_METHOD_FETCH_OBJECT;
111123

124+
if (collator_check_initialized(co) == FAILURE) {
125+
RETURN_THROWS();
126+
}
127+
112128
/* Set given strength. */
113129
ucol_setStrength( co->ucoll, strength );
114130

‎ext/intl/collator/collator_class.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,22 @@ static inline Collator_object *php_intl_collator_fetch_object(zend_object *obj)
4646
}
4747
#define Z_INTL_COLLATOR_P(zv) php_intl_collator_fetch_object(Z_OBJ_P(zv))
4848

49+
static zend_always_inline zend_result collator_check_initialized(Collator_object *co)
50+
{
51+
ZEND_ASSERT(co != NULL);
52+
53+
if (UNEXPECTED(co->ucoll == NULL)) {
54+
intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) );
55+
intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ),
56+
"Object not initialized", 0 );
57+
zend_throw_error(NULL, "Object not initialized");
58+
59+
return FAILURE;
60+
}
61+
62+
return SUCCESS;
63+
}
64+
4965
void collator_register_Collator_symbols(int module_number);
5066
void collator_object_init( Collator_object* co );
5167
void collator_object_destroy( Collator_object* co );

‎ext/intl/collator/collator_compare.c‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,7 @@ PHP_FUNCTION( collator_compare )
4848
/* Fetch the object. */
4949
COLLATOR_METHOD_FETCH_OBJECT;
5050

51-
if (!co || !co->ucoll) {
52-
intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) );
53-
intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ),
54-
"Object not initialized", 0 );
55-
zend_throw_error(NULL, "Object not initialized");
56-
51+
if (collator_check_initialized(co) == FAILURE) {
5752
RETURN_THROWS();
5853
}
5954

‎ext/intl/collator/collator_locale.c‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,12 +41,7 @@ PHP_FUNCTION( collator_get_locale )
4141
/* Fetch the object. */
4242
COLLATOR_METHOD_FETCH_OBJECT;
4343

44-
if (!co || !co->ucoll) {
45-
intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) );
46-
intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ),
47-
"Object not initialized", 0 );
48-
zend_throw_error(NULL, "Object not initialized");
49-
44+
if (collator_check_initialized(co) == FAILURE) {
5045
RETURN_THROWS();
5146
}
5247

‎ext/intl/collator/collator_sort.c‎

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -376,12 +376,7 @@ PHP_FUNCTION( collator_sort_with_sort_keys )
376376
/* Fetch the object. */
377377
COLLATOR_METHOD_FETCH_OBJECT;
378378

379-
if (!co || !co->ucoll) {
380-
intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) );
381-
intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ),
382-
"Object not initialized", 0 );
383-
zend_throw_error(NULL, "Object not initialized");
384-
379+
if (collator_check_initialized(co) == FAILURE) {
385380
RETURN_THROWS();
386381
}
387382

@@ -532,12 +527,7 @@ PHP_FUNCTION( collator_get_sort_key )
532527
/* Fetch the object. */
533528
COLLATOR_METHOD_FETCH_OBJECT;
534529

535-
if (!co || !co->ucoll) {
536-
intl_error_set_code( NULL, COLLATOR_ERROR_CODE( co ) );
537-
intl_errors_set_custom_msg( COLLATOR_ERROR_P( co ),
538-
"Object not initialized", 0 );
539-
zend_throw_error(NULL, "Object not initialized");
540-
530+
if (collator_check_initialized(co) == FAILURE) {
541531
RETURN_THROWS();
542532
}
543533

‎ext/intl/tests/collator_attribute_unconstructed.phpt‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
--TEST--
2+
Collator attribute and strength methods on unconstructed object
3+
--EXTENSIONS--
4+
intl
5+
--FILE--
6+
<?php
7+
8+
class Collator2 extends Collator {
9+
public function __construct() {
10+
// omitting parent::__construct($someLocale);
11+
}
12+
}
13+
14+
$c = new Collator2();
15+
16+
$methods = [
17+
'getAttribute' => fn() => $c->getAttribute(Collator::NUMERIC_COLLATION),
18+
'setAttribute' => fn() => $c->setAttribute(Collator::NUMERIC_COLLATION, Collator::ON),
19+
'getStrength' => fn() => $c->getStrength(),
20+
'setStrength' => fn() => $c->setStrength(Collator::SECONDARY),
21+
];
22+
23+
foreach ($methods as $method => $call) {
24+
try {
25+
$call();
26+
} catch (Error $e) {
27+
echo $method, ': ', $e::class, ': ', $e->getMessage(), PHP_EOL;
28+
}
29+
}
30+
31+
$functions = [
32+
'collator_get_attribute' => fn() => collator_get_attribute($c, Collator::NUMERIC_COLLATION),
33+
'collator_set_attribute' => fn() => collator_set_attribute($c, Collator::NUMERIC_COLLATION, Collator::ON),
34+
'collator_get_strength' => fn() => collator_get_strength($c),
35+
'collator_set_strength' => fn() => collator_set_strength($c, Collator::SECONDARY),
36+
];
37+
38+
foreach ($functions as $function => $call) {
39+
try {
40+
$call();
41+
} catch (Error $e) {
42+
echo $function, ': ', $e::class, ': ', $e->getMessage(), PHP_EOL;
43+
}
44+
}
45+
46+
?>
47+
--EXPECT--
48+
getAttribute: Error: Object not initialized
49+
setAttribute: Error: Object not initialized
50+
getStrength: Error: Object not initialized
51+
setStrength: Error: Object not initialized
52+
collator_get_attribute: Error: Object not initialized
53+
collator_set_attribute: Error: Object not initialized
54+
collator_get_strength: Error: Object not initialized
55+
collator_set_strength: Error: Object not initialized

0 commit comments

Comments
 (0)