Skip to content
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
2 changes: 2 additions & 0 deletions NEWS
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ PHP NEWS
- SOAP:
. Fixed use of uninitialized func in do_request() on OOM bailout.
(David Carlier)
. Fixed bug GH-10026 (Garbage collection stops after exception in
SoapClient). (ndossche, Valery Ivashchanka)

- Standard:
. Fixed chown() and lchown() failing to resolve user names in ZTS builds
Expand Down
10 changes: 7 additions & 3 deletions Zend/zend.c
Original file line number Diff line number Diff line change
Expand Up @@ -1249,21 +1249,25 @@ ZEND_COLD void zenderror(const char *error) /* {{{ */
}
/* }}} */

ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout(const char *filename, uint32_t lineno) /* {{{ */
ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout_without_gc_protect(const char *filename, uint32_t lineno)
{

if (!EG(bailout)) {
zend_output_debug_string(1, "%s(%d) : Bailed out without a bailout address!", filename, lineno);
exit(-1);
}
gc_protect(1);
CG(unclean_shutdown) = 1;
CG(active_class_entry) = NULL;
CG(in_compilation) = 0;
CG(memoize_mode) = 0;
EG(current_execute_data) = NULL;
LONGJMP(*EG(bailout), FAILURE);
}

ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout(const char *filename, uint32_t lineno) /* {{{ */
{
gc_protect(true);
_zend_bailout_without_gc_protect(filename, lineno);
}
/* }}} */

ZEND_API size_t zend_get_page_size(void)
Expand Down
2 changes: 2 additions & 0 deletions Zend/zend.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ typedef struct _zend_utility_values {

typedef size_t (*zend_write_func_t)(const char *str, size_t str_length);

#define zend_bailout_without_gc_protect() _zend_bailout_without_gc_protect(__FILE__, __LINE__)
#define zend_bailout() _zend_bailout(__FILE__, __LINE__)

#define zend_try \
Expand Down Expand Up @@ -293,6 +294,7 @@ zend_result zend_post_startup(void);
void zend_set_utility_values(zend_utility_values *utility_values);
void zend_unload_modules(void);

ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout_without_gc_protect(const char *filename, uint32_t lineno);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

that s also something to definitely avoid in a stable branch.

ZEND_API ZEND_COLD ZEND_NORETURN void _zend_bailout(const char *filename, uint32_t lineno);
ZEND_API size_t zend_get_page_size(void);

Expand Down
2 changes: 1 addition & 1 deletion ext/soap/php_encoding.c
Original file line number Diff line number Diff line change
Expand Up @@ -1582,7 +1582,7 @@ static zval *to_zval_object_ex(zval *ret, encodeTypePtr type, xmlNodePtr data, z
master_to_zval(&data, attr->encode, dummy);
} zend_catch {
xmlFreeNode(dummy);
zend_bailout();
zend_bailout_without_gc_protect();
} zend_end_try();
xmlFreeNode(dummy);
set_zval_property(ret, attr->name, &data);
Expand Down
2 changes: 1 addition & 1 deletion ext/soap/php_packet_soap.c
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ static void master_to_zval_with_doc_cleanup(zval *ret, encodePtr encode, xmlNode
zval_ptr_dtor(ret);
}
xmlFreeDoc(doc);
zend_bailout();
zend_bailout_without_gc_protect();
}
}

Expand Down
4 changes: 2 additions & 2 deletions ext/soap/php_sdl.c
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ static void load_wsdl_ex(zval *this_ptr, char *struri, sdlCtx *ctx, int include)
load_wsdl_ex(this_ptr, (char*)uri, ctx, 1);
} zend_catch {
xmlFree(uri);
zend_bailout();
zend_bailout_without_gc_protect();
} zend_end_try();
xmlFree(uri);
}
Expand Down Expand Up @@ -1148,7 +1148,7 @@ static sdlPtr load_wsdl(zval *this_ptr, char *struri)
} zend_catch {
/* Avoid persistent memory leak. */
zend_hash_destroy(&ctx.docs);
zend_bailout();
zend_bailout_without_gc_protect();
} zend_end_try();

zend_hash_destroy(&ctx.messages);
Expand Down
25 changes: 21 additions & 4 deletions ext/soap/php_soap.h
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ ZEND_BEGIN_MODULE_GLOBALS(soap)
char* error_code;
zval error_object;
bool use_soap_error_handler;
bool error_is_own; /* error raised by ext/soap itself via soap_errorN() */
char cache_mode;
char cache_enabled;
char* cache_dir;
Expand Down Expand Up @@ -201,16 +202,32 @@ extern zend_class_entry* soap_sdl_class_entry;
void add_soap_fault(zval *obj, char *fault_code, char *fault_string, char *fault_actor, zval *fault_detail);

#define soap_error0(severity, format) \
php_error(severity, "SOAP-ERROR: " format)
do { \
SOAP_GLOBAL(error_is_own) = true; \

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this sort of solution around a global is really fragile.

php_error(severity, "SOAP-ERROR: " format); \
SOAP_GLOBAL(error_is_own) = false; \
} while (0)

