GH-22681: avoid truncation on null bytes in Reflection*::__toString()

Given that there is not an unchecked version of `smart_str_append_printf()`, to
avoid compiler warnings from using `%S` to format `zend_string` pointers (which
the smart string printing machinery supports, but printf does not support) in a
few places the format specifier is declared in a variable rather than being
specified directly in the `smart_str_append_printf()` call.

Author: DanielEScherzer

ext/reflection/php_reflection.c

@@ -308,9 +308,9 @@ static zval *reflection_instantiate(zend_class_entry *pce, zval *object) /* {{{
 }
 /* }}} */
 
-static void _const_string(smart_str *str, const char *name, zval *value, const char *indent);
+static void _const_string(smart_str *str, const zend_string *name, zval *value, const char *indent);
 static void _function_string(smart_str *str, zend_function *fptr, zend_class_entry *scope, const char* indent);
-static void _property_string(smart_str *str, zend_property_info *prop, const char *prop_name, const char* indent);
+static void _property_string(smart_str *str, zend_property_info *prop, const zend_string *prop_name, const char* indent);
 static void _class_const_string(smart_str *str, const zend_string *name, zend_class_constant *c, const char* indent);
 static void _class_string(smart_str *str, zend_class_entry *ce, zval *obj, const char *indent);
 static void _extension_string(smart_str *str, const zend_module_entry *module, const char *indent);
