Add zend_ini_parse_quantity() and deprecate zend_atol(), zend_atoi()

zend_atol() and zend_atoi() don't just do number parsing.
They also check for a 'K', 'M', or 'G' at the end of the string,
and multiply the parsed value out accordingly.

Unfortunately, they ignore any other non-numerics between the
numeric component and the last character in the string.
This means that numbers such as the following are both valid
and non-intuitive in their final output.

* "123KMG" is interpreted as "123G" -> 132070244352
* "123G " is interpreted as "123 " -> 123
* "123GB" is interpreted as "123B" -> 123
* "123 I like tacos." is also interpreted as "123." -> 123

Currently, in php-src these functions are used only for parsing ini values.

In this change we deprecate zend_atol(), zend_atoi(), and introduce a new
function with the same behaviour, but with the ability to report invalid inputs
to the caller. The function's name also makes the behaviour less unexpected:
zend_ini_parse_quantity().

Co-authored-by: Sara Golemon <pollita@php.net>

Author: arnaud-lb

Zend/zend_ini.c

@@ -29,6 +29,8 @@ static HashTable *registered_zend_ini_directives;
 #define NO_VALUE_PLAINTEXT		"no value"
 #define NO_VALUE_HTML			"<i>no value</i>"
 
+#define ZEND_IS_WHITESPACE(c) (((c) == ' ') || ((c) == '\t') || ((c) == '\n') || ((c) == '\r') || ((c) == '\v') || ((c) == '\f'))
+
 /*
  * hash_apply functions
  */
@@ -495,6 +497,67 @@ ZEND_API bool zend_ini_parse_bool(zend_string *str)
 	}
 }
 