#define soap_error1(severity, format, param1) \
php_error(severity, "SOAP-ERROR: " format, param1)
do { \
SOAP_GLOBAL(error_is_own) = true; \
php_error(severity, "SOAP-ERROR: " format, param1); \
SOAP_GLOBAL(error_is_own) = false; \
} while (0)

#define soap_error2(severity, format, param1, param2) \
php_error(severity, "SOAP-ERROR: " format, param1, param2)
do { \
SOAP_GLOBAL(error_is_own) = true; \
php_error(severity, "SOAP-ERROR: " format, param1, param2); \
SOAP_GLOBAL(error_is_own) = false; \
} while (0)

#define soap_error3(severity, format, param1, param2, param3) \
php_error(severity, "SOAP-ERROR: " format, param1, param2, param3)
do { \
SOAP_GLOBAL(error_is_own) = true; \
php_error(severity, "SOAP-ERROR: " format, param1, param2, param3); \
SOAP_GLOBAL(error_is_own) = false; \
} while (0)

static zend_always_inline zval *php_soap_deref(zval *zv) {
if (UNEXPECTED(Z_TYPE_P(zv) == IS_REFERENCE)) {
Expand Down
34 changes: 23 additions & 11 deletions ext/soap/soap.c
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ static void soap_error_handler(int error_num, zend_string *error_filename, uint3
Z_OBJ(SOAP_GLOBAL(error_object)) = _old_error_object;\
SOAP_GLOBAL(soap_version) = _old_soap_version;\
if (_bailout) {\
zend_bailout();\
zend_bailout_without_gc_protect();\
}

#define FETCH_THIS_SDL(ss) \
Expand Down Expand Up @@ -443,6 +443,7 @@ static void php_soap_init_globals(zend_soap_globals *soap_globals)
soap_globals->defEncNs = defEncNs;
soap_globals->typemap = NULL;
soap_globals->use_soap_error_handler = 0;
soap_globals->error_is_own = false;
soap_globals->error_code = NULL;
ZVAL_OBJ(&soap_globals->error_object, NULL);
soap_globals->sdl = NULL;
Expand Down Expand Up @@ -1008,7 +1009,7 @@ PHP_METHOD(SoapServer, __construct)
} zend_catch {
xmlCharEncCloseFunc(service->encoding);
service->encoding = NULL;
zend_bailout();
zend_bailout_without_gc_protect();
} zend_end_try();
if (service->uri == NULL) {
if (service->sdl->target_ns) {
Expand Down Expand Up @@ -1435,7 +1436,7 @@ PHP_METHOD(SoapServer, handle)
} zend_catch {
/* Avoid leaking persistent memory */
xmlFreeDoc(doc_request);
zend_bailout();
zend_bailout_without_gc_protect();
} zend_end_try();

xmlFreeDoc(doc_request);
Expand Down Expand Up @@ -1858,7 +1859,7 @@ static ZEND_NORETURN void soap_server_fault(char* code, char* string, char *acto
}
/* }}} */

static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, zend_string *error_filename, const uint32_t error_lineno, zend_string *message) /* {{{ */
static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, zend_string *error_filename, const uint32_t error_lineno, zend_string *message, bool error_is_own) /* {{{ */
{
bool _old_in_compilation;
zend_execute_data *_old_current_execute_data;
Expand All @@ -1884,6 +1885,12 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, z
add_soap_fault_ex(&fault, &SOAP_GLOBAL(error_object), code, ZSTR_VAL(message), NULL, NULL);
Z_ADDREF(fault);
zend_throw_exception_object(&fault);
if (error_is_own) {
/* ext/soap raised this error itself at a known point, so execution
* can safely continue after the fault is caught. Engine fatal errors
* (e.g. memory exhaustion, timeouts) keep the GC protected. */
zend_bailout_without_gc_protect();
}
zend_bailout();
} else if (!use_exceptions ||
!SOAP_GLOBAL(error_code) ||
Expand Down Expand Up @@ -1954,10 +1961,15 @@ static zend_never_inline ZEND_COLD void soap_real_error_handler(int error_num, z

static void soap_error_handler(int error_num, zend_string *error_filename, const uint32_t error_lineno, zend_string *message) /* {{{ */
{
bool error_is_own = SOAP_GLOBAL(error_is_own);

/* Consume the flag right away: any error raised while handling this one
* (e.g. memory exhaustion) is not a SOAP error. */
SOAP_GLOBAL(error_is_own) = false;
if (EXPECTED(!SOAP_GLOBAL(use_soap_error_handler))) {
old_error_handler(error_num, error_filename, error_lineno, message);
} else {
soap_real_error_handler(error_num, error_filename, error_lineno, message);
soap_real_error_handler(error_num, error_filename, error_lineno, message, error_is_own);
}
}
/* }}} */
Expand Down Expand Up @@ -2286,7 +2298,7 @@ static bool do_request(zval *this_ptr, xmlDoc *request, const char *location, co
zval_ptr_dtor(&params[0]);
xmlFree(buf);
if (_bailout) {
zend_bailout();
zend_bailout_without_gc_protect();
}
if (ret && Z_TYPE_P(Z_CLIENT_SOAP_FAULT_P(this_ptr)) == IS_OBJECT) {
ret = false;
Expand Down Expand Up @@ -2429,7 +2441,7 @@ static void do_soap_call(zend_execute_data *execute_data,
encode_finish();
if (parse_bailout) {
zval_ptr_dtor(&response);
zend_bailout();
zend_bailout_without_gc_protect();
}
}

Expand Down Expand Up @@ -2483,7 +2495,7 @@ static void do_soap_call(zend_execute_data *execute_data,
encode_finish();
if (parse_bailout) {
zval_ptr_dtor(&response);
zend_bailout();
zend_bailout_without_gc_protect();
}
}

Expand Down Expand Up @@ -2531,7 +2543,7 @@ static void do_soap_call(zend_execute_data *execute_data,
if (request) {
xmlFreeDoc(request);
}
zend_bailout();
zend_bailout_without_gc_protect();
}
SOAP_CLIENT_END_CODE();
}
Expand Down Expand Up @@ -3907,7 +3919,7 @@ static xmlDocPtr serialize_response_call(sdlFunctionPtr function, const char *fu
} zend_catch {
/* Avoid persistent memory leak. */
xmlFreeDoc(doc);
zend_bailout();
zend_bailout_without_gc_protect();
} zend_end_try();

if (function && function->responseName == NULL &&
Expand Down Expand Up @@ -4113,7 +4125,7 @@ static xmlDocPtr serialize_function_call(zval *this_ptr, sdlFunctionPtr function
} zend_catch {
/* Avoid persistent memory leak. */
xmlFreeDoc(doc);
zend_bailout();
zend_bailout_without_gc_protect();
} zend_end_try();

return doc;
Expand Down
65 changes: 65 additions & 0 deletions ext/soap/tests/gh10026.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
--TEST--
GH-10026 (Garbage collection stops after exception in SoapClient)
--EXTENSIONS--
soap
--FILE--
<?php

class A
{
public $b;
}

class B
{
public $a;
}

function buildCycle()
{
$a = new A();
$b = new B();
$a->b = $b;
$b->a = $a;
}

function failySoapCall()
{
try {
new SoapClient('https://127.0.0.1/?WSDL');
} catch (Exception) {
echo "Soap call failed\n";
}
}

function gc()
{
gc_collect_cycles();
echo "GC Runs: " . gc_status()['runs'] . "\n";
}


buildCycle();
gc();
buildCycle();
gc();
buildCycle();
gc();

failySoapCall();

buildCycle();
gc();
buildCycle();
gc();
buildCycle();
gc();
?>
--EXPECT--
GC Runs: 1
GC Runs: 2
GC Runs: 3
Soap call failed
GC Runs: 4
GC Runs: 5
GC Runs: 6
27 changes: 27 additions & 0 deletions ext/soap/tests/gh10026_engine_fatal.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
--TEST--
GH-10026 (Engine fatal errors converted to SoapFault keep the GC protected)
--EXTENSIONS--
soap
--INI--
memory_limit=8M
--FILE--
<?php
class TestSoapClient extends SoapClient {
public function __doRequest($request, $location, $action, $version, $oneWay = false): ?string {
return str_repeat("x", 64 * 1024 * 1024);
}
}

var_dump(gc_status()["protected"]);
$client = new TestSoapClient(null, ["location" => "test://", "uri" => "urn:test"]);
try {
$client->test();
} catch (SoapFault $e) {
echo $e->getMessage(), "\n";
}
var_dump(gc_status()["protected"]);
?>
--EXPECTF--
bool(false)
Allowed memory size of %d bytes exhausted%s(tried to allocate %d bytes)
bool(true)
25 changes: 25 additions & 0 deletions ext/soap/tests/gh10026_protected.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
--TEST--
GH-10026 (WSDL errors raised by ext/soap must not leave the GC protected)
--EXTENSIONS--
soap
--FILE--
<?php
$wsdl = __DIR__ . "/gh10026_protected.wsdl";
file_put_contents($wsdl, "not a wsdl at all");

var_dump(gc_status()["protected"]);
try {
new SoapClient($wsdl, ["cache_wsdl" => WSDL_CACHE_NONE]);
} catch (SoapFault $e) {
echo trim($e->getMessage()), "\n";
}
var_dump(gc_status()["protected"]);
?>
--CLEAN--
<?php
@unlink(__DIR__ . "/gh10026_protected.wsdl");
?>
--EXPECTF--
bool(false)
SOAP-ERROR: Parsing WSDL: Couldn't load from %s
bool(false)
Loading