From 7e4961313979d3e4ca7828b4cfd2cb5870aa6cbd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Mon, 17 Aug 2020 09:15:43 +0200 Subject: [PATCH 1/2] Promote warnings to exceptions in ext/soap and ext/xmlwriter --- ext/soap/soap.c | 110 +++++++++--------- ext/soap/soap.stub.php | 2 +- ext/soap/soap_arginfo.h | 4 +- ext/soap/tests/bugs/bug31755.phpt | 13 ++- ext/soap/tests/fault_warning.phpt | 33 ++++-- ext/xmlwriter/php_xmlwriter.c | 4 +- .../tests/xmlwriter_open_uri_error_001.phpt | 11 +- 7 files changed, 100 insertions(+), 77 deletions(-) diff --git a/ext/soap/soap.c b/ext/soap/soap.c index 527753bd1b503..41e9fb90fa28b 100644 --- a/ext/soap/soap.c +++ b/ext/soap/soap.c @@ -149,9 +149,9 @@ static void soap_error_handler(int error_num, const char *error_filename, const if ((tmp = zend_hash_str_find(Z_OBJPROP_P(ZEND_THIS),"service", sizeof("service")-1)) != NULL) { \ ss = (soapServicePtr)zend_fetch_resource_ex(tmp, "service", le_service); \ } else { \ - php_error_docref(NULL, E_WARNING, "Can not fetch service object"); \ + zend_throw_error(NULL, "Cannot fetch SoapServer object"); \ SOAP_SERVER_END_CODE(); \ - return; \ + RETURN_THROWS(); \ } \ } @@ -525,9 +525,10 @@ PHP_METHOD(SoapParam, __construct) if (zend_parse_parameters(ZEND_NUM_ARGS(), "zs", &data, &name, &name_length) == FAILURE) { RETURN_THROWS(); } + if (name_length == 0) { - php_error_docref(NULL, E_WARNING, "Invalid parameter name"); - return; + zend_argument_value_error(2, "cannot be empty"); + RETURN_THROWS(); } this_ptr = ZEND_THIS; @@ -559,12 +560,12 @@ PHP_METHOD(SoapHeader, __construct) ZEND_PARSE_PARAMETERS_END(); if (ns_len == 0) { - php_error_docref(NULL, E_WARNING, "Invalid namespace"); - return; + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); } if (name_len == 0) { - php_error_docref(NULL, E_WARNING, "Invalid header name"); - return; + zend_argument_value_error(2, "cannot be empty"); + RETURN_THROWS(); } this_ptr = ZEND_THIS; @@ -579,13 +580,15 @@ PHP_METHOD(SoapHeader, __construct) if (ZSTR_LEN(actor_str) > 2) { add_property_stringl(this_ptr, "actor", ZSTR_VAL(actor_str), ZSTR_LEN(actor_str)); } else { - php_error_docref(NULL, E_WARNING, "Invalid actor"); + zend_argument_value_error(2, "must be longer than 2 characters"); + RETURN_THROWS(); } } else if (!actor_is_null) { if ((actor_long == SOAP_ACTOR_NEXT || actor_long == SOAP_ACTOR_NONE || actor_long == SOAP_ACTOR_UNLIMATERECEIVER)) { add_property_long(this_ptr, "actor", actor_long); } else { - php_error_docref(NULL, E_WARNING, "Invalid actor"); + zend_argument_value_error(5, "must be either SOAP_ACTOR_NEXT, SOAP_ACTOR_NONE or SOAP_ACTOR_UNLIMATERECEIVER"); + RETURN_THROWS(); } } } @@ -624,9 +627,10 @@ PHP_METHOD(SoapFault, __construct) } if ((code_str || code_ht) && (fault_code == NULL || fault_code_len == 0)) { - php_error_docref(NULL, E_WARNING, "Invalid fault code"); - return; + zend_argument_value_error(1, "is not a valid fault code"); + RETURN_THROWS(); } + if (name != NULL && name_len == 0) { name = NULL; } @@ -700,8 +704,8 @@ PHP_METHOD(SoapVar, __construct) if (zend_hash_index_exists(&SOAP_GLOBAL(defEncIndex), type)) { add_property_long(this_ptr, "enc_type", type); } else { - php_error_docref(NULL, E_WARNING, "Invalid type ID"); - return; + zend_argument_value_error(2, "is not a valid encoding"); + RETURN_THROWS(); } } @@ -739,7 +743,7 @@ static HashTable* soap_create_typemap(sdlPtr sdl, HashTable *ht) /* {{{ */ zend_string *name; if (Z_TYPE_P(tmp) != IS_ARRAY) { - php_error_docref(NULL, E_WARNING, "Wrong 'typemap' option"); + zend_type_error("SoapHeader::__construct(): \"typemap\" option must be of type array, %s given", zend_zval_type_name(tmp)); return NULL; } ht2 = Z_ARRVAL_P(tmp); @@ -971,12 +975,14 @@ PHP_METHOD(SoapServer, setPersistence) value == SOAP_PERSISTENCE_REQUEST) { service->soap_class.persistence = value; } else { - php_error_docref(NULL, E_WARNING, "Tried to set persistence with bogus value (" ZEND_LONG_FMT ")", value); - return; + zend_argument_value_error( + 1, "must be either SOAP_PERSISTENCE_SESSION or SOAP_PERSISTENCE_REQUEST when the SOAP server is used in class mode" + ); + RETURN_THROWS(); } } else { - php_error_docref(NULL, E_WARNING, "Tried to set persistence when you are using you SOAP SERVER in function mode, no persistence needed"); - return; + zend_throw_error(NULL, "SoapServer::setPersistence(): Persistence cannot be set when the SOAP server is used in function mode"); + RETURN_THROWS(); } SOAP_SERVER_END_CODE(); @@ -988,12 +994,11 @@ PHP_METHOD(SoapServer, setPersistence) PHP_METHOD(SoapServer, setClass) { soapServicePtr service; - zend_string *classname; - zend_class_entry *ce; + zend_class_entry *ce = NULL; int num_args = 0; zval *argv = NULL; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "S*", &classname, &argv, &num_args) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "C*", &ce, &argv, &num_args) == FAILURE) { RETURN_THROWS(); } @@ -1001,24 +1006,17 @@ PHP_METHOD(SoapServer, setClass) FETCH_THIS_SERVICE(service); - ce = zend_lookup_class(classname); + service->type = SOAP_CLASS; + service->soap_class.ce = ce; - if (ce) { - service->type = SOAP_CLASS; - service->soap_class.ce = ce; - - service->soap_class.persistence = SOAP_PERSISTENCE_REQUEST; - service->soap_class.argc = num_args; - if (service->soap_class.argc > 0) { - int i; - service->soap_class.argv = safe_emalloc(sizeof(zval), service->soap_class.argc, 0); - for (i = 0;i < service->soap_class.argc;i++) { - ZVAL_COPY(&service->soap_class.argv[i], &argv[i]); - } + service->soap_class.persistence = SOAP_PERSISTENCE_REQUEST; + service->soap_class.argc = num_args; + if (service->soap_class.argc > 0) { + int i; + service->soap_class.argv = safe_emalloc(sizeof(zval), service->soap_class.argc, 0); + for (i = 0;i < service->soap_class.argc;i++) { + ZVAL_COPY(&service->soap_class.argv[i], &argv[i]); } - } else { - php_error_docref(NULL, E_WARNING, "Tried to set a non existent class (%s)", ZSTR_VAL(classname)); - return; } SOAP_SERVER_END_CODE(); @@ -1122,14 +1120,15 @@ PHP_METHOD(SoapServer, addFunction) zend_function *f; if (Z_TYPE_P(tmp_function) != IS_STRING) { - php_error_docref(NULL, E_WARNING, "Tried to add a function that isn't a string"); - return; + zend_argument_type_error(1, "must contain only strings"); + RETURN_THROWS(); } key = zend_string_tolower(Z_STR_P(tmp_function)); if ((f = zend_hash_find_ptr(EG(function_table), key)) == NULL) { - php_error_docref(NULL, E_WARNING, "Tried to add a non existent function '%s'", Z_STRVAL_P(tmp_function)); + zend_type_error("SoapServer::addFunction(): Function \"%s\" not found", Z_STRVAL_P(tmp_function)); + RETURN_THROWS(); return; } @@ -1146,8 +1145,8 @@ PHP_METHOD(SoapServer, addFunction) key = zend_string_tolower(Z_STR_P(function_name)); if ((f = zend_hash_find_ptr(EG(function_table), key)) == NULL) { - php_error_docref(NULL, E_WARNING, "Tried to add a non existent function '%s'", Z_STRVAL_P(function_name)); - return; + zend_argument_type_error(1, "must be a valid callback, function \"%s\" not found", Z_STRVAL_P(function_name)); + RETURN_THROWS(); } if (service->soap_functions.ft == NULL) { service->soap_functions.functions_all = FALSE; @@ -1166,8 +1165,8 @@ PHP_METHOD(SoapServer, addFunction) } service->soap_functions.functions_all = TRUE; } else { - php_error_docref(NULL, E_WARNING, "Invalid value passed"); - return; + zend_argument_value_error(1, "must be SOAP_FUNCTIONS_ALL when an integer is passed"); + RETURN_THROWS(); } } @@ -1217,7 +1216,7 @@ PHP_METHOD(SoapServer, handle) int old_features; zval tmp_soap; - if (zend_parse_parameters(ZEND_NUM_ARGS(), "|s", &arg, &arg_len) == FAILURE) { + if (zend_parse_parameters(ZEND_NUM_ARGS(), "|s!", &arg, &arg_len) == FAILURE) { RETURN_THROWS(); } @@ -1226,7 +1225,7 @@ PHP_METHOD(SoapServer, handle) FETCH_THIS_SERVICE(service); SOAP_GLOBAL(soap_version) = service->version; - if (ZEND_NUM_ARGS() > 0 && ZEND_SIZE_T_INT_OVFL(arg_len)) { + if (arg && ZEND_SIZE_T_INT_OVFL(arg_len)) { soap_server_fault("Server", "Input string is too long", NULL, NULL, NULL); return; } @@ -1281,7 +1280,7 @@ PHP_METHOD(SoapServer, handle) php_error_docref(NULL, E_ERROR,"ob_start failed"); } - if (ZEND_NUM_ARGS() == 0) { + if (!arg) { if (SG(request_info).request_body && 0 == php_stream_rewind(SG(request_info).request_body)) { zval *server_vars, *encoding; php_stream_filter *zf = NULL; @@ -1741,8 +1740,8 @@ PHP_METHOD(SoapServer, addSoapHeader) FETCH_THIS_SERVICE(service); if (!service || !service->soap_headers_ptr) { - php_error_docref(NULL, E_WARNING, "The SoapServer::addSoapHeader function may be called only during SOAP request processing"); - return; + zend_throw_error(NULL, "SoapServer::addSoapHeader() may be called only during SOAP request processing"); + RETURN_THROWS(); } p = service->soap_headers_ptr; @@ -2567,9 +2566,9 @@ void soap_client_call_impl(INTERNAL_FUNCTION_PARAMETERS, zend_bool is_soap_call) zend_hash_next_index_insert(soap_headers, headers); Z_ADDREF_P(headers); free_soap_headers = 1; - } else{ - php_error_docref(NULL, E_WARNING, "Invalid SOAP header"); - return; + } else { + zend_argument_type_error(4, "must be of type SoapHeader|array|null, %s given", zend_zval_type_name(headers)); + RETURN_THROWS(); } /* Add default headers */ @@ -2875,8 +2874,9 @@ PHP_METHOD(SoapClient, __setSoapHeaders) add_next_index_zval(&default_headers, headers); add_property_zval(this_ptr, "__default_headers", &default_headers); Z_DELREF_P(&default_headers); - } else{ - php_error_docref(NULL, E_WARNING, "Invalid SOAP header"); + } else { + zend_argument_type_error(1, "must be of type SoapHeader|array|null, %s given", zend_zval_type_name(headers)); + RETURN_THROWS(); } RETURN_TRUE; } diff --git a/ext/soap/soap.stub.php b/ext/soap/soap.stub.php index 44f2b01135221..acbbe2a6fa19a 100644 --- a/ext/soap/soap.stub.php +++ b/ext/soap/soap.stub.php @@ -57,7 +57,7 @@ public function getFunctions() {} public function addFunction($functions) {} /** @return void */ - public function handle(string $soap_request = UNKNOWN) {} + public function handle(?string $soap_request = null) {} } class SoapClient diff --git a/ext/soap/soap_arginfo.h b/ext/soap/soap_arginfo.h index 49162a5d0e232..03b559d4fcb18 100644 --- a/ext/soap/soap_arginfo.h +++ b/ext/soap/soap_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 75e1f968f03aacc08ad7858adff05040ed61e23d */ + * Stub hash: b33d57ddba20c64739d51bfba39f2557026939cd */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_use_soap_error_handler, 0, 0, _IS_BOOL, 0) ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, handler, _IS_BOOL, 0, "true") @@ -81,7 +81,7 @@ ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SoapServer_addFunction, 0, 0, 1) ZEND_END_ARG_INFO() ZEND_BEGIN_ARG_INFO_EX(arginfo_class_SoapServer_handle, 0, 0, 0) - ZEND_ARG_TYPE_INFO(0, soap_request, IS_STRING, 0) + ZEND_ARG_TYPE_INFO_WITH_DEFAULT_VALUE(0, soap_request, IS_STRING, 1, "null") ZEND_END_ARG_INFO() #define arginfo_class_SoapClient___construct arginfo_class_SoapServer___construct diff --git a/ext/soap/tests/bugs/bug31755.phpt b/ext/soap/tests/bugs/bug31755.phpt index d3c7ea8984c1b..aff600e39ae70 100644 --- a/ext/soap/tests/bugs/bug31755.phpt +++ b/ext/soap/tests/bugs/bug31755.phpt @@ -7,13 +7,18 @@ Bug #31422 (No Error-Logging on SoapServer-Side) $client=new SOAPClient(null, array('location' => 'http://localhost', 'uri' => 'myNS', 'exceptions' => false, 'trace' => true)); -$header = new SOAPHeader(null, 'foo', 'bar'); +try { + new SOAPHeader('', 'foo', 'bar'); +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} +$header = new SOAPHeader('namespace', 'foo', 'bar'); $response= $client->__soapCall('function', array(), null, $header); print $client->__getLastRequest(); ?> ---EXPECTF-- -Warning: SoapHeader::__construct(): Invalid namespace in %s on line %d +--EXPECT-- +SoapHeader::__construct(): Argument #1 ($namespace) cannot be empty - +bar diff --git a/ext/soap/tests/fault_warning.phpt b/ext/soap/tests/fault_warning.phpt index e43b1d83593f4..77b5154b9a5e9 100644 --- a/ext/soap/tests/fault_warning.phpt +++ b/ext/soap/tests/fault_warning.phpt @@ -4,7 +4,12 @@ SoapFault class: Invalid Fault code warning given? Can't be an empty string, an --FILE-- getMessage() . "\n"; +} try { new SoapFault(new stdClass(), "message"); // Can't be a non-string (except for null) @@ -16,18 +21,28 @@ $fault = new SoapFault("Sender", "message"); echo get_class($fault) . "\n"; $fault = new SoapFault(null, "message"); echo get_class($fault) . "\n"; -$fault = new SoapFault(["more"], "message"); // two elements in array required -$fault = new SoapFault(["m", "more", "superfluous"], "message"); // two required + +try { + new SoapFault(["more"], "message"); // two elements in array required +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} + +try { + new SoapFault(["m", "more", "superfluous"], "message"); // two required +} catch (ValueError $exception) { + echo $exception->getMessage() . "\n"; +} + $fault = new SoapFault(["more-ns", "Sender"], "message"); // two given echo get_class($fault); + ?> ---EXPECTF-- -Warning: SoapFault::__construct(): Invalid fault code in %s on line %d +--EXPECT-- +SoapFault::__construct(): Argument #1 ($faultcode) is not a valid fault code SoapFault::__construct(): Argument #1 ($faultcode) must be of type string|array|null, stdClass given SoapFault SoapFault - -Warning: SoapFault::__construct(): Invalid fault code in %s on line %d - -Warning: SoapFault::__construct(): Invalid fault code in %s on line %d +SoapFault::__construct(): Argument #1 ($faultcode) is not a valid fault code +SoapFault::__construct(): Argument #1 ($faultcode) is not a valid fault code SoapFault diff --git a/ext/xmlwriter/php_xmlwriter.c b/ext/xmlwriter/php_xmlwriter.c index a6740729bb739..3dfaab97ee2a6 100644 --- a/ext/xmlwriter/php_xmlwriter.c +++ b/ext/xmlwriter/php_xmlwriter.c @@ -896,8 +896,8 @@ PHP_FUNCTION(xmlwriter_open_uri) } if (source_len == 0) { - php_error_docref(NULL, E_WARNING, "Empty string as source"); - RETURN_FALSE; + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); } valid_file = _xmlwriter_get_valid_file_path(source, resolved_path, MAXPATHLEN); diff --git a/ext/xmlwriter/tests/xmlwriter_open_uri_error_001.phpt b/ext/xmlwriter/tests/xmlwriter_open_uri_error_001.phpt index 64d93a19c265f..a3af772d8934d 100644 --- a/ext/xmlwriter/tests/xmlwriter_open_uri_error_001.phpt +++ b/ext/xmlwriter/tests/xmlwriter_open_uri_error_001.phpt @@ -4,12 +4,15 @@ xmlwriter_open_uri with empty string as parameter --FILE-- getMessage() . "\n"; +} ?> --CREDITS-- Koen Kuipers koenk82@gmail.com Theo van der Zee #Test Fest Utrecht 09-05-2009 ---EXPECTF-- -Warning: xmlwriter_open_uri(): Empty string as source in %s on line %d -bool(false) +--EXPECT-- +xmlwriter_open_uri(): Argument #1 ($uri) cannot be empty From 09a6b83f92538837bc010b1876d7a275da28633e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?M=C3=A1t=C3=A9=20Kocsis?= Date: Tue, 25 Aug 2020 13:22:24 +0200 Subject: [PATCH 2/2] Fix review --- ext/soap/soap.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/ext/soap/soap.c b/ext/soap/soap.c index 41e9fb90fa28b..e503a0ecbd6bf 100644 --- a/ext/soap/soap.c +++ b/ext/soap/soap.c @@ -1145,7 +1145,7 @@ PHP_METHOD(SoapServer, addFunction) key = zend_string_tolower(Z_STR_P(function_name)); if ((f = zend_hash_find_ptr(EG(function_table), key)) == NULL) { - zend_argument_type_error(1, "must be a valid callback, function \"%s\" not found", Z_STRVAL_P(function_name)); + zend_argument_type_error(1, "must be a valid function name, function \"%s\" not found", Z_STRVAL_P(function_name)); RETURN_THROWS(); } if (service->soap_functions.ft == NULL) { @@ -1168,6 +1168,9 @@ PHP_METHOD(SoapServer, addFunction) zend_argument_value_error(1, "must be SOAP_FUNCTIONS_ALL when an integer is passed"); RETURN_THROWS(); } + } else { + zend_argument_type_error(1, "must be of type array|string|int, %s given", zend_zval_type_name(function_name)); + RETURN_THROWS(); } SOAP_SERVER_END_CODE();