Skip to content

Commit ed56e07

Browse files
GirgiasGirgias
authored
ext/soap: use {Type|Value}Errors for programming errors in SoapServer constructor (#22573)
Add extra tests for option parsing of SoapServer constructor
1 parent d673a1f commit ed56e07

9 files changed

Lines changed: 283 additions & 71 deletions

‎ext/soap/soap.c‎

Lines changed: 118 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -933,110 +933,165 @@ static HashTable* soap_create_typemap(sdlPtr sdl, HashTable *ht) /* {{{ */
933933
/* {{{ SoapServer constructor */
934934
PHP_METHOD(SoapServer, __construct)
935935
{
936-
soapServicePtr service;
937-
zval *options = NULL;
936+
HashTable *options = NULL;
938937
zend_string *wsdl;
939938
int version = SOAP_1_1;
940939
zend_long cache_wsdl;
941940
HashTable *typemap_ht = NULL;
942941

943-
if (zend_parse_parameters(ZEND_NUM_ARGS(), "S!|a", &wsdl, &options) == FAILURE) {
942+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "S!|h", &wsdl, &options) == FAILURE) {
944943
RETURN_THROWS();
945944
}
946945

947-
SOAP_SERVER_BEGIN_CODE();
948-
949-
service = emalloc(sizeof(soapService));
946+
soapServicePtr service = emalloc(sizeof(soapService));
950947
memset(service, 0, sizeof(soapService));
951-
service->send_errors = 1;
948+
service->send_errors = true;
952949

953950
cache_wsdl = SOAP_GLOBAL(cache_enabled) ? SOAP_GLOBAL(cache_mode) : 0;
954951

955952
if (options != NULL) {
956-
HashTable *ht = Z_ARRVAL_P(options);
957-
zval *tmp;
958-
959-
if ((tmp = zend_hash_str_find(ht, "soap_version", sizeof("soap_version")-1)) != NULL) {
960-
if (Z_TYPE_P(tmp) == IS_LONG &&
961-
(Z_LVAL_P(tmp) == SOAP_1_1 || Z_LVAL_P(tmp) == SOAP_1_2)) {
962-
version = Z_LVAL_P(tmp);
963-
} else {
964-
php_error_docref(NULL, E_ERROR, "'soap_version' option must be SOAP_1_1 or SOAP_1_2");
953+
const zval *soap_version_zv = zend_hash_str_find(options, ZEND_STRL("soap_version"));
954+
if (soap_version_zv) {
955+
if (UNEXPECTED(Z_TYPE_P(soap_version_zv) != IS_LONG)) {
956+
zend_argument_type_error(2, "\"soap_version\" option must be of type int, %s given", zend_zval_type_name(soap_version_zv));
957+
goto cleanup;
965958
}
959+
zend_long soap_version = Z_LVAL_P(soap_version_zv);
960+
if (UNEXPECTED(soap_version != SOAP_1_1 && soap_version != SOAP_1_2)) {
961+
zend_argument_value_error(2, "\"soap_version\" option must be SOAP_1_1 or SOAP_1_2");
962+
goto cleanup;
963+
}
964+
version = soap_version;
966965
}
967966

968-
if ((tmp = zend_hash_str_find(ht, "uri", sizeof("uri")-1)) != NULL &&
969-
Z_TYPE_P(tmp) == IS_STRING) {
970-
service->uri = estrndup(Z_STRVAL_P(tmp), Z_STRLEN_P(tmp));
967+
const zval *uri_zv = zend_hash_str_find(options, ZEND_STRL("uri"));
968+
if (uri_zv) {
969+
if (UNEXPECTED(Z_TYPE_P(uri_zv) != IS_STRING)) {
970+
zend_argument_type_error(2, "\"uri\" option must be of type string, %s given", zend_zval_type_name(uri_zv));
971+
goto cleanup;
972+
}
973+
service->uri = estrndup(Z_STRVAL_P(uri_zv), Z_STRLEN_P(uri_zv));
971974
} else if (!wsdl) {
972-
php_error_docref(NULL, E_ERROR, "'uri' option is required in nonWSDL mode");
975+
zend_argument_value_error(2, "\"uri\" option is required when argument #1 ($wsdl) is null");
976+
goto cleanup;
973977
}
974978

975-
if ((tmp = zend_hash_str_find(ht, "actor", sizeof("actor")-1)) != NULL &&
976-
Z_TYPE_P(tmp) == IS_STRING) {
977-
service->actor = estrndup(Z_STRVAL_P(tmp), Z_STRLEN_P(tmp));
979+
const zval *actor_zv = zend_hash_str_find(options, ZEND_STRL("actor"));
980+
if (actor_zv) {
981+
if (UNEXPECTED(Z_TYPE_P(actor_zv) != IS_STRING)) {
982+
zend_argument_type_error(2, "\"actor\" option must be of type string, %s given", zend_zval_type_name(actor_zv));
983+
goto cleanup;
984+
}
985+
service->actor = estrndup(Z_STRVAL_P(actor_zv), Z_STRLEN_P(actor_zv));
978986
}
979987

980-
if ((tmp = zend_hash_str_find(ht, "encoding", sizeof("encoding")-1)) != NULL &&
981-
Z_TYPE_P(tmp) == IS_STRING) {
982-
xmlCharEncodingHandlerPtr encoding;
988+
const zval *encoding_zv = zend_hash_str_find(options, ZEND_STRL("encoding"));
989+
if (encoding_zv) {
990+
if (UNEXPECTED(Z_TYPE_P(encoding_zv) != IS_STRING)) {
991+
zend_argument_type_error(2, "\"encoding\" option must be of type string, %s given", zend_zval_type_name(encoding_zv));
992+
goto cleanup;
993+
}
983994

984-
encoding = xmlFindCharEncodingHandler(Z_STRVAL_P(tmp));
985-
if (encoding == NULL) {
986-
php_error_docref(NULL, E_ERROR, "Invalid 'encoding' option - '%s'", Z_STRVAL_P(tmp));
987-
} else {
988-
service->encoding = encoding;
995+
// TODO Check for null bytes?
996+
xmlCharEncodingHandlerPtr encoding = xmlFindCharEncodingHandler(Z_STRVAL_P(encoding_zv));
997+
if (UNEXPECTED(encoding == NULL)) {
998+
zend_argument_value_error(2, "\"encoding\" option must be a valid encoding, \"%s\" given", Z_STRVAL_P(encoding_zv));
999+
goto cleanup;
9891000
}
1001+
service->encoding = encoding;
9901002
}
9911003

992-
if ((tmp = zend_hash_str_find(ht, "classmap", sizeof("classmap")-1)) != NULL &&
993-
Z_TYPE_P(tmp) == IS_ARRAY) {
994-
if (HT_IS_PACKED(Z_ARRVAL_P(tmp))) {
995-
php_error_docref(NULL, E_ERROR, "'classmap' option must be an associative array");
1004+
const zval *class_map_zv = zend_hash_str_find(options, ZEND_STRL("classmap"));
1005+
if (class_map_zv) {
1006+
if (UNEXPECTED(Z_TYPE_P(class_map_zv) != IS_ARRAY)) {
1007+
zend_argument_type_error(2, "\"classmap\" option must be of type array, %s given", zend_zval_type_name(class_map_zv));
1008+
goto cleanup;
9961009
}
997-
service->class_map = zend_array_dup(Z_ARRVAL_P(tmp));
1010+
// TODO: this still accepts mixed keys arrays and not all numerically indexed arrays are packed
1011+
if (HT_IS_PACKED(Z_ARRVAL_P(class_map_zv))) {
1012+
zend_argument_value_error(2, "\"classmap\" option must be an associative array");
1013+
goto cleanup;
1014+
}
1015+
service->class_map = zend_array_dup(Z_ARR_P(class_map_zv));
9981016
}
9991017

1000-
if ((tmp = zend_hash_str_find(ht, "typemap", sizeof("typemap")-1)) != NULL &&
1001-
Z_TYPE_P(tmp) == IS_ARRAY &&
1002-
zend_hash_num_elements(Z_ARRVAL_P(tmp)) > 0) {
1003-
typemap_ht = Z_ARRVAL_P(tmp);
1018+
const zval *type_map_zv = zend_hash_str_find(options, ZEND_STRL("typemap"));
1019+
if (type_map_zv) {
1020+
if (UNEXPECTED(Z_TYPE_P(type_map_zv) != IS_ARRAY)) {
1021+
zend_argument_type_error(2, "\"typemap\" option must be of type array, %s given", zend_zval_type_name(type_map_zv));
1022+
goto cleanup;
1023+
}
1024+
if (zend_hash_num_elements(Z_ARR_P(type_map_zv)) > 0) {
1025+
typemap_ht = Z_ARRVAL_P(type_map_zv);
1026+
}
10041027
}
10051028

1006-
if ((tmp = zend_hash_str_find(ht, "features", sizeof("features")-1)) != NULL &&
1007-
Z_TYPE_P(tmp) == IS_LONG) {
1008-
service->features = Z_LVAL_P(tmp);
1029+
const zval *features_zv = zend_hash_str_find(options, ZEND_STRL("features"));
1030+
if (features_zv) {
1031+
if (UNEXPECTED(Z_TYPE_P(features_zv) != IS_LONG)) {
1032+
zend_argument_type_error(2, "\"features\" option must be of type int, %s given", zend_zval_type_name(features_zv));
1033+
goto cleanup;
1034+
}
1035+
service->features = Z_LVAL_P(features_zv);
10091036
}
10101037

1011-
if ((tmp = zend_hash_str_find(ht, "cache_wsdl", sizeof("cache_wsdl")-1)) != NULL &&
1012-
Z_TYPE_P(tmp) == IS_LONG) {
1013-
cache_wsdl = Z_LVAL_P(tmp);
1038+
const zval *cache_wsdl_zv = zend_hash_str_find(options, ZEND_STRL("cache_wsdl"));
1039+
if (cache_wsdl_zv) {
1040+
if (UNEXPECTED(Z_TYPE_P(cache_wsdl_zv) != IS_LONG)) {
1041+
zend_argument_type_error(2, "\"cache_wsdl\" option must be of type int, %s given", zend_zval_type_name(cache_wsdl_zv));
1042+
goto cleanup;
1043+
}
1044+
cache_wsdl = Z_LVAL_P(cache_wsdl_zv);
10141045
}
10151046

1016-
if ((tmp = zend_hash_str_find(ht, "send_errors", sizeof("send_errors")-1)) != NULL) {
1017-
if (Z_TYPE_P(tmp) == IS_FALSE) {
1018-
service->send_errors = 0;
1019-
} else if (Z_TYPE_P(tmp) == IS_TRUE) {
1020-
service->send_errors = 1;
1021-
} else if (Z_TYPE_P(tmp) == IS_LONG) {
1022-
service->send_errors = Z_LVAL_P(tmp);
1047+
const zval *send_errors_zv = zend_hash_str_find(options, ZEND_STRL("send_errors"));
1048+
if (send_errors_zv) {
1049+
if (UNEXPECTED(Z_TYPE_P(send_errors_zv) != IS_FALSE && Z_TYPE_P(send_errors_zv) != IS_TRUE && Z_TYPE_P(send_errors_zv) != IS_LONG)) {
1050+
zend_argument_type_error(2, "\"send_errors\" option must be of type bool, %s given", zend_zval_type_name(send_errors_zv));
1051+
goto cleanup;
1052+
}
1053+
switch (Z_TYPE_P(send_errors_zv)) {
1054+
case IS_FALSE:
1055+
service->send_errors = false;
1056+
break;
1057+
case IS_TRUE:
1058+
service->send_errors = true;
1059+
break;
1060+
case IS_LONG:
1061+
service->send_errors = Z_LVAL_P(send_errors_zv);
1062+
break;
10231063
}
10241064
}
10251065

1026-
if ((tmp = zend_hash_find(ht, ZSTR_KNOWN(ZEND_STR_TRACE))) != NULL &&
1027-
(Z_TYPE_P(tmp) == IS_TRUE ||
1028-
(Z_TYPE_P(tmp) == IS_LONG && Z_LVAL_P(tmp) == 1))) {
1029-
service->trace = true;
1066+
const zval *trace_zv = zend_hash_find(options, ZSTR_KNOWN(ZEND_STR_TRACE));
1067+
if (trace_zv) {
1068+
if (UNEXPECTED(Z_TYPE_P(trace_zv) != IS_FALSE && Z_TYPE_P(trace_zv) != IS_TRUE && Z_TYPE_P(trace_zv) != IS_LONG)) {
1069+
zend_argument_type_error(2, "\"trace\" option must be of type bool, %s given", zend_zval_type_name(trace_zv));
1070+
goto cleanup;
1071+
}
1072+
switch (Z_TYPE_P(trace_zv)) {
1073+
case IS_FALSE:
1074+
service->trace = false;
1075+
break;
1076+
case IS_TRUE:
1077+
service->trace = true;
1078+
break;
1079+
case IS_LONG:
1080+
service->trace = Z_LVAL_P(trace_zv);
1081+
break;
1082+
}
10301083
}
10311084
} else if (!wsdl) {
1032-
php_error_docref(NULL, E_ERROR, "'uri' option is required in nonWSDL mode");
1085+
zend_argument_value_error(2, "\"uri\" option is required when argument #1 ($wsdl) is null");
1086+
goto cleanup;
10331087
}
10341088

10351089
service->version = version;
10361090
service->type = SOAP_FUNCTIONS;
10371091
service->soap_functions.functions_all = FALSE;
10381092
service->soap_functions.ft = zend_new_array(0);
10391093

1094+
SOAP_SERVER_BEGIN_CODE();
10401095
if (wsdl) {
10411096
zend_try {
10421097
service->sdl = get_sdl(ZEND_THIS, ZSTR_VAL(wsdl), cache_wsdl);
@@ -1063,6 +1118,11 @@ PHP_METHOD(SoapServer, __construct)
10631118
server_obj->service = service;
10641119

10651120
SOAP_SERVER_END_CODE();
1121+
return;
1122+
1123+
cleanup:
1124+
delete_service(service);
1125+
RETURN_THROWS();
10661126
}
10671127
/* }}} */
10681128

‎ext/soap/tests/SoapServer/SoapServer_constructor_errors.phpt‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
--TEST--
2+
SoapServer constructor invalid type errors
3+
--EXTENSIONS--
4+
soap
5+
--FILE--
6+
<?php
7+
8+
$wsdl = 'irrelevant';
9+
10+
$options = [
11+
'uri' => 25,
12+
];
13+
try {
14+
$server = new SoapServer($wsdl, $options);
15+
} catch (Throwable $e) {
16+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
17+
}
18+
19+
$options = [
20+
'actor' => 25,
21+
];
22+
try {
23+
$server = new SoapServer($wsdl, $options);
24+
} catch (Throwable $e) {
25+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
26+
}
27+
28+
$options = [
29+
'classmap' => 25,
30+
];
31+
try {
32+
$server = new SoapServer($wsdl, $options);
33+
} catch (Throwable $e) {
34+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
35+
}
36+
37+
$options = [
38+
'typemap' => 25,
39+
];
40+
try {
41+
$server = new SoapServer($wsdl, $options);
42+
} catch (Throwable $e) {
43+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
44+
}
45+
46+
$options = [
47+
'features' => 'not an int',
48+
];
49+
try {
50+
$server = new SoapServer($wsdl, $options);
51+
} catch (Throwable $e) {
52+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
53+
}
54+
55+
$options = [
56+
'cache_wsdl' => 'not an int',
57+
];
58+
try {
59+
$server = new SoapServer($wsdl, $options);
60+
} catch (Throwable $e) {
61+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
62+
}
63+
64+
$options = [
65+
'send_errors' => 'not an int or bool',
66+
];
67+
try {
68+
$server = new SoapServer($wsdl, $options);
69+
} catch (Throwable $e) {
70+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
71+
}
72+
73+
$options = [
74+
'trace' => 'not an int or bool',
75+
];
76+
try {
77+
$server = new SoapServer($wsdl, $options);
78+
} catch (Throwable $e) {
79+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
80+
}
81+
82+
?>
83+
--EXPECT--
84+
TypeError: SoapServer::__construct(): Argument #2 ($options) "uri" option must be of type string, int given
85+
TypeError: SoapServer::__construct(): Argument #2 ($options) "actor" option must be of type string, int given
86+
TypeError: SoapServer::__construct(): Argument #2 ($options) "classmap" option must be of type array, int given
87+
TypeError: SoapServer::__construct(): Argument #2 ($options) "typemap" option must be of type array, int given
88+
TypeError: SoapServer::__construct(): Argument #2 ($options) "features" option must be of type int, string given
89+
TypeError: SoapServer::__construct(): Argument #2 ($options) "cache_wsdl" option must be of type int, string given
90+
TypeError: SoapServer::__construct(): Argument #2 ($options) "send_errors" option must be of type bool, string given
91+
TypeError: SoapServer::__construct(): Argument #2 ($options) "trace" option must be of type bool, string given

ext/soap/tests/bugs/gh19784.phpt renamed to ext/soap/tests/SoapServer/gh19784.phpt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ GH-19784 (SoapServer memory leak)
44
soap
55
--FILE--
66
<?php
7-
$v_5256 = 'zpHOks6TzpTOlc6WzpfOmM6ZzprOm86czp3Ons6fzqDOoc6jzqTOpc6mzqfOqM6p';
8-
$v_5257 = base64_decode($v_5256,);
9-
$v_5238 = array('encoding' => $v_5257);
10-
new SoapServer('foobarbaz',$v_5238,);
7+
8+
$options = ['encoding' => 'UTF-8'];
9+
$server = new SoapServer('foobarbaz', $options);
10+
1111
?>
1212
--EXPECTF--
1313
<?xml version="1.0" encoding="UTF-8"?>

‎ext/soap/tests/SoapServer/invalid-encoding-option-type.phpt‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
--TEST--
2+
SoapServer constructor with an invalid encoding option
3+
--EXTENSIONS--
4+
soap
5+
--FILE--
6+
<?php
7+
8+
class ExtendedSoapServer extends SoapServer {}
9+
10+
$wsdl = 'irrelevant';
11+
$options = [
12+
'encoding' => 25,
13+
];
14+
try {
15+
$client = new SoapServer($wsdl, $options);
16+
} catch (Throwable $e) {
17+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
18+
}
19+
try {
20+
$client = new ExtendedSoapServer($wsdl, $options);
21+
} catch (Throwable $e) {
22+
echo $e::class, ': ', $e->getMessage(), PHP_EOL;
23+
}
24+
25+
26+
?>
27+
--EXPECT--
28+
TypeError: SoapServer::__construct(): Argument #2 ($options) "encoding" option must be of type string, int given
29+
TypeError: SoapServer::__construct(): Argument #2 ($options) "encoding" option must be of type string, int given

‎ext/soap/tests/SoapServer/invalid-encoding-option.phpt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,5 @@ try {
2525

2626
?>
2727
--EXPECT--
28-
<?xml version="1.0" encoding="UTF-8"?>
29-
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/"><SOAP-ENV:Body><SOAP-ENV:Fault><faultcode>SOAP-ENV:Server</faultcode><faultstring>SoapServer::__construct(): Invalid 'encoding' option - 'non-sense'</faultstring></SOAP-ENV:Fault></SOAP-ENV:Body></SOAP-ENV:Envelope>
28+
ValueError: SoapServer::__construct(): Argument #2 ($options) "encoding" option must be a valid encoding, "non-sense" given
29+
ValueError: SoapServer::__construct(): Argument #2 ($options) "encoding" option must be a valid encoding, "non-sense" given

0 commit comments

Comments
 (0)