Fix IntlPartsIterator key off-by-one error

Closes GH-7734 Closes GH-8172
php · Mar 25, 2022 · e706d02 · e706d02
1 parent 62a1c06
commit e706d02
Show file tree

Hide file tree

Showing 5 changed files with 66 additions and 10 deletions.
diff --git a/NEWS b/NEWS
@@ -13,6 +13,8 @@ PHP                                                                        NEWS
   . Fixed bug GH-8115 (Can't catch arg type deprecation when instantiating Intl
     classes). (ilutov)
   . Fixed bug GH-8142 (Compilation error on cygwin). (David Carlier)
+  . Fixed bug GH-7734 (Fix IntlPartsIterator key off-by-one error and first
+    key). (ilutov)
 
 - MBString:
   . Fixed bug GH-8208 (mb_encode_mimeheader: $indent functionality broken).

diff --git a/ext/intl/breakiterator/breakiterator_iterators.cpp b/ext/intl/breakiterator/breakiterator_iterators.cpp
@@ -127,6 +127,7 @@ typedef struct zoi_break_iter_parts {
 	zoi_with_current zoi_cur;
 	parts_iter_key_type key_type;
 	BreakIterator_object *bio; /* so we don't have to fetch it all the time */
+	zend_ulong index_right;
 } zoi_break_iter_parts;
 
 static void _breakiterator_parts_destroy_it(zend_object_iterator *iter)
@@ -136,8 +137,16 @@ static void _breakiterator_parts_destroy_it(zend_object_iterator *iter)
 
 static void _breakiterator_parts_get_current_key(zend_object_iterator *iter, zval *key)
 {
-	/* the actual work is done in move_forward and rewind */
-	ZVAL_LONG(key, iter->index);
+	// The engine resets the iterator index to -1 after rewinding. When using
+	// PARTS_ITERATOR_KEY_RIGHT we store it in zoi_break_iter_parts.index_right
+	// so it doesn't get lost.
+	zoi_break_iter_parts *zoi_bit = (zoi_break_iter_parts*)iter;
+
+	if (zoi_bit->key_type == PARTS_ITERATOR_KEY_RIGHT && iter->index == 0) {
+		ZVAL_LONG(key, zoi_bit->index_right);
+	} else {
+		ZVAL_LONG(key, iter->index);
+	}
 }
 
 static void _breakiterator_parts_move_forward(zend_object_iterator *iter)
@@ -163,6 +172,7 @@ static void _breakiterator_parts_move_forward(zend_object_iterator *iter)
 		iter->index = cur;
 	} else if (zoi_bit->key_type == PARTS_ITERATOR_KEY_RIGHT) {
 		iter->index = next;
+		zoi_bit->index_right = next;
 	}
 	/* else zoi_bit->key_type == PARTS_ITERATOR_KEY_SEQUENTIAL
 	 * No need to do anything, the engine increments ->index */
@@ -229,6 +239,7 @@ void IntlIterator_from_BreakIterator_parts(zval *break_iter_zv,
 	assert(((zoi_break_iter_parts*)ii->iterator)->bio->biter != NULL);
 
 	((zoi_break_iter_parts*)ii->iterator)->key_type = key_type;
+	((zoi_break_iter_parts*)ii->iterator)->index_right = 0;
 }
 
 U_CFUNC PHP_METHOD(IntlPartsIterator, getBreakIterator)

diff --git a/ext/intl/common/common_enum.cpp b/ext/intl/common/common_enum.cpp
@@ -291,6 +291,7 @@ U_CFUNC void intl_register_IntlIterator_class(void)
 	IntlIterator_ce_ptr = register_class_IntlIterator(zend_ce_iterator);
 	IntlIterator_ce_ptr->create_object = IntlIterator_object_create;
 	IntlIterator_ce_ptr->get_iterator = IntlIterator_get_iterator;
+	IntlIterator_ce_ptr->ce_flags |= ZEND_ACC_REUSE_GET_ITERATOR;
 
 	memcpy(&IntlIterator_handlers, &std_object_handlers,
 		sizeof IntlIterator_handlers);

diff --git a/ext/intl/tests/breakiter_getPartsIterator_var1.phpt b/ext/intl/tests/breakiter_getPartsIterator_var1.phpt
@@ -33,24 +33,24 @@ array(5) {
 array(5) {
   [0]=>
   string(3) "foo"
-  [4]=>
+  [3]=>
   string(1) " "
-  [5]=>
+  [4]=>
   string(3) "bar"
-  [8]=>
+  [7]=>
   string(1) " "
-  [9]=>
+  [8]=>
   string(3) "tao"
 }
 array(5) {
   [3]=>
   string(3) "foo"
-  [5]=>
+  [4]=>
   string(1) " "
-  [8]=>
+  [7]=>
   string(3) "bar"
-  [9]=>
+  [8]=>
   string(1) " "
-  [12]=>
+  [11]=>
   string(3) "tao"
 }
diff --git a/ext/intl/tests/gh7734.phpt b/ext/intl/tests/gh7734.phpt
@@ -0,0 +1,42 @@
+--TEST--
+GH-7734 (IntlPartsIterator key is wrong for KEY_LEFT/KEY_RIGHT)
+--EXTENSIONS--
+intl
+--FILE--
+<?php
+
+$iter = \IntlBreakIterator::createCodePointInstance();
+$iter->setText('ABC');
+
+foreach ($iter->getPartsIterator(\IntlPartsIterator::KEY_SEQUENTIAL) as $key => $value) {
+    var_dump($key, $value);
+}
+
+foreach ($iter->getPartsIterator(\IntlPartsIterator::KEY_LEFT) as $key => $value) {
+    var_dump($key, $value);
+}
+
+foreach ($iter->getPartsIterator(\IntlPartsIterator::KEY_RIGHT) as $key => $value) {
+    var_dump($key, $value);
+}
+
+?>
+--EXPECT--
+int(0)
+string(1) "A"
+int(1)
+string(1) "B"
+int(2)
+string(1) "C"
+int(0)
+string(1) "A"
+int(1)
+string(1) "B"
+int(2)
+string(1) "C"
+int(1)
+string(1) "A"
+int(2)
+string(1) "B"
+int(3)
+string(1) "C"