Improve error messages of ext/hash

kocsismate · kocsismate · commit c5fe1a192df4 · 2020-03-24T10:23:11.000+01:00
Closes GH-5275
diff --git a/ext/hash/hash.c b/ext/hash/hash.c
@@ -137,7 +137,7 @@ static void php_hash_do_hash(INTERNAL_FUNCTION_PARAMETERS, int isfilename, zend_
 	}
 	if (isfilename) {
 		if (CHECK_NULL_PATH(data, data_len)) {
-			zend_argument_value_error(1, "must be a valid path");
+			zend_argument_type_error(1, "must be a valid path");
 			RETURN_THROWS();
 		}
 		stream = php_stream_open_wrapper_ex(data, "rb", REPORT_ERRORS, NULL, FG(default_context));
@@ -254,18 +254,14 @@ static void php_hash_do_hash_hmac(INTERNAL_FUNCTION_PARAMETERS, int isfilename,
 	}
 
 	ops = php_hash_fetch_ops(algo);
-	if (!ops) {
-		zend_throw_error(NULL, "Unknown hashing algorithm: %s", ZSTR_VAL(algo));
-		RETURN_THROWS();
-	}
-	else if (!ops->is_crypto) {
-		zend_throw_error(NULL, "Non-cryptographic hashing algorithm: %s", ZSTR_VAL(algo));
+	if (!ops || !ops->is_crypto) {
+		zend_argument_value_error(1, "must be a valid cryptographic hashing algorithm");
 		RETURN_THROWS();
 	}
 
 	if (isfilename) {
 		if (CHECK_NULL_PATH(data, data_len)) {
-			zend_throw_error(NULL, "Invalid path");
+			zend_argument_type_error(2, "must be a valid path");
 			RETURN_THROWS();
 		}
 		stream = php_stream_open_wrapper_ex(data, "rb", REPORT_ERRORS, NULL, FG(default_context));
@@ -361,18 +357,18 @@ PHP_FUNCTION(hash_init)
 
 	ops = php_hash_fetch_ops(algo);
 	if (!ops) {
-		zend_throw_error(NULL, "Unknown hashing algorithm: %s", ZSTR_VAL(algo));
+		zend_argument_value_error(1, "must be a valid hashing algorithm");
 		RETURN_THROWS();
 	}
 
 	if (options & PHP_HASH_HMAC) {
 		if (!ops->is_crypto) {
-			zend_throw_error(NULL, "HMAC requested with a non-cryptographic hashing algorithm: %s", ZSTR_VAL(algo));
+			zend_argument_value_error(1, "must be a cryptographic hashing algorithm if HMAC is requested");
 			RETURN_THROWS();
 		}
 		if (!key || (ZSTR_LEN(key) == 0)) {
 			/* Note: a zero length key is no key at all */
-			zend_throw_error(NULL, "HMAC requested without a key");
+			zend_argument_value_error(3, "cannot be empty when HMAC is requested");
 			RETURN_THROWS();
 		}
 	}
@@ -649,28 +645,23 @@ PHP_FUNCTION(hash_hkdf)
 	}
 
 	ops = php_hash_fetch_ops(algo);
-	if (!ops) {
-		zend_throw_error(NULL, "Unknown hashing algorithm: %s", ZSTR_VAL(algo));
-		RETURN_THROWS();
-	}
-
-	if (!ops->is_crypto) {
-		zend_throw_error(NULL, "Non-cryptographic hashing algorithm: %s", ZSTR_VAL(algo));
+	if (!ops || !ops->is_crypto) {
+		zend_argument_value_error(1, "must be a valid cryptographic hashing algorithm");
 		RETURN_THROWS();
 	}
 
 	if (ZSTR_LEN(ikm) == 0) {
-		zend_throw_error(NULL, "Input keying material cannot be empty");
+		zend_argument_value_error(2, "cannot be empty");
 		RETURN_THROWS();
 	}
 
 	if (length < 0) {
-		zend_throw_error(NULL, "Length must be greater than or equal to 0: " ZEND_LONG_FMT, length);
+		zend_argument_value_error(3, "must be greater than or equal to 0");
 		RETURN_THROWS();
 	} else if (length == 0) {
 		length = ops->digest_size;
 	} else if (length > (zend_long) (ops->digest_size * 255)) {
-		zend_throw_error(NULL, "Length must be less than or equal to %zd: " ZEND_LONG_FMT, ops->digest_size * 255, length);
+		zend_argument_value_error(3, "must be less than or equal to %zd", ops->digest_size * 255);
 		RETURN_THROWS();
 	}
 
@@ -749,27 +740,23 @@ PHP_FUNCTION(hash_pbkdf2)
 	}
 
 	ops = php_hash_fetch_ops(algo);
-	if (!ops) {
-		zend_throw_error(NULL, "Unknown hashing algorithm: %s", ZSTR_VAL(algo));
+	if (!ops || !ops->is_crypto) {
+		zend_argument_value_error(1, "must be a valid cryptographic hashing algorithm");
 		RETURN_THROWS();
 	}
-	else if (!ops->is_crypto) {
-		zend_throw_error(NULL, "Non-cryptographic hashing algorithm: %s", ZSTR_VAL(algo));
+
+	if (salt_len > INT_MAX - 4) {
+		zend_argument_value_error(3, "must be less than or equal to INT_MAX - 4 bytes");
 		RETURN_THROWS();
 	}
 
 	if (iterations <= 0) {
-		zend_throw_error(NULL, "Iterations must be a positive integer: " ZEND_LONG_FMT, iterations);
+		zend_argument_value_error(4, "must be greater than 0");
 		RETURN_THROWS();
 	}
 
 	if (length < 0) {
-		zend_throw_error(NULL, "Length must be greater than or equal to 0: " ZEND_LONG_FMT, length);
-		RETURN_THROWS();
-	}
-
-	if (salt_len > INT_MAX - 4) {
-		zend_throw_error(NULL, "Supplied salt is too long, max of INT_MAX - 4 bytes: %zd supplied", salt_len);
+		zend_argument_value_error(5, "must be greater than or equal to 0");
 		RETURN_THROWS();
 	}
 
@@ -875,12 +862,12 @@ PHP_FUNCTION(hash_equals)
 
 	/* We only allow comparing string to prevent unexpected results. */
 	if (Z_TYPE_P(known_zval) != IS_STRING) {
-		zend_type_error("Expected known_string to be a string, %s given", zend_zval_type_name(known_zval));
+		zend_argument_type_error(1, "must be of type string, %s given", zend_zval_type_name(known_zval));
 		RETURN_THROWS();
 	}
 
 	if (Z_TYPE_P(user_zval) != IS_STRING) {
-		zend_type_error("Expected user_string to be a string, %s given", zend_zval_type_name(user_zval));
+		zend_argument_type_error(2, "must be of type string, %s given", zend_zval_type_name(user_zval));
 		RETURN_THROWS();
 	}
 
diff --git a/ext/hash/tests/hash_equals.phpt b/ext/hash/tests/hash_equals.phpt
@@ -39,9 +39,9 @@ bool(false)
 bool(false)
 bool(false)
 bool(true)
-[TypeError] Expected known_string to be a string, int given
-[TypeError] Expected user_string to be a string, int given
-[TypeError] Expected known_string to be a string, int given
-[TypeError] Expected known_string to be a string, null given
-[TypeError] Expected known_string to be a string, null given
-[TypeError] Expected known_string to be a string, null given
+[TypeError] hash_equals(): Argument #1 ($known_string) must be of type string, int given
+[TypeError] hash_equals(): Argument #2 ($user_string) must be of type string, int given
+[TypeError] hash_equals(): Argument #1 ($known_string) must be of type string, int given
+[TypeError] hash_equals(): Argument #1 ($known_string) must be of type string, null given
+[TypeError] hash_equals(): Argument #1 ($known_string) must be of type string, null given
+[TypeError] hash_equals(): Argument #1 ($known_string) must be of type string, null given
diff --git a/ext/hash/tests/hash_hkdf_edges.phpt b/ext/hash/tests/hash_hkdf_edges.phpt
@@ -31,4 +31,4 @@ Length < digestSize: 98b16391063ece
 Length % digestSize != 0: 98b16391063ecee006a3ca8ee5776b1e5f
 Algo name case-sensitivity: true
 Non-crypto algo name case-sensitivity:
-[Error] Non-cryptographic hashing algorithm: jOaAt
+[Error] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
diff --git a/ext/hash/tests/hash_hkdf_error.phpt b/ext/hash/tests/hash_hkdf_error.phpt
@@ -53,19 +53,19 @@ trycatch_dump(
 *** Testing hash_hkdf(): error conditions ***
 
 -- Testing hash_hkdf() function with invalid hash algorithm --
-[Error] Unknown hashing algorithm: foo
+[ValueError] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
 
 -- Testing hash_hkdf() function with non-cryptographic hash algorithm --
-[Error] Non-cryptographic hashing algorithm: adler32
-[Error] Non-cryptographic hashing algorithm: crc32
-[Error] Non-cryptographic hashing algorithm: crc32b
-[Error] Non-cryptographic hashing algorithm: fnv132
-[Error] Non-cryptographic hashing algorithm: fnv1a32
-[Error] Non-cryptographic hashing algorithm: fnv164
-[Error] Non-cryptographic hashing algorithm: fnv1a64
-[Error] Non-cryptographic hashing algorithm: joaat
+[ValueError] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
+[ValueError] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
+[ValueError] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
+[ValueError] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
+[ValueError] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
+[ValueError] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
+[ValueError] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
+[ValueError] hash_hkdf(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
 
 -- Testing hash_hkdf() function with invalid parameters --
-[Error] Input keying material cannot be empty
-[Error] Length must be greater than or equal to 0: -1
-[Error] Length must be less than or equal to 5100: 5101
+[ValueError] hash_hkdf(): Argument #2 ($ikm) cannot be empty
+[ValueError] hash_hkdf(): Argument #3 ($length) must be greater than or equal to 0
+[ValueError] hash_hkdf(): Argument #3 ($length) must be less than or equal to 5100
diff --git a/ext/hash/tests/hash_hmac_error.phpt b/ext/hash/tests/hash_hmac_error.phpt
@@ -33,7 +33,7 @@ catch (\Error $e) {
 *** Testing hash_hmac() : error conditions ***
 
 -- Testing hash_hmac() function with invalid hash algorithm --
-Unknown hashing algorithm: foo
+hash_hmac(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
 
 -- Testing hash_hmac() function with non-cryptographic hash algorithm --
-Non-cryptographic hashing algorithm: crc32
+hash_hmac(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
diff --git a/ext/hash/tests/hash_hmac_file_error.phpt b/ext/hash/tests/hash_hmac_file_error.phpt
@@ -34,7 +34,7 @@ echo "\n-- Testing hash_hmac_file() function with bad path --\n";
 try {
     var_dump(hash_hmac_file('md5', $file.chr(0).$file, $key, TRUE));
 }
-catch (\Error $e) {
+catch (TypeError $e) {
     echo $e->getMessage() . "\n";
 }
 
@@ -43,10 +43,10 @@ catch (\Error $e) {
 *** Testing hash() : error conditions ***
 
 -- Testing hash_hmac_file() function with invalid hash algorithm --
-Unknown hashing algorithm: foo
+hash_hmac_file(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
 
 -- Testing hash_hmac_file() function with non-cryptographic hash algorithm --
-Non-cryptographic hashing algorithm: crc32
+hash_hmac_file(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
 
 -- Testing hash_hmac_file() function with bad path --
-Invalid path
+hash_hmac_file(): Argument #2 ($data) must be a valid path
diff --git a/ext/hash/tests/hash_init_error.phpt b/ext/hash/tests/hash_init_error.phpt
@@ -41,11 +41,11 @@ catch (\Error $e) {
 *** Testing hash_init(): error conditions ***
 
 -- Testing hash_init() function with unknown algorithms --
-Unknown hashing algorithm: dummy
+hash_init(): Argument #1 ($algo) must be a valid hashing algorithm
 
 -- Testing hash_init() function with HASH_HMAC and non-cryptographic algorithms --
-HMAC requested with a non-cryptographic hashing algorithm: crc32
+hash_init(): Argument #1 ($algo) must be a cryptographic hashing algorithm if HMAC is requested
 
 -- Testing hash_init() function with HASH_HMAC and no key --
-HMAC requested without a key
-HMAC requested without a key
+hash_init(): Argument #3 ($key) cannot be empty when HMAC is requested
+hash_init(): Argument #3 ($key) cannot be empty when HMAC is requested
diff --git a/ext/hash/tests/hash_pbkdf2_error.phpt b/ext/hash/tests/hash_pbkdf2_error.phpt
@@ -57,14 +57,14 @@ catch (\Error $e) {
 *** Testing hash_pbkdf2() : error conditions ***
 
 -- Testing hash_pbkdf2() function with invalid hash algorithm --
-Unknown hashing algorithm: foo
+hash_pbkdf2(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
 
 -- Testing hash_pbkdf2() function with non-cryptographic hash algorithm --
-Non-cryptographic hashing algorithm: crc32
+hash_pbkdf2(): Argument #1 ($algo) must be a valid cryptographic hashing algorithm
 
 -- Testing hash_pbkdf2() function with invalid iterations --
-Iterations must be a positive integer: 0
-Iterations must be a positive integer: -1
+hash_pbkdf2(): Argument #4 ($iterations) must be greater than 0
+hash_pbkdf2(): Argument #4 ($iterations) must be greater than 0
 
 -- Testing hash_pbkdf2() function with invalid length --
-Length must be greater than or equal to 0: -1
+hash_pbkdf2(): Argument #5 ($length) must be greater than or equal to 0
