Correctly halt on error in sprintf parsing (fixes #1393)

bin/tests/test_string.cpp

@@ -519,12 +519,13 @@ bool test_28() {
 	char output_format[] = "\tTest:\t%ls => %ls (%s)\n";
 	String format, output;
 	Array args;
+	bool error;
 	
 	// %%
 	format = "fish %% frog";
 	args.clear();
-	output = format.sprintf(args);
-	success = (output == String("fish % frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish % frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -534,8 +535,8 @@ bool test_28() {
 	format = "fish %d frog";
 	args.clear();
 	args.push_back(5);
-	output = format.sprintf(args);
-	success = (output == String("fish 5 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 5 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -543,8 +544,8 @@ bool test_28() {
 	format = "fish %05d frog";
 	args.clear();
 	args.push_back(5);
-	output = format.sprintf(args);
-	success = (output == String("fish 00005 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 00005 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -552,8 +553,8 @@ bool test_28() {
 	format = "fish %5d frog";
 	args.clear();
 	args.push_back(5);
-	output = format.sprintf(args);
-	success = (output == String("fish     5 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish     5 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -561,8 +562,8 @@ bool test_28() {
 	format = "fish %-5d frog";
 	args.clear();
 	args.push_back(5);
-	output = format.sprintf(args);
-	success = (output == String("fish 5     frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 5     frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -570,8 +571,8 @@ bool test_28() {
 	format = "fish %+d frog";
 	args.clear();
 	args.push_back(5);
-	output = format.sprintf(args);
-	success = (output == String("fish +5 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish +5 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -579,8 +580,8 @@ bool test_28() {
 	format = "fish %d frog";
 	args.clear();
 	args.push_back(-5);
-	output = format.sprintf(args);
-	success = (output == String("fish -5 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish -5 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -588,8 +589,8 @@ bool test_28() {
 	format = "fish %x frog";
 	args.clear();
 	args.push_back(45);
-	output = format.sprintf(args);
-	success = (output == String("fish 2d frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 2d frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -597,8 +598,8 @@ bool test_28() {
 	format = "fish %X frog";
 	args.clear();
 	args.push_back(45);
-	output = format.sprintf(args);
-	success = (output == String("fish 2D frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 2D frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -606,8 +607,8 @@ bool test_28() {
 	format = "fish %o frog";
 	args.clear();
 	args.push_back(99);
-	output = format.sprintf(args);
-	success = (output == String("fish 143 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 143 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -617,8 +618,8 @@ bool test_28() {
 	format = "fish %f frog";
 	args.clear();
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == String("fish 99.990000 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 99.990000 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -626,8 +627,8 @@ bool test_28() {
 	format = "fish %11f frog";
 	args.clear();
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == String("fish   99.990000 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish   99.990000 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -635,8 +636,8 @@ bool test_28() {
 	format = "fish %-11f frog";
 	args.clear();
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == String("fish 99.990000   frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 99.990000   frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -644,8 +645,8 @@ bool test_28() {
 	format = "fish %f frog";
 	args.clear();
 	args.push_back(99);
-	output = format.sprintf(args);
-	success = (output == String("fish 99.000000 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 99.000000 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -653,8 +654,8 @@ bool test_28() {
 	format = "fish %+f frog";
 	args.clear();
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == String("fish +99.990000 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish +99.990000 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -662,8 +663,8 @@ bool test_28() {
 	format = "fish %.1f frog";
 	args.clear();
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == String("fish 100.0 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 100.0 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -671,8 +672,8 @@ bool test_28() {
 	format = "fish %.12f frog";
 	args.clear();
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == String("fish 99.990000000000 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 99.990000000000 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -680,8 +681,8 @@ bool test_28() {
 	format = "fish %.f frog";
 	args.clear();
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == String("fish 100 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish 100 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -691,8 +692,8 @@ bool test_28() {
 	format = "fish %s frog";
 	args.clear();
 	args.push_back("cheese");
-	output = format.sprintf(args);
-	success = (output == String("fish cheese frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish cheese frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -700,8 +701,8 @@ bool test_28() {
 	format = "fish %10s frog";
 	args.clear();
 	args.push_back("cheese");
-	output = format.sprintf(args);
-	success = (output == String("fish     cheese frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish     cheese frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -709,8 +710,8 @@ bool test_28() {
 	format = "fish %-10s frog";
 	args.clear();
 	args.push_back("cheese");
-	output = format.sprintf(args);
-	success = (output == String("fish cheese     frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish cheese     frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -720,8 +721,8 @@ bool test_28() {
 	format = "fish %c frog";
 	args.clear();
 	args.push_back("A");
-	output = format.sprintf(args);
-	success = (output == String("fish A frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish A frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -729,8 +730,8 @@ bool test_28() {
 	format = "fish %c frog";
 	args.clear();
 	args.push_back(65);
-	output = format.sprintf(args);
-	success = (output == String("fish A frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish A frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -741,8 +742,8 @@ bool test_28() {
 	args.clear();
 	args.push_back(10);
 	args.push_back("cheese");
-	output = format.sprintf(args);
-	success = (output == String("fish     cheese frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish     cheese frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -751,8 +752,8 @@ bool test_28() {
 	args.clear();
 	args.push_back(10);
 	args.push_back(99);
-	output = format.sprintf(args);
-	success = (output == String("fish         99 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish         99 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -762,8 +763,8 @@ bool test_28() {
 	args.push_back(10);
 	args.push_back(3);
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == String("fish     99.990 frog"));
+	output = format.sprintf(args, &error);
+	success = (output == String("fish     99.990 frog") && !error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -773,8 +774,8 @@ bool test_28() {
 	format = "fish %s %s frog";
 	args.clear();
 	args.push_back("cheese");
-	output = format.sprintf(args);
-	success = (output == "");
+	output = format.sprintf(args, &error);
+	success = (output == "not enough arguments for format string" && error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -783,8 +784,8 @@ bool test_28() {
 	args.clear();
 	args.push_back("hello");
 	args.push_back("cheese");
-	output = format.sprintf(args);
-	success = (output == "");
+	output = format.sprintf(args, &error);
+	success = (output == "not all arguments converted during string formatting" && error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -792,8 +793,8 @@ bool test_28() {
 	format = "fish %10";
 	args.clear();
 	args.push_back("cheese");
-	output = format.sprintf(args);
-	success = (output == "");
+	output = format.sprintf(args, &error);
+	success = (output == "incomplete format" && error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -801,8 +802,8 @@ bool test_28() {
 	format = "fish %&f frog";
 	args.clear();
 	args.push_back("cheese");
-	output = format.sprintf(args);
-	success = (output == "");
+	output = format.sprintf(args, &error);
+	success = (output == "unsupported format character" && error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -810,8 +811,8 @@ bool test_28() {
 	format = "fish %2.2.2f frog";
 	args.clear();
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == "");
+	output = format.sprintf(args, &error);
+	success = (output == "too many decimal points in format" && error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -820,8 +821,8 @@ bool test_28() {
 	args.clear();
 	args.push_back("cheese");
 	args.push_back(99.99);
-	output = format.sprintf(args);
-	success = (output == "");
+	output = format.sprintf(args, &error);
+	success = (output == "* wants number" && error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -829,8 +830,8 @@ bool test_28() {
 	format = "fish %c frog";
 	args.clear();
 	args.push_back("sc");
-	output = format.sprintf(args);
-	success = (output == "");
+	output = format.sprintf(args, &error);
+	success = (output == "%c requires number or single-character string" && error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 
@@ -838,8 +839,8 @@ bool test_28() {
 	format = "fish %c frog";
 	args.clear();
 	args.push_back(Array());
-	output = format.sprintf(args);
-	success = (output == "");
+	output = format.sprintf(args, &error);
+	success = (output == "%c requires number or single-character string" && error);
 	OS::get_singleton()->print(output_format, format.c_str(), output.c_str(), success ? "OK" : "FAIL");
 	if (!success) state = false;
 

core/ustring.cpp

@@ -3550,8 +3550,8 @@ String String::lpad(int min_length, const String& character) const {
 // sprintf is implemented in GDScript via:
 //   "fish %s pie" % "frog"
 //   "fish %s %d pie" % ["frog", 12]
-String String::sprintf(const Array& values) const {
-
+// In case of an error, the string returned is the error description and "error" is true.
+String String::sprintf(const Array& values, bool* error) const {
 	String formatted;
 	CharType* self = (CharType*)c_str();
 	int num_items = values.size();
@@ -3564,6 +3564,7 @@ String String::sprintf(const Array& values) const {
 	bool left_justified;
 	bool show_sign;
 
+	*error = true;
 
 	for (; *self; self++) {
 		const CharType c = *self;
@@ -3580,13 +3581,11 @@ String String::sprintf(const Array& values) const {
 				case 'x': // Hexadecimal (lowercase)
 				case 'X': { // Hexadecimal (uppercase)
 					if (value_index >= values.size()) {
-						ERR_EXPLAIN("not enough arguments for format string");
-						ERR_FAIL_V("");
+						return "not enough arguments for format string";
 					}
 
 					if (!values[value_index].is_num()) {
-						ERR_EXPLAIN("a number is required");
-						ERR_FAIL_V("");
+						return "a number is required";
 					}
 					
 					int64_t value = values[value_index];
@@ -3622,13 +3621,11 @@ String String::sprintf(const Array& values) const {
 				}
 				case 'f': { // Float
 					if (value_index >= values.size()) {
-						ERR_EXPLAIN("not enough arguments for format string");
-						ERR_FAIL_V("");
+						return "not enough arguments for format string";
 					}
 
 					if (!values[value_index].is_num()) {
-						ERR_EXPLAIN("a number is required");
-						ERR_FAIL_V("");
+						return "a number is required";
 					}
 
 					double value = values[value_index];
@@ -3657,8 +3654,7 @@ String String::sprintf(const Array& values) const {
 				}
 				case 's': { // String
 					if (value_index >= values.size()) {
-						ERR_EXPLAIN("not enough arguments for format string");
-						ERR_FAIL_V("");
+						return "not enough arguments for format string";
 					}
 
 					String str = values[value_index];
@@ -3676,8 +3672,7 @@ String String::sprintf(const Array& values) const {
 				}
 				case 'c': {
 					if (value_index >= values.size()) {
-						ERR_EXPLAIN("not enough arguments for format string");
-						ERR_FAIL_V("");
+						return "not enough arguments for format string";
 					}
 
 					// Convert to character.
@@ -3685,22 +3680,18 @@ String String::sprintf(const Array& values) const {
 					if (values[value_index].is_num()) {
 						int value = values[value_index];
 						if (value < 0) {
-							ERR_EXPLAIN("unsigned byte integer is lower than maximum")
-							ERR_FAIL_V("");
+							return "unsigned byte integer is lower than maximum";
 						} else if (value > 255) {
-							ERR_EXPLAIN("unsigned byte integer is greater than maximum")
-							ERR_FAIL_V("");
+							return "unsigned byte integer is greater than maximum";
 						}
 						str = chr(values[value_index]);
 					} else if (values[value_index].get_type() == Variant::STRING) {
 						str = values[value_index];
 						if (str.length() != 1) {
-							ERR_EXPLAIN("%c requires number or single-character string");
-							ERR_FAIL_V("");
+							return "%c requires number or single-character string";
 						}
 					} else {
-						ERR_EXPLAIN("%c requires number or single-character string");
-						ERR_FAIL_V("");
+						return "%c requires number or single-character string";
 					}
 
 					// Padding.
@@ -3741,8 +3732,7 @@ String String::sprintf(const Array& values) const {
 				}
 				case '.': { // Float separtor.
 					if (in_decimals) {
-						ERR_EXPLAIN("too many decimal points in format");
-						ERR_FAIL_V("");
+						return "too many decimal points in format";
 					}
 					in_decimals = true;
 					min_decimals = 0; // We want to add the value manually.
@@ -3751,13 +3741,11 @@ String String::sprintf(const Array& values) const {
 
 				case '*': { // Dyanmic width, based on value.
 					if (value_index >= values.size()) {
-						ERR_EXPLAIN("not enough arguments for format string");
-						ERR_FAIL_V("");
+						return "not enough arguments for format string";
 					}
 
 					if (!values[value_index].is_num()) {
-						ERR_EXPLAIN("* wants number");
-						ERR_FAIL_V("");
+						return "* wants number";
 					}
 
 					int size = values[value_index];
@@ -3773,8 +3761,7 @@ String String::sprintf(const Array& values) const {
 				}
 
 				default: {
-					ERR_EXPLAIN("unsupported format character");
-  					ERR_FAIL_V("");
+					return "unsupported format character";
   				}
 			}
 		} else { // Not in format string.
@@ -3796,14 +3783,13 @@ String String::sprintf(const Array& values) const {
 	}
 
 	if (in_format) {
-		ERR_EXPLAIN("incomplete format");
-  		ERR_FAIL_V("");
+		return "incomplete format";
 	}
 
 	if (value_index != values.size()) {
-		ERR_EXPLAIN("not all arguments converted during string formatting");
-  		ERR_FAIL_V("");
+		return "not all arguments converted during string formatting";
 	}
 
+	*error = false;
 	return formatted;
 }

core/ustring.h

@@ -130,7 +130,7 @@ public:
 	String pad_zeros(int p_digits) const;
 	String lpad(int min_length,const String& character=" ") const;
 	String rpad(int min_length,const String& character=" ") const;
-	String sprintf(const Array& values) const;
+	String sprintf(const Array& values, bool* error) const;
 	static String num(double p_num,int p_decimals=-1);
 	static String num_scientific(double p_num);
 	static String num_real(double p_num);

core/variant_op.cpp

@@ -738,18 +738,22 @@ void Variant::evaluate(const Operator& p_op, const Variant& p_a, const Variant&
 				_RETURN( p_a._data._int % p_b._data._int );
 				
 			} else if (p_a.type==STRING) {
-				const String *str=reinterpret_cast<const String*>(p_a._data._mem);
+				const String* format=reinterpret_cast<const String*>(p_a._data._mem);
 
+				String result;
+				bool error;
 				if (p_b.type==ARRAY) {
 					// e.g. "frog %s %d" % ["fish", 12]
-					const Array *arr=reinterpret_cast<const Array*>(p_b._data._mem);
-					_RETURN(str->sprintf(*arr));
+					const Array* args=reinterpret_cast<const Array*>(p_b._data._mem);
+					result=format->sprintf(*args, &error);
 				} else {
 					// e.g. "frog %d" % 12
-					Array arr;
-					arr.push_back(p_b);
-					_RETURN(str->sprintf(arr));
+					Array args;
+					args.push_back(p_b);
+					result=format->sprintf(args, &error);
 				}
+				r_valid = !error;
+				_RETURN(result);
 			}
 
 			r_valid=false;

modules/gdscript/gd_script.cpp

@@ -337,7 +337,7 @@ Variant GDFunction::call(GDInstance *p_instance, const Variant **p_args, int p_a
 
 				Variant::evaluate(op,*a,*b,*dst,valid);
 				if (!valid) {
-					if (false && dst->get_type()==Variant::STRING) {
+					if (dst->get_type()==Variant::STRING) {
 						//return a string when invalid with the error
 						err_text=*dst;
 					} else {