Skip to content

Commit 69328ba

Browse files
nielsdosnielsdos
committed
Fix GH-18990, bug #81029, bug #47314: SOAP HTTP socket not closing on object destruction
Currently the resource is attached to the object and its refcount is increased. This means that the refcount to the resource is 2 instead of 1 as expected. A refcount of 2 is necessary in the current code because of how the error handling works: by using convert_to_null() the resource actually goes to rc_dtor_func(), dropping its refcount to 1. So on error the refcount is correct. To solve the issue, let `stream` conceptually be a borrow of the resource with refcount 1, and just use ZVAL_NULL() to prevent calling rc_dtor_func() on the resource. Closes GH-19001.
1 parent 09c223d commit 69328ba

File tree

3 files changed

+71
-12
lines changed

3 files changed

+71
-12
lines changed

‎NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ PHP NEWS
3232
. Fixed bug GH-18958 (Fatal error during shutdown after pcntl_rfork() or
3333
pcntl_forkx() with zend-max-execution-timers). (Arnaud)
3434

35+
- SOAP:
36+
. Fixed bug GH-18990, bug #81029, bug #47314 (SOAP HTTP socket not closing
37+
on object destruction). (nielsdos)
38+
3539
- Standard:
3640
. Fix misleading errors in printf(). (nielsdos)
3741
. Fix RCN violations in array functions. (nielsdos)

