ext/intl: Refactor ResourceBundle get and dimension access (#13503)

Author: Girgias

NEWS

@@ -62,6 +62,13 @@ PHP                                                                        NEWS
   . Added IntlDateFormatter::getIanaID/intltz_get_iana_id method/function.
     (David Carlier)
   . Set to C++17 standard for icu 74 and onwards. (David Carlier)
+  . resourcebundle_get(), ResourceBundle::get(), and accessing offsets on a
+    ResourceBundle object now throw:
+    - TypeError for invalid offset types
+    - ValueError for an empty string
+    - ValueError if the integer index does not fit in a signed 32 bit integer
+  . ResourceBundle::get() now has a tentative return type of:
+    ResourceBundle|array|string|int|null
 
 - LDAP:
   . Added LDAP_OPT_X_TLS_PROTOCOL_MAX/LDAP_OPT_X_TLS_PROTOCOL_TLS1_3

UPGRADING

@@ -45,6 +45,13 @@ PHP 8.4 UPGRADE NOTES
     object. This is no longer possible, and cloning a DOMXPath object now throws
     an error.
 
+- Intl:
+  . resourcebundle_get(), ResourceBundle::get(), and accessing offsets on a
+    ResourceBundle object now throw:
+    - TypeError for invalid offset types
+    - ValueError for an empty string
+    - ValueError if the integer index does not fit in a signed 32 bit integer
+
 - MBString:
   . mb_encode_numericentity() and mb_decode_numericentity() now check that
     the $map is only composed of integers, if not a ValueError is thrown.
@@ -321,6 +328,8 @@ PHP 8.4 UPGRADE NOTES
     RFC: https://wiki.php.net/rfc/new_rounding_modes_to_round_function
   . NumberFormatter::ROUND_HALFODD added to complement existing
     NumberFormatter::ROUND_HALFEVEN functionality.
+  . ResourceBundle::get() now has a tentative return type of:
+    ResourceBundle|array|string|int|null
 
 - MBString:
   . The behavior of mb_strcut is more consistent now on invalid UTF-8 and UTF-16

ext/intl/php_intl.stub.php

@@ -543,8 +543,7 @@ function normalizer_get_raw_decomposition(string $string, int $form = Normalizer
 
 function resourcebundle_create(?string $locale, ?string $bundle, bool $fallback = true): ?ResourceBundle {}
 
-/** @param string|int $index */
-function resourcebundle_get(ResourceBundle $bundle, $index, bool $fallback = true): mixed {}
+function resourcebundle_get(ResourceBundle $bundle, string|int $index, bool $fallback = true): ResourceBundle|array|string|int|null {}
 
 function resourcebundle_count(ResourceBundle $bundle): int {}
 

ext/intl/php_intl_arginfo.h

@@ -71,11 +71,7 @@ void resourcebundle_extract_value( zval *return_value, ResourceBundle_object *so
 			source->child = NULL;
 			intl_errors_reset(INTL_DATA_ERROR_P(source));
 			break;
-
-		default:
-			intl_errors_set(INTL_DATA_ERROR_P(source), U_ILLEGAL_ARGUMENT_ERROR, "Unknown resource type", 0);
-			RETURN_FALSE;
-			break;
+		EMPTY_SWITCH_DEFAULT_CASE();
 	}
 }
 /* }}} */

ext/intl/resourcebundle/resourcebundle.c

@@ -13,12 +13,8 @@ public function __construct(?string $locale, ?string $bundle, bool $fallback = t
      */
     public static function create(?string $locale, ?string $bundle, bool $fallback = true): ?ResourceBundle {}
 
-    /**
-     * @param string|int $index
-     * @tentative-return-type
-     * @alias resourcebundle_get
-     */
-    public function get($index, bool $fallback = true): mixed {}
+    /** @tentative-return-type */
+    public function get(string|int $index, bool $fallback = true): ResourceBundle|array|string|int|null {}
 
     /**
      * @tentative-return-type

ext/intl/resourcebundle/resourcebundle.stub.php

@@ -168,84 +168,134 @@ PHP_FUNCTION( resourcebundle_create )
 /* }}} */
 
 /* {{{ resourcebundle_array_fetch */
-static void resourcebundle_array_fetch(zend_object *object, zval *offset, zval *return_value, int fallback)
+static zval *resource_bundle_array_fetch(
+	zend_object *object, zend_string *offset_str, zend_long offset_int,
+	zval *return_value, bool fallback, uint32_t offset_arg_num)
 {
-	int32_t     meindex = 0;
-	char *      mekey = NULL;
-	bool        is_numeric = 0;
-	char        *pbuf;
+	int32_t index = 0;
+	char *key = NULL;
+	bool is_numeric = offset_str == NULL;
+	char *pbuf;
 	ResourceBundle_object *rb;
 
 	rb = php_intl_resourcebundle_fetch_object(object);
 	intl_error_reset(NULL);
 	intl_error_reset(INTL_DATA_ERROR_P(rb));
 
-	if(Z_TYPE_P(offset) == IS_LONG) {
-		is_numeric = 1;
-		meindex = (int32_t)Z_LVAL_P(offset);
-		rb->child = ures_getByIndex( rb->me, meindex, rb->child, &INTL_DATA_ERROR_CODE(rb) );
-	} else if(Z_TYPE_P(offset) == IS_STRING) {
-		mekey = Z_STRVAL_P(offset);
-		rb->child = ures_getByKey(rb->me, mekey, rb->child, &INTL_DATA_ERROR_CODE(rb) );
+	if (offset_str) {
+		if (UNEXPECTED(ZSTR_LEN(offset_str) == 0)) {
+			if (offset_arg_num) {
+				zend_argument_value_error(offset_arg_num, "cannot be empty");
+			} else {
+				zend_value_error("Offset cannot be empty");
+			}
+			return NULL;
+		}
+		key = ZSTR_VAL(offset_str);
+		rb->child = ures_getByKey(rb->me, key, rb->child, &INTL_DATA_ERROR_CODE(rb) );
 	} else {
-		intl_errors_set(INTL_DATA_ERROR_P(rb), U_ILLEGAL_ARGUMENT_ERROR,
-			"resourcebundle_get: index should be integer or string", 0);
-		RETURN_NULL();
+		if (UNEXPECTED(offset_int < (zend_long)INT32_MIN || offset_int > (zend_long)INT32_MAX)) {
+			if (offset_arg_num) {
+				zend_argument_value_error(offset_arg_num, "index must be between %d and %d", INT32_MIN, INT32_MAX);
+			} else {
+				zend_value_error("Index must be between %d and %d", INT32_MIN, INT32_MAX);
+			}
+			return NULL;
+		}
+		index = (int32_t)offset_int;
+		rb->child = ures_getByIndex(rb->me, index, rb->child, &INTL_DATA_ERROR_CODE(rb));
 	}
 
 	intl_error_set_code( NULL, INTL_DATA_ERROR_CODE(rb) );
 	if (U_FAILURE(INTL_DATA_ERROR_CODE(rb))) {
 		if (is_numeric) {
-			spprintf( &pbuf, 0, "Cannot load resource element %d", meindex );
+			spprintf( &pbuf, 0, "Cannot load resource element %d", index );
 		} else {
-			spprintf( &pbuf, 0, "Cannot load resource element '%s'", mekey );
+			spprintf( &pbuf, 0, "Cannot load resource element '%s'", key );
 		}
 		intl_errors_set_custom_msg( INTL_DATA_ERROR_P(rb), pbuf, 1 );
 		efree(pbuf);
-		RETURN_NULL();
+		RETVAL_NULL();
+		return return_value;
 	}
 
 	if (!fallback && (INTL_DATA_ERROR_CODE(rb) == U_USING_FALLBACK_WARNING || INTL_DATA_ERROR_CODE(rb) == U_USING_DEFAULT_WARNING)) {
 		UErrorCode icuerror;
 		const char * locale = ures_getLocaleByType( rb->me, ULOC_ACTUAL_LOCALE, &icuerror );
 		if (is_numeric) {
-			spprintf( &pbuf, 0, "Cannot load element %d without fallback from to %s", meindex, locale );
+			spprintf(&pbuf, 0, "Cannot load element %d without fallback from to %s", index, locale);
 		} else {
-			spprintf( &pbuf, 0, "Cannot load element '%s' without fallback from to %s", mekey, locale );
+			spprintf(&pbuf, 0, "Cannot load element '%s' without fallback from to %s", key, locale);
 		}
-		intl_errors_set_custom_msg( INTL_DATA_ERROR_P(rb), pbuf, 1 );
+		intl_errors_set_custom_msg( INTL_DATA_ERROR_P(rb), pbuf, 1);
 		efree(pbuf);
-		RETURN_NULL();
+		RETVAL_NULL();
+		return return_value;
 	}
 
 	resourcebundle_extract_value( return_value, rb );
+	return return_value;
 }
 /* }}} */
 
 /* {{{ resourcebundle_array_get */
 zval *resourcebundle_array_get(zend_object *object, zval *offset, int type, zval *rv)
 {
-	if(offset == NULL) {
-		php_error( E_ERROR, "Cannot apply [] to ResourceBundle object" );
+	if (offset == NULL) {
+		zend_throw_error(NULL, "Cannot apply [] to ResourceBundle object");
+		return NULL;
+	}
+
+	ZVAL_DEREF(offset);
+	if (Z_TYPE_P(offset) == IS_LONG) {
+		return resource_bundle_array_fetch(object, /* offset_str */ NULL, Z_LVAL_P(offset), rv, /* fallback */ true, /* arg_num */ 0);
+	} else if (Z_TYPE_P(offset) == IS_STRING) {
+		return resource_bundle_array_fetch(object, Z_STR_P(offset), /* offset_int */ 0, rv, /* fallback */ true, /* arg_num */ 0);
+	} else {
+		zend_illegal_container_offset(object->ce->name, offset, type);
+		return NULL;
 	}
-	ZVAL_NULL(rv);
-	resourcebundle_array_fetch(object, offset, rv, 1);
-	return rv;
 }
 /* }}} */
 
 /* {{{ Get resource identified by numerical index or key name. */
 PHP_FUNCTION( resourcebundle_get )
 {
-	bool   fallback = 1;
-	zval *		offset;
-	zval *      object;
-
-	if (zend_parse_method_parameters(ZEND_NUM_ARGS(), getThis(), "Oz|b",	&object, ResourceBundle_ce_ptr, &offset, &fallback ) == FAILURE) {
+	bool fallback = true;
+	zend_object *resource_bundle = NULL;
+	zend_string *offset_str = NULL;
+	zend_long offset_long = 0;
+
+	ZEND_PARSE_PARAMETERS_START(2, 3)
+		Z_PARAM_OBJ_OF_CLASS(resource_bundle, ResourceBundle_ce_ptr)
+		Z_PARAM_STR_OR_LONG(offset_str, offset_long)
+		Z_PARAM_OPTIONAL
+		Z_PARAM_BOOL(fallback)
+	ZEND_PARSE_PARAMETERS_END();
+
+	zval *retval = resource_bundle_array_fetch(resource_bundle, offset_str, offset_long, return_value, fallback, /* arg_num */ 2);
+	if (!retval) {
 		RETURN_THROWS();
 	}
+}
+/* }}} */
 
-	resourcebundle_array_fetch(Z_OBJ_P(object), offset, return_value, fallback);
+PHP_METHOD(ResourceBundle , get)
+{
+	bool fallback = true;
+	zend_string *offset_str = NULL;
+	zend_long offset_long = 0;
+
+	ZEND_PARSE_PARAMETERS_START(1, 2)
+		Z_PARAM_STR_OR_LONG(offset_str, offset_long)
+		Z_PARAM_OPTIONAL
+		Z_PARAM_BOOL(fallback)
+	ZEND_PARSE_PARAMETERS_END();
+
+	zval *retval = resource_bundle_array_fetch(Z_OBJ_P(ZEND_THIS), offset_str, offset_long, return_value, fallback, /* arg_num */ 1);
+	if (!retval) {
+		RETURN_THROWS();
+	}
 }
 /* }}} */
 

ext/intl/resourcebundle/resourcebundle_arginfo.h

@@ -23,6 +23,11 @@ intl
     $r2 = $r['testarray'];
     printf( "testarray: %s\n", $r2[2] );
 
+    echo "Using a reference as an offset:\n";
+    $offset = 'teststring';
+    $ref = &$offset;
+    var_dump($r[$ref]);
+
     $t = $r['nonexisting'];
     echo debug( $t );
 ?>
@@ -46,5 +51,7 @@ Array
 testbin: a1b2c3d4e5f67890
 testtable: 3
 testarray: string 3
+Using a reference as an offset:
+string(12) "Hello World!"
 NULL
     2: Cannot load resource element 'nonexisting': U_MISSING_RESOURCE_ERROR

ext/intl/resourcebundle/resourcebundle_class.c

@@ -0,0 +1,61 @@
+--TEST--
+Test ResourceBundle errors with []
+--EXTENSIONS--
+intl
+--FILE--
+<?php
+include "resourcebundle.inc";
+
+// fall back
+$r = new ResourceBundle( 'en_US', BUNDLE );
+
+try {
+    $ref = &$r[];
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump(isset($r['non-existent']));
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($r['non-existent'] ?? "default");
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($r[12.5]);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($r[new stdClass()]);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    var_dump($r['']);
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+try {
+    /* This can only happen on 64bit */
+    if (PHP_INT_SIZE > 4) {
+        var_dump($r[0xFFFFFFFFF]);
+    } else {
+        echo 'ValueError: Index must be between -2147483648 and 2147483647', PHP_EOL;
+    }
+} catch (\Throwable $e) {
+    echo $e::class, ': ', $e->getMessage(), PHP_EOL;
+}
+
+?>
+--EXPECT--
+Error: Cannot apply [] to ResourceBundle object
+Error: Cannot use object of type ResourceBundle as array
+string(7) "default"
+TypeError: Cannot access offset of type float on ResourceBundle
+TypeError: Cannot access offset of type stdClass on ResourceBundle
+ValueError: Offset cannot be empty
+ValueError: Index must be between -2147483648 and 2147483647