Skip to content

Keep compiled RuleBasedBreakIterator rules alive for the iterator #23322

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

Open
iliaal wants to merge 1 commit into
base: PHP-8.4
Choose a base branch
from
Open

Keep compiled RuleBasedBreakIterator rules alive for the iterator #23322

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions NEWS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ PHP NEWS
. Fixed bug GH-15375 (Nested "yield from" skips items after a valid() or
next() call on the inner generator). (iliaal)

- Intl:
. Fixed a use-after-free when IntlRuleBasedBreakIterator is constructed
from compiled rules. (iliaal)

- Opcache:
. Fixed opcache.protect_memory race under ZTS. (realFlowControl)

Expand Down
8 changes: 8 additions & 0 deletions ext/intl/breakiterator/breakiterator_class.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,9 @@ static zend_object *BreakIterator_clone_obj(zend_object *object)
} else {
bio_new->biter = new_biter;
ZVAL_COPY(&bio_new->text, &bio_orig->text);
if (bio_orig->compiled_rules) {
bio_new->compiled_rules = zend_string_copy(bio_orig->compiled_rules);
}
}
} else {
zend_throw_error(NULL, "Cannot clone uninitialized BreakIterator");
Expand Down Expand Up @@ -163,6 +166,7 @@ static void breakiterator_object_init(BreakIterator_object *bio)
{
intl_error_init(BREAKITER_ERROR_P(bio));
bio->biter = NULL;
bio->compiled_rules = NULL;
ZVAL_UNDEF(&bio->text);
}
/* }}} */
Expand All @@ -177,6 +181,10 @@ static void BreakIterator_objects_free(zend_object *object)
delete bio->biter;
bio->biter = NULL;
}
if (bio->compiled_rules) {
zend_string_release(bio->compiled_rules);
bio->compiled_rules = NULL;
}
intl_error_reset(BREAKITER_ERROR_P(bio));

zend_object_std_dtor(&bio->zo);
Expand Down
2 changes: 2 additions & 0 deletions ext/intl/breakiterator/breakiterator_class.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ typedef struct {
// current text
zval text;

zend_string *compiled_rules;

zend_object zo;
} BreakIterator_object;

Expand Down
12 changes: 7 additions & 5 deletions ext/intl/breakiterator/rulebasedbreakiterator_methods.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,14 @@ static inline RuleBasedBreakIterator *fetch_rbbi(BreakIterator_object *bio) {

static void _php_intlrbbi_constructor_body(INTERNAL_FUNCTION_PARAMETERS, zend_error_handling *error_handling, bool *error_handling_replaced)
{
char *rules;
size_t rules_len;
zend_string *rules;
bool compiled = false;
UErrorCode status = U_ZERO_ERROR;
BREAKITER_METHOD_INIT_VARS;
object = ZEND_THIS;

ZEND_PARSE_PARAMETERS_START(1, 2)
Z_PARAM_STRING(rules, rules_len)
Z_PARAM_STR(rules)
Z_PARAM_OPTIONAL
Z_PARAM_BOOL(compiled)
ZEND_PARSE_PARAMETERS_END();
Expand All @@ -62,7 +61,7 @@ static void _php_intlrbbi_constructor_body(INTERNAL_FUNCTION_PARAMETERS, zend_er
if (!compiled) {
UnicodeString rulesStr;
UParseError parseError = UParseError();
if (intl_stringFromChar(rulesStr, rules, rules_len, &status)
if (intl_stringFromChar(rulesStr, ZSTR_VAL(rules), ZSTR_LEN(rules), &status)
== FAILURE) {
zend_throw_exception(IntlException_ce_ptr,
"IntlRuleBasedBreakIterator::__construct(): "
Expand All @@ -84,7 +83,7 @@ static void _php_intlrbbi_constructor_body(INTERNAL_FUNCTION_PARAMETERS, zend_er
RETURN_THROWS();
}
} else { // compiled
rbbi = new RuleBasedBreakIterator((uint8_t*)rules, rules_len, status);
rbbi = new RuleBasedBreakIterator(reinterpret_cast<uint8_t *>(ZSTR_VAL(rules)), ZSTR_LEN(rules), status);
if (U_FAILURE(status)) {
zend_throw_exception(IntlException_ce_ptr,
"IntlRuleBasedBreakIterator::__construct(): "
Expand All @@ -95,6 +94,9 @@ static void _php_intlrbbi_constructor_body(INTERNAL_FUNCTION_PARAMETERS, zend_er
}

breakiterator_object_create(return_value, rbbi, 0);
if (compiled) {
Z_INTL_BREAKITERATOR_P(return_value)->compiled_rules = zend_string_copy(rules);
}
}

U_CFUNC PHP_METHOD(IntlRuleBasedBreakIterator, __construct)
Expand Down
50 changes: 50 additions & 0 deletions ext/intl/tests/rbbiter_compiled_rules_lifetime.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
--TEST--
IntlRuleBasedBreakIterator compiled rules outlive the source string
--EXTENSIONS--
intl
--SKIPIF--
<?php if (version_compare(INTL_ICU_VERSION, '68.1') < 0) die('skip for ICU >= 68.1'); ?>
--FILE--
<?php

$rules = <<<RULES
\$LN = [[:letter:] [:number:]];
\$S = [.;,:];

!!forward;
\$LN+ {1};
\$S+ {42};
!!reverse;
\$LN+ {1};
\$S+ {42};
!!safe_forward;
!!safe_reverse;
RULES;

$src = new IntlRuleBasedBreakIterator($rules);

@devnexen devnexen Aug 16, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the fix is good enough, the test needs to truly trigger the case we re interested in. You need to force rules reclaims here.

$it = new IntlRuleBasedBreakIterator($src->getBinaryRules(), true);
unset($src);

$it->setText('ab,cd');
echo $it->first(), "\n";
while (true) {
$n = $it->next();
if ($n === IntlBreakIterator::DONE) {
break;
}
echo $n, "\n";
}

$clone = clone $it;
$clone->setText('xy');
echo $clone->first(), "\n";
echo $clone->next(), "\n";

?>
--EXPECT--
0
2
3
5
0
2
Loading