From: George Peter Banyard <girgias@php.net>
Date: Thu, 2 Apr 2020 19:57:38 +0000 (+0200)
Subject: Promote mb_str_split warning to ValueError
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=d44ee9112fa299cbc1600fe808ebcdf66be89c7c;p=php

Promote mb_str_split warning to ValueError

Also add a TODO about documenting this funcion on PHP.net
Convert some checks to assertions as if they don't hold something went wrong during memory allocation
Due to these changes this function cannot return false anymore, fix stubs accordingly
---

diff --git a/ext/mbstring/mbstring.c b/ext/mbstring/mbstring.c
index b9a07b2f55..46c09d3e8c 100644
--- a/ext/mbstring/mbstring.c
+++ b/ext/mbstring/mbstring.c
@@ -1862,6 +1862,7 @@ static int mbfl_split_output(int c, void *data)
     return 0;
 }
 
+/* TODO Document this function on php.net */
 PHP_FUNCTION(mb_str_split)
 {
 	zend_string *str, *encoding = NULL;
@@ -1879,8 +1880,8 @@ PHP_FUNCTION(mb_str_split)
 	ZEND_PARSE_PARAMETERS_END();
 
 	if (split_length <= 0) {
-		php_error_docref(NULL, E_WARNING, "The length of each segment must be greater than zero");
-		RETURN_FALSE;
+		zend_argument_value_error(2, "must be greater than 0");
+		RETURN_THROWS();
 	}
 
 	/* fill mbfl_string structure */
@@ -1945,10 +1946,8 @@ PHP_FUNCTION(mb_str_split)
 				mbfl_memory_device_output,
 				NULL,
 				&device);
-		/* if something wrong with the decoded */
-		if (decoder == NULL) {
-			RETURN_FALSE;
-		}
+		/* assert that nothing is wrong with the decoder */
+		ZEND_ASSERT(decoder != NULL);
 
 		/* wchar filter */
 		mbfl_string_init(&result_string); /* mbfl_string to store chunk in the callback */
@@ -1966,11 +1965,8 @@ PHP_FUNCTION(mb_str_split)
 				mbfl_split_output,
 				NULL,
 				&params);
-		/* if something wrong with the filter */
-		if (filter == NULL){
-			mbfl_convert_filter_delete(decoder); /* this will free allocated memory for the decoded */
-			RETURN_FALSE;
-		}
+		/* assert that nothing is wrong with the filter */
+		ZEND_ASSERT(filter != NULL);
 
 		while (p < last - 1) { /* cycle each byte except last with callback function */
 			(*filter->filter_function)(*p++, filter);
diff --git a/ext/mbstring/mbstring.stub.php b/ext/mbstring/mbstring.stub.php
index 5598aa8c12..6fccd72449 100644
--- a/ext/mbstring/mbstring.stub.php
+++ b/ext/mbstring/mbstring.stub.php
@@ -19,7 +19,7 @@ function mb_parse_str(string $encoded_string, &$result): bool {}
 
 function mb_output_handler(string $contents, int $status): string {}
 
-function mb_str_split(string $str, int $split_length = 1, string $encoding = UNKNOWN): array|false {}
+function mb_str_split(string $str, int $split_length = 1, string $encoding = UNKNOWN): array {}
 
 function mb_strlen(string $str, string $encoding = UNKNOWN): int|false {}
 
diff --git a/ext/mbstring/mbstring_arginfo.h b/ext/mbstring/mbstring_arginfo.h
index 8d37ae1cbe..69d50f581a 100644
--- a/ext/mbstring/mbstring_arginfo.h
+++ b/ext/mbstring/mbstring_arginfo.h
@@ -36,7 +36,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_mb_output_handler, 0, 2, IS_STRI
 	ZEND_ARG_TYPE_INFO(0, status, IS_LONG, 0)
 ZEND_END_ARG_INFO()
 
-ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_mb_str_split, 0, 1, MAY_BE_ARRAY|MAY_BE_FALSE)
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_mb_str_split, 0, 1, IS_ARRAY, 0)
 	ZEND_ARG_TYPE_INFO(0, str, IS_STRING, 0)
 	ZEND_ARG_TYPE_INFO(0, split_length, IS_LONG, 0)
 	ZEND_ARG_TYPE_INFO(0, encoding, IS_STRING, 0)
diff --git a/ext/mbstring/tests/mb_str_split_error_conditions.phpt b/ext/mbstring/tests/mb_str_split_error_conditions.phpt
new file mode 100644
index 0000000000..77f04aad09
--- /dev/null
+++ b/ext/mbstring/tests/mb_str_split_error_conditions.phpt
@@ -0,0 +1,33 @@
+--TEST--
+mb_str_split() error conditions
+--SKIPIF--
+<?php extension_loaded('mbstring') or die('skip mbstring not available'); ?>
+--FILE--
+<?php
+
+$string = "日本"; /* 2 chars */
+
+// Invalid split length
+try {
+    mb_str_split($string, 0);
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+try {
+    mb_str_split($string, -5);
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+
+//Invalid Encoding
+try {
+    mb_str_split($string, 1, "BAD_ENCODING");
+} catch (\ValueError $e) {
+    echo $e->getMessage() . \PHP_EOL;
+}
+
+?>
+--EXPECT--
+mb_str_split(): Argument #2 ($split_length) must be greater than 0
+mb_str_split(): Argument #2 ($split_length) must be greater than 0
+mb_str_split(): Argument #3 ($encoding) must be a valid encoding, "BAD_ENCODING" given