Skip to content

Commit 7e339a3

Browse files
nikicnikic
committed
Make null byte error a ValueError
Currently we treat paths with null bytes as a TypeError, which is incorrect, and rather inconsistent, as we treat empty paths as ValueError. We do this because the error is generated by zpp and it's easier to always throw TypeError there. This changes the zpp implementation to throw a TypeError only if the type is actually wrong and throw ValueError for null bytes. The error message is also split accordingly, to be more precise. Closes GH-6094.
1 parent 2386f65 commit 7e339a3

File tree

59 files changed

+213
-197
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

59 files changed

+213
-197
lines changed

‎Zend/zend_API.c

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,12 @@ ZEND_API ZEND_COLD void ZEND_FASTCALL zend_wrong_parameter_type_error(uint32_t n
254254
return;
255255
}
256256

257+
if ((expected_type == Z_EXPECTED_PATH || expected_type == Z_EXPECTED_PATH_OR_NULL)
258+
&& Z_TYPE_P(arg) == IS_STRING) {
259+
zend_argument_value_error(num, "must not contain any null bytes");
260+
return;
261+
}
262+
257263
zend_argument_type_error(num, "must be %s, %s given", expected_error[expected_type], zend_zval_type_name(arg));
258264
}
259265
/* }}} */
@@ -668,10 +674,12 @@ static const char *zend_parse_arg_impl(zval *arg, va_list *va, const char **spec
668674
char **p = va_arg(*va, char **);
669675
size_t *pl = va_arg(*va, size_t *);
670676
if (!zend_parse_arg_path(arg, p, pl, check_null)) {
671-
zend_spprintf(error, 0, "a valid path%s, %s given",
672-
check_null ? " or null" : "", zend_zval_type_name(arg)
673-
);
674-
return "";
677+
if (Z_TYPE_P(arg) == IS_STRING) {
678+
zend_spprintf(error, 0, "must not contain any null bytes");
679+
return "";
680+
} else {
681+
return check_null ? "?string" : "string";
682+
}
675683
}
676684
}
677685
break;
@@ -680,10 +688,12 @@ static const char *zend_parse_arg_impl(zval *arg, va_list *va, const char **spec
680688
{
681689
zend_string **str = va_arg(*va, zend_string **);
682690
if (!zend_parse_arg_path_str(arg, str, check_null)) {
683-
zend_spprintf(error, 0, "a valid path%s, %s given",
684-
check_null ? " or null" : "", zend_zval_type_name(arg)
685-
);
686-
return "";
691+
if (Z_TYPE_P(arg) == IS_STRING) {
692+
zend_spprintf(error, 0, "must not contain any null bytes");
693+
return "";
694+
} else {
695+
return check_null ? "?string" : "string";
696+
}
687697
}
688698
}
689699
break;
@@ -762,7 +772,7 @@ static const char *zend_parse_arg_impl(zval *arg, va_list *va, const char **spec
762772
if (!zend_parse_arg_object(arg, p, ce, check_null)) {
763773
if (ce) {
764774
if (check_null) {
765-
zend_spprintf(error, 0, "of type ?%s, %s given", ZSTR_VAL(ce->name), zend_zval_type_name(arg));
775+
zend_spprintf(error, 0, "must be of type ?%s, %s given", ZSTR_VAL(ce->name), zend_zval_type_name(arg));
766776
return "";
767777
} else {
768778
return ZSTR_VAL(ce->name);
@@ -795,14 +805,14 @@ static const char *zend_parse_arg_impl(zval *arg, va_list *va, const char **spec
795805
}
796806
if (ce_base) {
797807
if ((!*pce || !instanceof_function(*pce, ce_base))) {
798-
zend_spprintf(error, 0, "a class name derived from %s%s, %s given",
808+
zend_spprintf(error, 0, "must be a class name derived from %s%s, %s given",
799809
ZSTR_VAL(ce_base->name), check_null ? " or null" : "", Z_STRVAL_P(arg));
800810
*pce = NULL;
801811
return "";
802812
}
803813
}
804814
if (!*pce) {
805-
zend_spprintf(error, 0, "a valid class name%s, %s given",
815+
zend_spprintf(error, 0, "must be a valid class name%s, %s given",
806816
check_null ? " or null" : "", Z_STRVAL_P(arg));
807817
return "";
808818
}
@@ -833,7 +843,7 @@ static const char *zend_parse_arg_impl(zval *arg, va_list *va, const char **spec
833843
}
834844

835845
if (is_callable_error) {
836-
zend_spprintf(error, 0, "a valid callback%s, %s", check_null ? " or null" : "", is_callable_error);
846+
zend_spprintf(error, 0, "must be a valid callback%s, %s", check_null ? " or null" : "", is_callable_error);
837847
efree(is_callable_error);
838848
return "";
839849
} else {
@@ -874,7 +884,11 @@ static zend_result zend_parse_arg(uint32_t arg_num, zval *arg, va_list *va, cons
874884
}
875885
if (!(flags & ZEND_PARSE_PARAMS_QUIET) && (*expected_type || error)) {
876886
if (error) {
877-
zend_argument_type_error(arg_num, "must be %s", error);
887+
if (strcmp(error, "must not contain any null bytes") == 0) {
888+
zend_argument_value_error(arg_num, "%s", error);
889+
} else {
890+
zend_argument_type_error(arg_num, "%s", error);
891+
}
878892
efree(error);
879893
} else {
880894
zend_argument_type_error(arg_num, "must be of type %s, %s given", expected_type, zend_zval_type_name(arg));

‎Zend/zend_API.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,8 +1215,8 @@ static zend_always_inline zval *zend_try_array_init(zval *zv)
12151215
_(Z_EXPECTED_FUNC_OR_NULL, "a valid callback or null") \
12161216
_(Z_EXPECTED_RESOURCE, "of type resource") \
12171217
_(Z_EXPECTED_RESOURCE_OR_NULL, "of type resource or null") \
1218-
_(Z_EXPECTED_PATH, "a valid path") \
1219-
_(Z_EXPECTED_PATH_OR_NULL, "a valid path or null") \
1218+
_(Z_EXPECTED_PATH, "of type string") \
1219+
_(Z_EXPECTED_PATH_OR_NULL, "of type ?string") \
12201220
_(Z_EXPECTED_OBJECT, "of type object") \
12211221
_(Z_EXPECTED_OBJECT_OR_NULL, "of type ?object") \
12221222
_(Z_EXPECTED_DOUBLE, "of type float") \

‎ext/curl/interface.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ static int php_curl_option_str(php_curl *ch, zend_long option, const char *str,
102102
CURLcode error = CURLE_OK;
103103

104104
if (strlen(str) != len) {
105-
zend_type_error("%s(): cURL option cannot contain any null-bytes", get_active_function_name());
105+
zend_value_error("%s(): cURL option must not contain any null bytes", get_active_function_name());
106106
return FAILURE;
107107
}
108108

‎ext/curl/tests/bug68089.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ $ch = curl_init();
1212

1313
try {
1414
curl_setopt($ch, CURLOPT_URL, $url);
15-
} catch (TypeError $exception) {
15+
} catch (ValueError $exception) {
1616
echo $exception->getMessage() . "\n";
1717
}
1818

1919
?>
2020
Done
2121
--EXPECT--
22-
curl_setopt(): cURL option cannot contain any null-bytes
22+
curl_setopt(): cURL option must not contain any null bytes
2323
Done

‎ext/exif/exif.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4541,7 +4541,7 @@ PHP_FUNCTION(exif_read_data)
45414541
}
45424542

45434543
if (CHECK_NULL_PATH(Z_STRVAL_P(stream), Z_STRLEN_P(stream))) {
4544-
zend_argument_type_error(1, "cannot contain any null-bytes");
4544+
zend_argument_value_error(1, "must not contain any null bytes");
45454545
RETURN_THROWS();
45464546
}
45474547

@@ -4718,7 +4718,7 @@ PHP_FUNCTION(exif_thumbnail)
47184718
}
47194719

47204720
if (CHECK_NULL_PATH(Z_STRVAL_P(stream), Z_STRLEN_P(stream))) {
4721-
zend_argument_type_error(1, "cannot contain any null-bytes");
4721+
zend_argument_value_error(1, "must not contain any null bytes");
47224722
RETURN_THROWS();
47234723
}
47244724

‎ext/exif/tests/filename_empty.phpt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,19 +17,19 @@ try {
1717

1818
try {
1919
exif_read_data("foo\0bar");
20-
} catch (TypeError $e) {
20+
} catch (ValueError $e) {
2121
echo $e->getMessage(), "\n";
2222
}
2323

2424
try {
2525
exif_thumbnail("foo\0bar");
26-
} catch (TypeError $e) {
26+
} catch (ValueError $e) {
2727
echo $e->getMessage(), "\n";
2828
}
2929

3030
?>
3131
--EXPECT--
3232
exif_read_data(): Argument #1 ($filename) cannot be empty
3333
exif_thumbnail(): Argument #1 ($filename) cannot be empty
34-
exif_read_data(): Argument #1 ($filename) cannot contain any null-bytes
35-
exif_thumbnail(): Argument #1 ($filename) cannot contain any null-bytes
34+
exif_read_data(): Argument #1 ($filename) must not contain any null bytes
35+
exif_thumbnail(): Argument #1 ($filename) must not contain any null bytes

‎ext/fileinfo/tests/finfo_open_001.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ finfo_open(): Testing magic_file names
77

88
try {
99
var_dump(finfo_open(FILEINFO_MIME, "\0"));
10-
} catch (TypeError $e) {
10+
} catch (ValueError $e) {
1111
echo $e->getMessage(), "\n";
1212
}
1313

@@ -19,7 +19,7 @@ var_dump(finfo_open(FILEINFO_MIME, '/foo/bar/inexistent'));
1919

2020
?>
2121
--EXPECTF--
22-
finfo_open(): Argument #2 ($arg) must be a valid path, string given
22+
finfo_open(): Argument #2 ($arg) must not contain any null bytes
2323
resource(%d) of type (file_info)
2424
resource(%d) of type (file_info)
2525

‎ext/gd/tests/imagegd2_nullbyte_injection.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ Testing null byte injection in imagegd2
99
$image = imagecreate(1,1);// 1px image
1010
try {
1111
imagegd($image, "./foo\0bar");
12-
} catch (TypeError $e) {
12+
} catch (ValueError $e) {
1313
echo $e->getMessage(), "\n";
1414
}
1515
?>
1616
--EXPECT--
17-
imagegd(): Argument #2 ($to) must be a valid path, string given
17+
imagegd(): Argument #2 ($to) must not contain any null bytes

‎ext/gd/tests/imagegd_nullbyte_injection.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ Testing null byte injection in imagegd
99
$image = imagecreate(1,1);// 1px image
1010
try {
1111
imagegd($image, "./foo\0bar");
12-
} catch (TypeError $e) {
12+
} catch (ValueError $e) {
1313
echo $e->getMessage(), "\n";
1414
}
1515
?>
1616
--EXPECT--
17-
imagegd(): Argument #2 ($to) must be a valid path, string given
17+
imagegd(): Argument #2 ($to) must not contain any null bytes

‎ext/gd/tests/imagexbm_nullbyte_injection.phpt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ if(!extension_loaded('gd')) die('skip gd extension not available');
99
$image = imagecreate(1,1);// 1px image
1010
try {
1111
imagexbm($image, "./foo\0bar");
12-
} catch (TypeError $e) {
12+
} catch (ValueError $e) {
1313
echo $e->getMessage(), "\n";
1414
}
1515
?>
1616
--EXPECT--
17-
imagexbm(): Argument #2 ($filename) must be a valid path or null, string given
17+
imagexbm(): Argument #2 ($filename) must not contain any null bytes

0 commit comments

Comments
 (0)