@@ -324,7 +324,10 @@ static void _class_string(smart_str *str, zend_class_entry *ce, zval *obj, const
 
 	/* TBD: Repair indenting of doc comment (or is this to be done in the parser?) */
 	if (ce->doc_comment) {
-		smart_str_append_printf(str, "%s%s", indent, ZSTR_VAL(ce->doc_comment));
+		// %S is used for zend_string pointers by smart str printing, but normally
+		// is for wide character strings and so compilers complain if this is inline
+		const char *format = "%s%S";
+		smart_str_append_printf(str, format, indent, ce->doc_comment);
 		smart_str_appendc(str, '\n');
 	}
 
@@ -494,7 +497,7 @@ static void _class_string(smart_str *str, zend_class_entry *ce, zval *obj, const
 				if (prop_name && ZSTR_LEN(prop_name) && ZSTR_VAL(prop_name)[0]) { /* skip all private and protected properties */
 					if (!zend_hash_exists(&ce->properties_info, prop_name)) {
 						count++;
-						_property_string(&prop_str, NULL, ZSTR_VAL(prop_name), ZSTR_VAL(sub_indent));
+						_property_string(&prop_str, NULL, prop_name, ZSTR_VAL(sub_indent));
 					}
 				}
 			} ZEND_HASH_FOREACH_END();
@@ -549,7 +552,7 @@ static void _class_string(smart_str *str, zend_class_entry *ce, zval *obj, const
 /* }}} */
 
 /* {{{ _const_string */
-static void _const_string(smart_str *str, const char *name, zval *value, const char *indent)
+static void _const_string(smart_str *str, const zend_string *name, zval *value, const char *indent)
 {
 	const char *type = zend_zval_type_name(value);
 	uint32_t flags = Z_CONSTANT_FLAGS_P(value);
@@ -579,7 +582,7 @@ static void _const_string(smart_str *str, const char *name, zval *value, const c
 
 	smart_str_appends(str, type);
 	smart_str_appendc(str, ' ');
-	smart_str_appends(str, name);
+	smart_str_append(str, name);
 	smart_str_appends(str, " ] { ");
 
 	if (Z_TYPE_P(value) == IS_ARRAY) {
@@ -610,7 +613,10 @@ static void _class_const_string(smart_str *str, const zend_string *name, zend_cl
 	const char *type = type_str ? ZSTR_VAL(type_str) : zend_zval_type_name(&c->value);
 
 	if (c->doc_comment) {
-		smart_str_append_printf(str, "%s%s\n", indent, ZSTR_VAL(c->doc_comment));
+		// %S is used for zend_string pointers by smart str printing, but normally
+		// is for wide character strings and so compilers complain if this is inline
+		const char *format = "%s%S\n";
+		smart_str_append_printf(str, format, indent, c->doc_comment);
 	}
 	smart_str_append_printf(str, "%sConstant [ %s%s %s %s ] { ",
 		indent, final, visibility, type, ZSTR_VAL(name));
@@ -826,10 +832,13 @@ static void _function_string(smart_str *str, zend_function *fptr, zend_class_ent
 	 * What's "wrong" is that any whitespace before the doc comment start is
 	 * swallowed, leading to an unaligned comment.
 	 */
+	// %S is used for zend_string pointers by smart str printing, but normally
+	// is for wide character strings and so compilers complain if this is inline
+	const char *format = "%s%S\n";
 	if (fptr->type == ZEND_USER_FUNCTION && fptr->op_array.doc_comment) {
-		smart_str_append_printf(str, "%s%s\n", indent, ZSTR_VAL(fptr->op_array.doc_comment));
+		smart_str_append_printf(str, format, indent, fptr->op_array.doc_comment);
 	} else if (fptr->type == ZEND_INTERNAL_FUNCTION && fptr->internal_function.doc_comment) {
-		smart_str_append_printf(str, "%s%s\n", indent, ZSTR_VAL(fptr->internal_function.doc_comment));
+		smart_str_append_printf(str, format, indent, fptr->internal_function.doc_comment);
 	}
 
 	smart_str_appendl(str, indent, strlen(indent));
@@ -939,14 +948,19 @@ static zval *property_get_default(zend_property_info *prop_info) {
 }
 
 /* {{{ _property_string */
-static void _property_string(smart_str *str, zend_property_info *prop, const char *prop_name, const char* indent)
+static void _property_string(smart_str *str, zend_property_info *prop, const zend_string *prop_name, const char* indent)
 {
 	if (prop && prop->doc_comment) {
-		smart_str_append_printf(str, "%s%s\n", indent, ZSTR_VAL(prop->doc_comment));
+		// %S is used for zend_string pointers by smart str printing, but normally
+		// is for wide character strings and so compilers complain if this is inline
+		const char *format = "%s%S\n";
+		smart_str_append_printf(str, format, indent, prop->doc_comment);
 	}
 	smart_str_append_printf(str, "%sProperty [ ", indent);
 	if (!prop) {
-		smart_str_append_printf(str, "<dynamic> public $%s", prop_name);
+		ZEND_ASSERT(prop_name && "Properties without info must have a name provided");
+		smart_str_appends(str, "<dynamic> public $");
+		smart_str_append(str, prop_name);
 	} else {
 		if (prop->flags & ZEND_ACC_ABSTRACT) {
 			smart_str_appends(str, "abstract ");
@@ -989,11 +1003,15 @@ static void _property_string(smart_str *str, zend_property_info *prop, const cha
 			smart_str_appendc(str, ' ');
 			zend_string_release(type_str);
 		}
+		smart_str_appendc(str, '$');
 		if (!prop_name) {
 			const char *class_name;
-			zend_unmangle_property_name(prop->name, &class_name, &prop_name);
+			const char *prop_name_cstr;
+			zend_unmangle_property_name(prop->name, &class_name, &prop_name_cstr);
+			smart_str_appends(str, prop_name_cstr);
+		} else {
+			smart_str_append(str, prop_name);
 		}
-		smart_str_append_printf(str, "$%s", prop_name);
 
 		zval *default_value = property_get_default(prop);
 		if (default_value && !Z_ISUNDEF_P(default_value)) {
@@ -1031,9 +1049,21 @@ static void _extension_ini_string(const zend_ini_entry *ini_entry, smart_str *st
 		}
 
 		smart_str_appends(str, "> ]\n");
-		smart_str_append_printf(str, "    %s  Current = '%s'\n", indent, ini_entry->value ? ZSTR_VAL(ini_entry->value) : "");
+		if (ini_entry->value) {
+			// %S is used for zend_string pointers by smart str printing, but normally
+			// is for wide character strings and so compilers complain if this is inline
+			const char *format = "    %s  Current = '%S'\n";
+			smart_str_append_printf(str, format, indent, ini_entry->value);
+		} else {
+			smart_str_append_printf(str, "    %s  Current = ''\n", indent);
+		}
 		if (ini_entry->modified) {
-			smart_str_append_printf(str, "    %s  Default = '%s'\n", indent, ini_entry->orig_value ? ZSTR_VAL(ini_entry->orig_value) : "");
+			if (ini_entry->orig_value) {
+				const char *format = "    %s  Default = '%S'\n";
+				smart_str_append_printf(str, format, indent, ini_entry->orig_value);
+			} else {
+				smart_str_append_printf(str, "    %s  Default = ''\n", indent);
+			}
 		}
 		smart_str_append_printf(str, "    %s}\n", indent);
 	}
@@ -1122,7 +1152,7 @@ static void _extension_string(smart_str *str, const zend_module_entry *module, c
 
 		ZEND_HASH_MAP_FOREACH_PTR(EG(zend_constants), constant) {
 			if (ZEND_CONSTANT_MODULE_NUMBER(constant) == module->module_number) {
-				_const_string(&str_constants, ZSTR_VAL(constant->name), &constant->value, "    ");
+				_const_string(&str_constants, constant->name, &constant->value, "    ");
 				num_constants++;
 			}
 		} ZEND_HASH_FOREACH_END();
@@ -5894,7 +5924,7 @@ ZEND_METHOD(ReflectionProperty, __toString)
 		RETURN_THROWS();
 	}
 	GET_REFLECTION_OBJECT_PTR(ref);
-	_property_string(&str, ref->prop, ZSTR_VAL(ref->unmangled_name), "");
+	_property_string(&str, ref->prop, ref->unmangled_name, "");
 	RETURN_STR(smart_str_extract(&str));
 }
 /* }}} */
@@ -7955,7 +7985,7 @@ ZEND_METHOD(ReflectionConstant, __toString)
 	}
 
 	GET_REFLECTION_OBJECT_PTR(const_);
-	_const_string(&str, ZSTR_VAL(const_->name), &const_->value, "");
+	_const_string(&str, const_->name, &const_->value, "");
 	RETURN_STR(smart_str_extract(&str));
 }
 

ext/reflection/tests/gh22681/ReflectionClassConstant_doc_comment.phpt

@@ -19,14 +19,14 @@ echo new ReflectionClass(Demo::class);
 
 ?>
 --EXPECTF--
-/** F
+/** F%0oo */
 Constant [ public bool DEMO ] { 1 }
 string(11) "/** F%0oo */"
 Class [ <user> class Demo ] {
   @@ %s(%d) : eval()'d code %d-%d
 
   - Constants [1] {
-    /** F
+    /** F%0oo */
     Constant [ public bool DEMO ] { 1 }
   }
 

ext/reflection/tests/gh22681/ReflectionClass_doc_comment.phpt

@@ -15,7 +15,7 @@ var_dump( $r->getDocComment() );
 
 ?>
 --EXPECTF--
-/** F
+/** F%0oo */
 Class [ <user> class Demo ] {
   @@ %s(%d) : eval()'d code %d-%d
 

ext/reflection/tests/gh22681/ReflectionConstant_name.phpt

@@ -11,5 +11,5 @@ var_dump( $r->getName() );
 
 ?>
 --EXPECTF--
-Constant [ bool F ] { 1 }
+Constant [ bool F%0oo ] { 1 }
 string(4) "F%0oo"

ext/reflection/tests/gh22681/ReflectionEnum_case_doc_comment.phpt

@@ -20,7 +20,7 @@ Class [ <user> final class Demo implements UnitEnum ] {
   @@ %s(%d) : eval()'d code %d-%d
 
   - Constants [1] {
-    /** F
+    /** F%0oo */
     Constant [ public Demo C ] { Object }
   }
 

ext/reflection/tests/gh22681/ReflectionExtension_ini_value.phpt

@@ -16,7 +16,7 @@ var_dump( $r->getINIEntries()['arg_separator.output'] );
 ?>
 --EXPECTF--
 Entry [ arg_separator.output <ALL> ]
-      Current = 'f'
+      Current = 'f%0oo'
       Default = '&'
     }
 

ext/reflection/tests/gh22681/ReflectionFunctionAbstract_doc_comment.phpt

@@ -24,12 +24,12 @@ var_dump( $r->getDocComment() );
 
 ?>
 --EXPECTF--
-/** F
+/** F%0oo */
 Function [ <user> function demo ] {
   @@ %s(%d) : eval()'d code %d - %d
 }
 string(11) "/** F%0oo */"
-/** B
+/** B%0ar */
 Method [ <user> public method demo ] {
   @@ %s(%d) : eval()'d code %d - %d
 }

ext/reflection/tests/gh22681/ReflectionProperty_doc_comment.phpt

@@ -19,7 +19,7 @@ echo new ReflectionClass(Demo::class);
 
 ?>
 --EXPECTF--
-/** F
+/** F%0oo */
 Property [ public $prop = NULL ]
 string(11) "/** F%0oo */"
 Class [ <user> class Demo ] {
@@ -35,7 +35,7 @@ Class [ <user> class Demo ] {
   }
 
   - Properties [1] {
-    /** F
+    /** F%0oo */
     Property [ public $prop = NULL ]
   }
 

ext/reflection/tests/gh22681/ReflectionProperty_dynamic_name.phpt

@@ -11,8 +11,8 @@ var_dump( $r->getDocComment() );
 echo new ReflectionObject($obj);
 
 ?>
---EXPECT--
-Property [ <dynamic> public $F ]
+--EXPECTF--
+Property [ <dynamic> public $F%0oo ]
 bool(false)
 Object of class [ <internal:Core> class stdClass ] {
 
@@ -29,7 +29,7 @@ Object of class [ <internal:Core> class stdClass ] {
   }
 
   - Dynamic properties [1] {
-    Property [ <dynamic> public $F ]
+    Property [ <dynamic> public $F%0oo ]
   }
 
   - Methods [0] {