‎ext/soap/php_http.c

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -511,9 +511,9 @@ int make_http_soap_request(zval *this_ptr,
511511
zend_string_equals(orig->host, phpurl->host) &&
512512
orig->port == phpurl->port))) {
513513
} else {
514+
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
514515
php_stream_close(stream);
515516
convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr));
516-
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
517517
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
518518
stream = NULL;
519519
use_proxy = 0;
@@ -522,9 +522,9 @@ int make_http_soap_request(zval *this_ptr,
522522

523523
/* Check if keep-alive connection is still opened */
524524
if (stream != NULL && php_stream_eof(stream)) {
525+
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
525526
php_stream_close(stream);
526527
convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr));
527-
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
528528
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
529529
stream = NULL;
530530
use_proxy = 0;
@@ -533,9 +533,7 @@ int make_http_soap_request(zval *this_ptr,
533533
if (!stream) {
534534
stream = http_connect(this_ptr, phpurl, use_ssl, context, &use_proxy);
535535
if (stream) {
536-
php_stream_auto_cleanup(stream);
537-
ZVAL_RES(Z_CLIENT_HTTPSOCKET_P(this_ptr), stream->res);
538-
GC_ADDREF(stream->res);
536+
php_stream_to_zval(stream, Z_CLIENT_HTTPSOCKET_P(this_ptr));
539537
ZVAL_LONG(Z_CLIENT_USE_PROXY_P(this_ptr), use_proxy);
540538
} else {
541539
php_url_free(phpurl);
@@ -555,7 +553,6 @@ int make_http_soap_request(zval *this_ptr,
555553
zval *cookies, *login, *password;
556554
zend_resource *ret = zend_register_resource(phpurl, le_url);
557555
ZVAL_RES(Z_CLIENT_HTTPURL_P(this_ptr), ret);
558-
GC_ADDREF(ret);
559556

560557
if (context &&
561558
(tmp = php_stream_context_get_option(context, "http", "protocol_version")) != NULL &&
@@ -683,9 +680,9 @@ int make_http_soap_request(zval *this_ptr,
683680

684681
if (UNEXPECTED(php_random_bytes_throw(&nonce, sizeof(nonce)) != SUCCESS)) {
685682
ZEND_ASSERT(EG(exception));
683+
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
686684
php_stream_close(stream);
687685
convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr));
688-
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
689686
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
690687
smart_str_free(&soap_headers_z);
691688
smart_str_free(&soap_headers);
@@ -901,9 +898,9 @@ int make_http_soap_request(zval *this_ptr,
901898
if (request != buf) {
902899
zend_string_release_ex(request, 0);
903900
}
901+
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
904902
php_stream_close(stream);
905903
convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr));
906-
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
907904
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
908905
add_soap_fault(this_ptr, "HTTP", "Failed Sending HTTP SOAP request", NULL, NULL);
909906
smart_str_free(&soap_headers_z);
@@ -919,8 +916,8 @@ int make_http_soap_request(zval *this_ptr,
919916
}
920917

921918
if (!return_value) {
919+
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
922920
php_stream_close(stream);
923-
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
924921
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
925922
smart_str_free(&soap_headers_z);
926923
efree(http_msg);
@@ -933,8 +930,8 @@ int make_http_soap_request(zval *this_ptr,
933930
if (request != buf) {
934931
zend_string_release_ex(request, 0);
935932
}
933+
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
936934
php_stream_close(stream);
937-
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
938935
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
939936
add_soap_fault(this_ptr, "HTTP", "Error Fetching http headers", NULL, NULL);
940937
smart_str_free(&soap_headers_z);
@@ -1102,9 +1099,9 @@ int make_http_soap_request(zval *this_ptr,
11021099
if (request != buf) {
11031100
zend_string_release_ex(request, 0);
11041101
}
1102+
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
11051103
php_stream_close(stream);
11061104
zend_string_release_ex(http_headers, 0);
1107-
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
11081105
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
11091106
add_soap_fault(this_ptr, "HTTP", "Error Fetching http body, No Content-Length, connection closed or chunked data", NULL, NULL);
11101107
if (http_msg) {
@@ -1119,8 +1116,8 @@ int make_http_soap_request(zval *this_ptr,
11191116
}
11201117

11211118
if (http_close) {
1119+
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
11221120
php_stream_close(stream);
1123-
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
11241121
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
11251122
stream = NULL;
11261123
}

‎ext/soap/tests/bugs/gh18990.phpt

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
--TEST--
2+
GH-18990 (SOAP HTTP socket not closing on object destruction)
3+
--INI--
4+
soap.wsdl_cache_enabled=0
5+
--EXTENSIONS--
6+
soap
7+
--SKIPIF--
8+
<?php
9+
require __DIR__.'/../../../standard/tests/http/server.inc';
10+
http_server_skipif();
11+
--FILE--
12+
<?php
13+
require __DIR__.'/../../../standard/tests/http/server.inc';
14+
15+
$wsdl = file_get_contents(__DIR__.'/../server030.wsdl');
16+
17+
$soap = <<<EOF
18+
<?xml version="1.0" encoding="UTF-8"?>
19+
<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/" xmlns:ns1="http://testuri.org" xmlns:SOAP-ENC="http://schemas.xmlsoap.org/soap/encoding/" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" SOAP-ENV:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/"><SOAP-ENV:Body><ns1:getItemsResponse><getItemsReturn SOAP-ENC:arrayType="ns1:Item[10]" xsi:type="ns1:ItemArray"><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text0</text></item><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text1</text></item><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text2</text></item><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text3</text></item><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text4</text></item><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text5</text></item><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text6</text></item><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text7</text></item><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text8</text></item><item xsi:type="ns1:Item"><text xsi:type="xsd:string">text9</text></item></getItemsReturn></ns1:getItemsResponse></SOAP-ENV:Body></SOAP-ENV:Envelope>
20+
EOF;
21+
22+
$responses = [
23+
"data://text/plain,HTTP/1.1 200 OK\r\n".
24+
"Content-Type: text/xml;charset=utf-8\r\n".
25+
"Connection: Keep-Alive\r\n".
26+
"Content-Length: ".strlen($wsdl)."\r\n".
27+
"\r\n".
28+
$wsdl,
29+
30+
"data://text/plain,HTTP/1.1 200 OK\r\n".
31+
"Content-Type: text/xml;charset=utf-8\r\n".
32+
"Connection: Keep-Alive\r\n".
33+
"Content-Length: ".strlen($soap)."\r\n".
34+
"\r\n".
35+
$soap,
36+
];
37+
38+
['pid' => $pid, 'uri' => $uri] = http_server($responses);
39+
40+
$options = [
41+
'trace' => false,
42+
'location' => $uri,
43+
];
44+
45+
$cnt = count(get_resources());
46+
47+
$client = new SoapClient($uri, $options);
48+
49+
var_dump(count($client->getItems()));
50+
51+
http_server_kill($pid);
52+
53+
unset($client);
54+
var_dump(count(get_resources()) - $cnt);
55+
?>
56+
--EXPECT--
57+
int(10)
58+
int(0)

0 commit comments

Comments
 (0)