From 7bfeef602a908aed83a603cfdf3a7f93d24c15cc Mon Sep 17 00:00:00 2001 From: =?utf8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Mon, 16 Dec 2019 19:29:51 +0100 Subject: [PATCH] Promote warnings to exceptions in stream-related functions GH-5017 --- ext/libxml/tests/004.phpt | 8 -- ext/standard/streamsfuncs.c | 83 ++++++++++--------- ext/standard/tests/file/userstreams_002.phpt | 22 ++--- .../filters/stream_filter_remove_error.phpt | 24 +++--- ext/standard/tests/streams/bug44712.phpt | 10 ++- ext/standard/tests/streams/bug71884.phpt | 10 ++- .../tests/streams/stream_set_chunk_size.phpt | 22 +++-- ext/standard/user_filters.c | 19 +---- 8 files changed, 95 insertions(+), 103 deletions(-) diff --git a/ext/libxml/tests/004.phpt b/ext/libxml/tests/004.phpt index d02fba2f6c..da96faad57 100644 --- a/ext/libxml/tests/004.phpt +++ b/ext/libxml/tests/004.phpt @@ -12,11 +12,8 @@ $ctxs = array( new stdclass, array('a'), stream_context_create(), - stream_context_create(array('file')), - stream_context_create(array('file' => array('some_opt' => 'aaa'))) ); - foreach ($ctxs as $ctx) { try { var_dump(libxml_set_streams_context($ctx)); @@ -31,7 +28,6 @@ echo "Done\n"; ?> --EXPECTF-- -Warning: stream_context_create(): options should have the form ["wrappername"]["optionname"] = $value in %s004.php on line %d libxml_set_streams_context() expects parameter 1 to be resource, null given bool(true) libxml_set_streams_context() expects parameter 1 to be resource, string given @@ -44,8 +40,4 @@ libxml_set_streams_context() expects parameter 1 to be resource, array given bool(true) NULL bool(true) -NULL -bool(true) -NULL -bool(true) Done diff --git a/ext/standard/streamsfuncs.c b/ext/standard/streamsfuncs.c index 89d5abd251..c38010e030 100644 --- a/ext/standard/streamsfuncs.c +++ b/ext/standard/streamsfuncs.c @@ -383,8 +383,8 @@ PHP_FUNCTION(stream_socket_recvfrom) } if (to_read <= 0) { - php_error_docref(NULL, E_WARNING, "Length parameter must be greater than 0"); - RETURN_FALSE; + zend_value_error("Length parameter must be greater than 0"); + return; } read_buf = zend_string_alloc(to_read, 0); @@ -493,7 +493,7 @@ PHP_FUNCTION(stream_copy_to_stream) } /* }}} */ -/* {{{ proto array|false stream_get_meta_data(resource fp) +/* {{{ proto array stream_get_meta_data(resource fp) Retrieves header/meta data from streams/file pointers */ PHP_FUNCTION(stream_get_meta_data) { @@ -784,8 +784,8 @@ PHP_FUNCTION(stream_select) } if (!sets) { - php_error_docref(NULL, E_WARNING, "No stream arrays were passed"); - RETURN_FALSE; + zend_value_error("No stream arrays were passed"); + return; } PHP_SAFE_MAX_FD(max_fd, max_set_count); @@ -793,11 +793,11 @@ PHP_FUNCTION(stream_select) /* If seconds is not set to null, build the timeval, else we wait indefinitely */ if (!secnull) { if (sec < 0) { - php_error_docref(NULL, E_WARNING, "The seconds parameter must be greater than 0"); - RETURN_FALSE; + zend_value_error("The seconds parameter must be greater than 0"); + return; } else if (usec < 0) { - php_error_docref(NULL, E_WARNING, "The microseconds parameter must be greater than 0"); - RETURN_FALSE; + zend_value_error("The microseconds parameter must be greater than 0"); + return; } /* Windows, Solaris and BSD do not like microsecond values which are >= 1 sec */ @@ -827,7 +827,7 @@ PHP_FUNCTION(stream_select) retval = php_select(max_fd+1, &rfds, &wfds, &efds, tv_p); if (retval == -1) { - php_error_docref(NULL, E_WARNING, "unable to select [%d]: %s (max_fd=%d)", + php_error_docref(NULL, E_WARNING, "Unable to select [%d]: %s (max_fd=%d)", errno, strerror(errno), max_fd); RETURN_FALSE; } @@ -892,7 +892,8 @@ static int parse_context_options(php_stream_context *context, zval *options) } } ZEND_HASH_FOREACH_END(); } else { - php_error_docref(NULL, E_WARNING, "options should have the form [\"wrappername\"][\"optionname\"] = $value"); + zend_value_error("Options should have the form [\"wrappername\"][\"optionname\"] = $value"); + return FAILURE; } } ZEND_HASH_FOREACH_END(); @@ -918,9 +919,10 @@ static int parse_context_params(php_stream_context *context, zval *params) } if (NULL != (tmp = zend_hash_str_find(Z_ARRVAL_P(params), "options", sizeof("options")-1))) { if (Z_TYPE_P(tmp) == IS_ARRAY) { - parse_context_options(context, tmp); + return parse_context_options(context, tmp); } else { - php_error_docref(NULL, E_WARNING, "Invalid stream/context parameter"); + zend_type_error("Invalid stream/context parameter"); + return FAILURE; } } @@ -957,7 +959,7 @@ static php_stream_context *decode_context_param(zval *contextresource) } /* }}} */ -/* {{{ proto array|false stream_context_get_options(resource context|resource stream) +/* {{{ proto array stream_context_get_options(resource context|resource stream) Retrieve options for a stream/wrapper/context */ PHP_FUNCTION(stream_context_get_options) { @@ -970,8 +972,8 @@ PHP_FUNCTION(stream_context_get_options) context = decode_context_param(zcontext); if (!context) { - php_error_docref(NULL, E_WARNING, "Invalid stream/context parameter"); - RETURN_FALSE; + zend_type_error("Invalid stream/context parameter"); + return; } ZVAL_COPY(return_value, &context->options); @@ -995,8 +997,8 @@ PHP_FUNCTION(stream_context_set_option) /* figure out where the context is coming from exactly */ if (!(context = decode_context_param(zcontext))) { - php_error_docref(NULL, E_WARNING, "Invalid stream/context parameter"); - RETURN_FALSE; + zend_type_error("Invalid stream/context parameter"); + return; } RETURN_BOOL(parse_context_options(context, options) == SUCCESS); @@ -1014,8 +1016,8 @@ PHP_FUNCTION(stream_context_set_option) /* figure out where the context is coming from exactly */ if (!(context = decode_context_param(zcontext))) { - php_error_docref(NULL, E_WARNING, "Invalid stream/context parameter"); - RETURN_FALSE; + zend_type_error("Invalid stream/context parameter"); + return; } RETURN_BOOL(php_stream_context_set_option(context, wrappername, optionname, zvalue) == SUCCESS); @@ -1037,15 +1039,15 @@ PHP_FUNCTION(stream_context_set_params) context = decode_context_param(zcontext); if (!context) { - php_error_docref(NULL, E_WARNING, "Invalid stream/context parameter"); - RETURN_FALSE; + zend_type_error("Invalid stream/context parameter"); + return; } RETVAL_BOOL(parse_context_params(context, params) == SUCCESS); } /* }}} */ -/* {{{ proto array|false stream_context_get_params(resource context|resource stream) +/* {{{ proto array stream_context_get_params(resource context|resource stream) Get parameters of a file context */ PHP_FUNCTION(stream_context_get_params) { @@ -1058,8 +1060,8 @@ PHP_FUNCTION(stream_context_get_params) context = decode_context_param(zcontext); if (!context) { - php_error_docref(NULL, E_WARNING, "Invalid stream/context parameter"); - RETURN_FALSE; + zend_type_error("Invalid stream/context parameter"); + return; } array_init(return_value); @@ -1090,7 +1092,9 @@ PHP_FUNCTION(stream_context_get_default) context = FG(default_context); if (params) { - parse_context_options(context, params); + if (parse_context_options(context, params) == FAILURE) { + return; + } } php_stream_context_to_zval(context, return_value); @@ -1113,7 +1117,9 @@ PHP_FUNCTION(stream_context_set_default) } context = FG(default_context); - parse_context_options(context, options); + if (parse_context_options(context, options) == FAILURE) { + return; + } php_stream_context_to_zval(context, return_value); } @@ -1253,10 +1259,9 @@ PHP_FUNCTION(stream_filter_remove) Z_PARAM_RESOURCE(zfilter) ZEND_PARSE_PARAMETERS_END(); - filter = zend_fetch_resource(Z_RES_P(zfilter), NULL, php_file_le_stream_filter()); + filter = zend_fetch_resource(Z_RES_P(zfilter), "stream filter", php_file_le_stream_filter()); if (!filter) { - php_error_docref(NULL, E_WARNING, "Invalid resource given, not a stream filter"); - RETURN_FALSE; + return; } if (php_stream_filter_flush(filter, 1) == FAILURE) { @@ -1293,8 +1298,8 @@ PHP_FUNCTION(stream_get_line) ZEND_PARSE_PARAMETERS_END(); if (max_length < 0) { - php_error_docref(NULL, E_WARNING, "The maximum allowed length must be greater than or equal to zero"); - RETURN_FALSE; + zend_value_error("The maximum allowed length must be greater than or equal to zero"); + return; } if (!max_length) { max_length = PHP_SOCK_CHUNK_SIZE; @@ -1429,16 +1434,16 @@ PHP_FUNCTION(stream_set_chunk_size) ZEND_PARSE_PARAMETERS_END(); if (csize <= 0) { - php_error_docref(NULL, E_WARNING, "The chunk size must be a positive integer, given " ZEND_LONG_FMT, csize); - RETURN_FALSE; + zend_value_error("The chunk size must be a positive integer, " ZEND_LONG_FMT " given", csize); + return; } /* stream.chunk_size is actually a size_t, but php_stream_set_option * can only use an int to accept the new value and return the old one. * In any case, values larger than INT_MAX for a chunk size make no sense. */ if (csize > INT_MAX) { - php_error_docref(NULL, E_WARNING, "The chunk size cannot be larger than %d", INT_MAX); - RETURN_FALSE; + zend_value_error("The chunk size cannot be larger than %d", INT_MAX); + return; } php_stream_from_zval(stream, zstream); @@ -1504,8 +1509,8 @@ PHP_FUNCTION(stream_socket_enable_crypto) zval *val; if (!GET_CTX_OPT(stream, "ssl", "crypto_method", val)) { - php_error_docref(NULL, E_WARNING, "When enabling encryption you must specify the crypto type"); - RETURN_FALSE; + zend_value_error("When enabling encryption you must specify the crypto type"); + return; } cryptokind = Z_LVAL_P(val); @@ -1679,7 +1684,7 @@ PHP_FUNCTION(sapi_windows_vt100_support) "%s() was not able to analyze the specified stream", get_active_function_name() ); - RETURN_FALSE; + return; } /* Check if the file descriptor is a console */ diff --git a/ext/standard/tests/file/userstreams_002.phpt b/ext/standard/tests/file/userstreams_002.phpt index 3068c93b46..41aad59b5d 100644 --- a/ext/standard/tests/file/userstreams_002.phpt +++ b/ext/standard/tests/file/userstreams_002.phpt @@ -24,7 +24,7 @@ function test($name, $fd, $return_value) { $w = $e = null; try { var_dump(stream_select($r, $w, $e, 0) !== false); - } catch (TypeError $e) { + } catch (TypeError|ValueError $e) { echo $e->getMessage(), "\n"; } } @@ -55,34 +55,26 @@ bool(true) Warning: stream_select(): test_wrapper_base::stream_cast is not implemented! in %s Warning: stream_select(): cannot represent a stream of type user-space as a select()able descriptor in %s - -Warning: stream_select(): No stream arrays were passed in %s -bool(false) +No stream arrays were passed ------ return value is false: ------- Warning: stream_select(): cannot represent a stream of type user-space as a select()able descriptor in %s - -Warning: stream_select(): No stream arrays were passed in %s -bool(false) +No stream arrays were passed ------ return value not a stream resource: ------- Warning: stream_select(): test_wrapper::stream_cast must return a stream resource in %s Warning: stream_select(): cannot represent a stream of type user-space as a select()able descriptor in %s - -Warning: stream_select(): No stream arrays were passed in %s -stream_select(): supplied argument is not a valid stream resource +No stream arrays were passed ------ return value is stream itself: ------- Warning: stream_select(): test_wrapper::stream_cast must not return itself in %s Warning: stream_select(): cannot represent a stream of type user-space as a select()able descriptor in %s - -Warning: stream_select(): No stream arrays were passed in %s -bool(false) +No stream arrays were passed ------ return value cannot be casted: ------- @@ -91,6 +83,4 @@ Warning: stream_select(): test_wrapper_base::stream_cast is not implemented! in Warning: stream_select(): cannot represent a stream of type user-space as a select()able descriptor in %s Warning: stream_select(): cannot represent a stream of type user-space as a select()able descriptor in %s - -Warning: stream_select(): No stream arrays were passed in %s -bool(false) +No stream arrays were passed diff --git a/ext/standard/tests/filters/stream_filter_remove_error.phpt b/ext/standard/tests/filters/stream_filter_remove_error.phpt index f9d26837c8..ff93358b31 100644 --- a/ext/standard/tests/filters/stream_filter_remove_error.phpt +++ b/ext/standard/tests/filters/stream_filter_remove_error.phpt @@ -21,12 +21,20 @@ $filter = stream_filter_append( $fp, "string.rot13", STREAM_FILTER_WRITE ); echo "*** Testing stream_filter_remove() : error conditions ***\n"; echo "\n-- Testing stream_filter_remove() function with bad resource --\n"; -var_dump( stream_filter_remove( $fp ) ); +try { + stream_filter_remove($fp); +} catch (TypeError $exception) { + echo $exception->getMessage() . "\n"; +} echo "\n-- Testing stream_filter_remove() function with an already removed filter --\n"; // Double remove it -var_dump( stream_filter_remove( $filter ) ); -var_dump( stream_filter_remove( $filter ) ); +var_dump(stream_filter_remove( $filter )); +try { + stream_filter_remove($filter); +} catch (TypeError $exception) { + echo $exception->getMessage() . "\n"; +} fclose( $fp ); @@ -38,16 +46,12 @@ $file = __DIR__ . DIRECTORY_SEPARATOR . 'streamfilterTest.txt'; unlink( $file ); ?> ---EXPECTF-- +--EXPECT-- *** Testing stream_filter_remove() : error conditions *** -- Testing stream_filter_remove() function with bad resource -- - -Warning: stream_filter_remove(): Invalid resource given, not a stream filter in %s on line %d -bool(false) +stream_filter_remove(): supplied resource is not a valid stream filter resource -- Testing stream_filter_remove() function with an already removed filter -- bool(true) - -Warning: stream_filter_remove(): Invalid resource given, not a stream filter in %s on line %d -bool(false) +stream_filter_remove(): supplied resource is not a valid stream filter resource diff --git a/ext/standard/tests/streams/bug44712.phpt b/ext/standard/tests/streams/bug44712.phpt index a5a718351f..04b7e5ce40 100644 --- a/ext/standard/tests/streams/bug44712.phpt +++ b/ext/standard/tests/streams/bug44712.phpt @@ -3,7 +3,11 @@ bug#44712 (stream_context_set_params segfaults on invalid arguments) --FILE-- 1)); +try { + stream_context_set_params($ctx, array("options" => 1)); +} catch (TypeError $exception) { + echo $exception->getMessage() . "\n"; +} ?> ---EXPECTF-- -Warning: stream_context_set_params(): Invalid stream/context parameter in %sbug44712.php on line %d +--EXPECT-- +Invalid stream/context parameter diff --git a/ext/standard/tests/streams/bug71884.phpt b/ext/standard/tests/streams/bug71884.phpt index 798c6b6431..3c5f841b3d 100644 --- a/ext/standard/tests/streams/bug71884.phpt +++ b/ext/standard/tests/streams/bug71884.phpt @@ -4,7 +4,11 @@ Bug #71884 (Null pointer deref (segfault) in stream_context_get_default) getMessage() . "\n"; +} ?> ---EXPECTF-- -Warning: stream_context_get_default(): options should have the form ["wrappername"]["optionname"] = $value in %sbug71884.php on line %d +--EXPECT-- +Options should have the form ["wrappername"]["optionname"] = $value diff --git a/ext/standard/tests/streams/stream_set_chunk_size.phpt b/ext/standard/tests/streams/stream_set_chunk_size.phpt index 9705580902..6b90f86cd0 100644 --- a/ext/standard/tests/streams/stream_set_chunk_size.phpt +++ b/ext/standard/tests/streams/stream_set_chunk_size.phpt @@ -49,9 +49,17 @@ echo "should have no effect on writes\n"; var_dump(strlen(fwrite($f, str_repeat('b', 250)))); echo "\nerror conditions\n"; -var_dump(stream_set_chunk_size($f, 0)); -var_dump(stream_set_chunk_size($f, -1)); ---EXPECTF-- +try { + stream_set_chunk_size($f, 0); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} +try { + stream_set_chunk_size($f, -1); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} +--EXPECT-- bool(true) should return previous chunk size (8192) int(8192) @@ -76,9 +84,5 @@ write with size: 250 int(3) error conditions - -Warning: stream_set_chunk_size(): The chunk size must be a positive integer, given 0 in %s on line %d -bool(false) - -Warning: stream_set_chunk_size(): The chunk size must be a positive integer, given -1 in %s on line %d -bool(false) +The chunk size must be a positive integer, 0 given +The chunk size must be a positive integer, -1 given diff --git a/ext/standard/user_filters.c b/ext/standard/user_filters.c index 18843e85d5..383377902f 100644 --- a/ext/standard/user_filters.c +++ b/ext/standard/user_filters.c @@ -20,6 +20,7 @@ #include "php_globals.h" #include "ext/standard/basic_functions.h" #include "ext/standard/file.h" +#include "ext/standard/basic_functions_arginfo.h" #define PHP_STREAM_BRIGADE_RES_NAME "userfilter.bucket brigade" #define PHP_STREAM_BUCKET_RES_NAME "userfilter.bucket" @@ -41,23 +42,11 @@ static int le_bucket; PHP_FUNCTION(user_filter_nop) { } -ZEND_BEGIN_ARG_INFO(arginfo_php_user_filter_filter, 0) - ZEND_ARG_INFO(0, in) - ZEND_ARG_INFO(0, out) - ZEND_ARG_INFO(1, consumed) - ZEND_ARG_INFO(0, closing) -ZEND_END_ARG_INFO() - -ZEND_BEGIN_ARG_INFO(arginfo_php_user_filter_onCreate, 0) -ZEND_END_ARG_INFO() - -ZEND_BEGIN_ARG_INFO(arginfo_php_user_filter_onClose, 0) -ZEND_END_ARG_INFO() static const zend_function_entry user_filter_class_funcs[] = { - PHP_NAMED_FE(filter, PHP_FN(user_filter_nop), arginfo_php_user_filter_filter) - PHP_NAMED_FE(onCreate, PHP_FN(user_filter_nop), arginfo_php_user_filter_onCreate) - PHP_NAMED_FE(onClose, PHP_FN(user_filter_nop), arginfo_php_user_filter_onClose) + PHP_NAMED_FE(filter, PHP_FN(user_filter_nop), arginfo_class_php_user_filter_filter) + PHP_NAMED_FE(onCreate, PHP_FN(user_filter_nop), arginfo_class_php_user_filter_onCreate) + PHP_NAMED_FE(onClose, PHP_FN(user_filter_nop), arginfo_class_php_user_filter_onClose) PHP_FE_END }; -- 2.50.1