Skip to content

Fix GH-18990, bug #81029, bug #47314: SOAP HTTP socket not closing on object destruction #19001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 9 additions & 12 deletions ext/soap/php_http.c
Original file line number Diff line number Diff line change
Expand Up @@ -511,9 +511,9 @@ int make_http_soap_request(zval *this_ptr,
zend_string_equals(orig->host, phpurl->host) &&
orig->port == phpurl->port))) {
} else {
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
php_stream_close(stream);
convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr));
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
stream = NULL;
use_proxy = 0;
Expand All @@ -522,9 +522,9 @@ int make_http_soap_request(zval *this_ptr,

/* Check if keep-alive connection is still opened */
if (stream != NULL && php_stream_eof(stream)) {
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
php_stream_close(stream);
convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr));
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
stream = NULL;
use_proxy = 0;
Expand All @@ -533,9 +533,7 @@ int make_http_soap_request(zval *this_ptr,
if (!stream) {
stream = http_connect(this_ptr, phpurl, use_ssl, context, &use_proxy);
if (stream) {
php_stream_auto_cleanup(stream);
ZVAL_RES(Z_CLIENT_HTTPSOCKET_P(this_ptr), stream->res);
GC_ADDREF(stream->res);
php_stream_to_zval(stream, Z_CLIENT_HTTPSOCKET_P(this_ptr));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for the auto cleanup anymore ? (IDK what that function does)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That macro will also do the same thing that auto cleanup does:

php-src/main/php_streams.h

Lines 270 to 274 in 59dd0f8

#define php_stream_auto_cleanup(stream) { (stream)->__exposed = 1; }
/* use this to assign the stream to a zval and tell the stream that is
* has been exported to the engine; it will expect to be closed automatically
* when the resources are auto-destructed */
#define php_stream_to_zval(stream, zval) { ZVAL_RES(zval, (stream)->res); (stream)->__exposed = 1; }

Anyway, that "auto cleanup" name is a badly chosen name. It just means that the resource is exposed as a zval and will be cleaned up that way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right... that is indeed a bad name

ZVAL_LONG(Z_CLIENT_USE_PROXY_P(this_ptr), use_proxy);
} else {
php_url_free(phpurl);
Expand All @@ -555,7 +553,6 @@ int make_http_soap_request(zval *this_ptr,
zval *cookies, *login, *password;
zend_resource *ret = zend_register_resource(phpurl, le_url);
ZVAL_RES(Z_CLIENT_HTTPURL_P(this_ptr), ret);
GC_ADDREF(ret);

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

if (UNEXPECTED(php_random_bytes_throw(&nonce, sizeof(nonce)) != SUCCESS)) {
ZEND_ASSERT(EG(exception));
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
php_stream_close(stream);
convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr));
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
smart_str_free(&soap_headers_z);
smart_str_free(&soap_headers);
Expand Down Expand Up @@ -901,9 +898,9 @@ int make_http_soap_request(zval *this_ptr,
if (request != buf) {
zend_string_release_ex(request, 0);
}
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
php_stream_close(stream);
convert_to_null(Z_CLIENT_HTTPURL_P(this_ptr));
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
add_soap_fault(this_ptr, "HTTP", "Failed Sending HTTP SOAP request", NULL, NULL);
smart_str_free(&soap_headers_z);
Expand All @@ -919,8 +916,8 @@ int make_http_soap_request(zval *this_ptr,
}

if (!return_value) {
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
php_stream_close(stream);
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
smart_str_free(&soap_headers_z);
efree(http_msg);
Expand All @@ -933,8 +930,8 @@ int make_http_soap_request(zval *this_ptr,
if (request != buf) {
zend_string_release_ex(request, 0);
}
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
php_stream_close(stream);
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
add_soap_fault(this_ptr, "HTTP", "Error Fetching http headers", NULL, NULL);
smart_str_free(&soap_headers_z);
Expand Down Expand Up @@ -1102,9 +1099,9 @@ int make_http_soap_request(zval *this_ptr,
if (request != buf) {
zend_string_release_ex(request, 0);
}
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
php_stream_close(stream);
zend_string_release_ex(http_headers, 0);
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
add_soap_fault(this_ptr, "HTTP", "Error Fetching http body, No Content-Length, connection closed or chunked data", NULL, NULL);
if (http_msg) {
Expand All @@ -1119,8 +1116,8 @@ int make_http_soap_request(zval *this_ptr,
}

if (http_close) {
ZVAL_NULL(Z_CLIENT_HTTPSOCKET_P(this_ptr));
php_stream_close(stream);
convert_to_null(Z_CLIENT_HTTPSOCKET_P(this_ptr));
convert_to_null(Z_CLIENT_USE_PROXY_P(this_ptr));
stream = NULL;
}
Expand Down
58 changes: 58 additions & 0 deletions ext/soap/tests/bugs/gh18990.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
--TEST--
GH-18990 (SOAP HTTP socket not closing on object destruction)
--INI--
soap.wsdl_cache_enabled=0
--EXTENSIONS--
soap
--SKIPIF--
<?php
require __DIR__.'/../../../standard/tests/http/server.inc';
http_server_skipif();
--FILE--
<?php
require __DIR__.'/../../../standard/tests/http/server.inc';

$wsdl = file_get_contents(__DIR__.'/../server030.wsdl');

$soap = <<<EOF
<?xml version="1.0" encoding="UTF-8"?>
<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>
EOF;

$responses = [
"data://text/plain,HTTP/1.1 200 OK\r\n".
"Content-Type: text/xml;charset=utf-8\r\n".
"Connection: Keep-Alive\r\n".
"Content-Length: ".strlen($wsdl)."\r\n".
"\r\n".
$wsdl,

"data://text/plain,HTTP/1.1 200 OK\r\n".
"Content-Type: text/xml;charset=utf-8\r\n".
"Connection: Keep-Alive\r\n".
"Content-Length: ".strlen($soap)."\r\n".
"\r\n".
$soap,
];

['pid' => $pid, 'uri' => $uri] = http_server($responses);

$options = [
'trace' => false,
'location' => $uri,
];

$cnt = count(get_resources());

$client = new SoapClient($uri, $options);

var_dump(count($client->getItems()));

http_server_kill($pid);

unset($client);
var_dump(count(get_resources()) - $cnt);
?>
--EXPECT--
int(10)
int(0)
Loading