+ZEND_API zend_long zend_ini_parse_quantity(zend_string *value, zend_string **errstr) /* {{{ */
+{
+	char *digits_end = NULL;
+	char *str = ZSTR_VAL(value);
+	size_t str_len = ZSTR_LEN(value);
+
+	/* Ignore trailing whitespace */
+	while (str_len && ZEND_IS_WHITESPACE(str[str_len-1])) --str_len;
+	if (!str_len) return 0;
+
+	/* Perform following multiplications on unsigned to avoid overflow UB.
+	 * For now overflow is silently ignored -- not clear what else can be
+	 * done here, especially as the final result of this function may be
+	 * used in an unsigned context (e.g. "memory_limit=3G", which overflows
+	 * zend_long on 32-bit, but not size_t). */
+	zend_ulong retval = (zend_ulong) ZEND_STRTOL(str, &digits_end, 0);
+
+	if (digits_end == str) {
+		*errstr = zend_strpprintf(0, "Invalid numeric string '%.*s': no valid leading digits, interpreting as '0' for backwards compatibility",
+						(int)str_len, str);
+		return 0;
+	}
+
+	/* Allow for whitespace between integer portion and any suffix character */
+	while (ZEND_IS_WHITESPACE(*digits_end)) ++digits_end;
+
+	/* No exponent suffix. */
+	if (!*digits_end) return retval;
+
+	if (str_len>0) {
+		switch (str[str_len-1]) {
+			case 'g':
+			case 'G':
+				retval *= 1024;
+				ZEND_FALLTHROUGH;
+			case 'm':
+			case 'M':
+				retval *= 1024;
+				ZEND_FALLTHROUGH;
+			case 'k':
+			case 'K':
+				retval *= 1024;
+				break;
+		default:
+			/* Unknown suffix */
+			*errstr = zend_strpprintf(0, "Invalid numeric string '%.*s': unknown multipler '%c', interpreting as '%.*s' for backwards compatibility",
+						(int)str_len, str, str[str_len-1], (int)(digits_end - str), str);
+			return retval;
+		}
+	}
+
+	if (digits_end < &str[str_len-1]) {
+		/* More than one character in suffix */
+		*errstr = zend_strpprintf(0, "Invalid numeric string '%.*s', interpreting as '%.*s%c' for backwards compatibility",
+						(int)str_len, str, (int)(digits_end - str), str, str[str_len-1]);
+	}
+
+	return (zend_long) retval;
+}
+/* }}} */
+
 ZEND_INI_DISP(zend_ini_boolean_displayer_cb) /* {{{ */
 {
 	int value;

Zend/zend_ini.h

@@ -88,6 +88,28 @@ ZEND_API char *zend_ini_string_ex(const char *name, size_t name_length, int orig
 ZEND_API zend_string *zend_ini_get_value(zend_string *name);
 ZEND_API bool zend_ini_parse_bool(zend_string *str);
 
+/**
+ * Parses a quantity
+ *
+ * value must be a string of digits optionally followed by a multiplier
+ * character K, M, or G (for 2**10, 2**20, and 2**30, respectively).
+ *
+ * The digits are parsed as decimal unless the first character is '0', in which
+ * case they are parsed as octal.
+ *
+ * Whitespaces before and after the digits portion are ignored.
+ *
+ * For backwards compatibility, invalid values are handled as follows:
+ * - No leading digits: value is treated as '0'
+ * - Invalid multiplier: multiplier is ignored
+ * - Invalid characters between digits and multiplier: invalid characters are
+ *   ignored
+ *
+ * In all of these cases an error string is stored in *errstr (caller must
+ * release it).
+ */
+ZEND_API zend_long zend_ini_parse_quantity(zend_string *value, zend_string **errstr);
+
 ZEND_API zend_result zend_ini_register_displayer(const char *name, uint32_t name_length, void (*displayer)(zend_ini_entry *ini_entry, int type));
 
 ZEND_API ZEND_INI_DISP(zend_ini_boolean_displayer_cb);

Zend/zend_operators.h

@@ -468,8 +468,11 @@ ZEND_API int ZEND_FASTCALL zend_compare_symbol_tables(HashTable *ht1, HashTable
 ZEND_API int ZEND_FASTCALL zend_compare_arrays(zval *a1, zval *a2);
 ZEND_API int ZEND_FASTCALL zend_compare_objects(zval *o1, zval *o2);
 
-ZEND_API int ZEND_FASTCALL zend_atoi(const char *str, size_t str_len);
-ZEND_API zend_long ZEND_FASTCALL zend_atol(const char *str, size_t str_len);
+/** Deprecatd in favor of ZEND_STRTOL() */
+ZEND_ATTRIBUTE_DEPRECATED ZEND_API int ZEND_FASTCALL zend_atoi(const char *str, size_t str_len);
+
+/** Deprecatd in favor of ZEND_STRTOL() */
+ZEND_ATTRIBUTE_DEPRECATED ZEND_API zend_long ZEND_FASTCALL zend_atol(const char *str, size_t str_len);
 
 #define convert_to_null_ex(zv) convert_to_null(zv)
 #define convert_to_boolean_ex(zv) convert_to_boolean(zv)

ext/zend_test/test.c

@@ -307,6 +307,23 @@ static ZEND_FUNCTION(zend_get_unit_enum)
 	RETURN_OBJ_COPY(zend_enum_get_case_cstr(zend_test_unit_enum, "Foo"));
 }
 
+static ZEND_FUNCTION(zend_test_zend_ini_parse_quantity)
+{
+	zend_string *str;
+	zend_string *errstr = NULL;
+
+	ZEND_PARSE_PARAMETERS_START(1, 1)
+		Z_PARAM_STR(str)
+	ZEND_PARSE_PARAMETERS_END();
+
+	RETVAL_LONG(zend_ini_parse_quantity(str, &errstr));
+
+	if (errstr) {
+		zend_error(E_WARNING, "%s", errstr);
+		zend_string_release(errstr);
+	}
+}
+
 static ZEND_FUNCTION(namespaced_func)
 {
 	ZEND_PARSE_PARAMETERS_NONE();

ext/zend_test/test.stub.php

@@ -92,6 +92,8 @@ function zend_weakmap_remove(object $object): bool {}
     function zend_weakmap_dump(): array {}
 
     function zend_get_unit_enum(): ZendTestUnitEnum {}
+
+    function zend_test_zend_ini_parse_quantity(string $str): int {}
 }
 
 namespace ZendTestNS {

ext/zend_test/test_arginfo.h

@@ -1,5 +1,5 @@
 /* This is a generated file, edit the .stub.php file instead.
- * Stub hash: 7326163f8ce5340c12e74af72d47a8926eb39786 */
+ * Stub hash: 6ab822237eacd3831c1d8dea23b7d5e4c88e4a88 */
 
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_array_return, 0, 0, IS_ARRAY, 0)
 ZEND_END_ARG_INFO()
@@ -71,6 +71,10 @@ ZEND_END_ARG_INFO()
 ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_zend_get_unit_enum, 0, 0, ZendTestUnitEnum, 0)
 ZEND_END_ARG_INFO()
 
+ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_zend_test_zend_ini_parse_quantity, 0, 1, IS_LONG, 0)
+	ZEND_ARG_TYPE_INFO(0, str, IS_STRING, 0)
+ZEND_END_ARG_INFO()
+
 ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ZendTestNS2_ZendSubNS_namespaced_func, 0, 0, _IS_BOOL, 0)
 ZEND_END_ARG_INFO()
 
@@ -116,6 +120,7 @@ static ZEND_FUNCTION(zend_weakmap_attach);
 static ZEND_FUNCTION(zend_weakmap_remove);
 static ZEND_FUNCTION(zend_weakmap_dump);
 static ZEND_FUNCTION(zend_get_unit_enum);
+static ZEND_FUNCTION(zend_test_zend_ini_parse_quantity);
 static ZEND_FUNCTION(namespaced_func);
 static ZEND_METHOD(_ZendTestClass, is_object);
 static ZEND_METHOD(_ZendTestClass, __toString);
@@ -147,6 +152,7 @@ static const zend_function_entry ext_functions[] = {
 	ZEND_FE(zend_weakmap_remove, arginfo_zend_weakmap_remove)
 	ZEND_FE(zend_weakmap_dump, arginfo_zend_weakmap_dump)
 	ZEND_FE(zend_get_unit_enum, arginfo_zend_get_unit_enum)
+	ZEND_FE(zend_test_zend_ini_parse_quantity, arginfo_zend_test_zend_ini_parse_quantity)
 	ZEND_NS_FE("ZendTestNS2\\ZendSubNS", namespaced_func, arginfo_ZendTestNS2_ZendSubNS_namespaced_func)
 	ZEND_FE_END
 };