Skip to content

ext/soap: report WSDL and schema errors without a bailout - #24366

Open
likemusic wants to merge 1 commit into
php:masterfrom
likemusic:soap-wsdl-exceptions
Open

likemusic wants to merge 1 commit into
php:masterfrom
likemusic:soap-wsdl-exceptions

Conversation

@likemusic

@likemusic likemusic commented Oct 15, 2026 •

Copy link
Copy Markdown

This is the direction suggested in the review of #24338: instead of a special bailout, the WSDL and XML schema loaders of ext/soap now report errors by throwing the SoapFault and returning, so SoapClient no longer goes through zend_bailout() for them. That is the case reported in #10026: a broken WSDL left gc_status()['protected'] at true for the rest of the request.

What changes

  • soap_throw_error() throws the same SoapFault that the error handler used to build from soap_errorN(E_ERROR, ...): same SOAP-ERROR: ... message, same fault code (WSDL).
  • All 135 soap_errorN() calls in php_sdl.c and php_schema.c use it now, and every function on the way up returns an error instead of relying on the bailout. Objects that are not yet owned by the sdl are freed where the error happens; everything else is freed by delete_sdl().
  • is_wsdl_element() returns -1 after throwing for an unknown required extension, so callers can tell that apart from "not a WSDL element".
  • SoapServer and SoapClient with exceptions => false keep the current behaviour: after the loader has cleaned up, the constructor turns the SoapFault back into the same E_ERROR (soap_fault_to_error()), so the server still answers with a SOAP fault and the client still dies with the same fatal error.
  • Code that only existed to survive the bailout is removed: the zend_try around WSDL imports in load_wsdl_ex(), requestify_string() and the targetNamespace copies in schema_load_file(). delete_type() now frees an unresolved ref, which an error before schema_pass2() can leave behind.

The diff of load_wsdl() is mostly re-indentation after removing its zend_try; it reads better with whitespace changes hidden (?w=1).

Encoding and decoding errors in php_encoding.c still bail out, so #10026 is not fully fixed by this PR alone: an encoding error in a call (for example SOAP-ERROR: Encoding: object has no 'x' property) still leaves protected at true. I would do that in a follow-up in the same way, unless you prefer it in this PR.

Testing

Debug build of master (0b4a5a84492), unless noted otherwise. After rebasing onto de7ff46337f (no changes in ext/soap in between), ext/soap was run again: all tests pass.

  • ext/soap: all tests pass.
  • Two new tests: bugs/gh10026.phpt, and wsdl_errors_no_bailout.phpt, which goes through 48 broken WSDL/schema files, one per error path with its own cleanup. On a debug build any leak on those paths fails the test. Both tests fail on unpatched master, and only on the lines that check the GC state; the messages are the same.
  • A larger corpus of 218 broken WSDL/schema files (generated by a script, I can share it), run on patched and unpatched master in three modes: SoapClient with exceptions, SoapClient with exceptions => false, and SoapServer:
    • with exceptions, the SoapFault message and fault code are identical in all 218 cases; protected is true after 216 of them on unpatched master and after none of them with the patch; no leaks, no assertions;
    • exceptions => false and SoapServer: output is byte-for-byte identical before and after (216 fatal errors / SOAP faults, 2 valid WSDLs);
    • with gcov, the corpus reaches 132 of the 135 error sites. The remaining three: Couldn't load from '%s' without a libxml error, Missing 'name' attribute for <binding> (a binding without a name is rejected earlier, in load_wsdl_ex()), and the second unexpected <%s> in union (after a loop that only ends with trav == NULL).
  • Release build with ASAN and UBSAN, USE_ZEND_ALLOC=0: the 218-file corpus in all three modes gives no ASAN or UBSAN report (leak detection off, since the fatal-error modes end the request without freeing; leaks are covered by the debug build above). run-tests --asan on ext/soap: both new tests pass; 6 other tests fail with LeakSanitizer reports (bug37013, bug55323, gh16318, gh22585, r3_groupE_list_005w, r3_groupE_list_006w), and they fail the same way without this patch.

🤖 Generated with Claude Code

WSDL and XML schema errors were raised with soap_errorN(E_ERROR, ...). SoapClient turned the fatal error into a SoapFault and caught the bailout in SOAP_CLIENT_END_CODE(), but zend_bailout() had already called gc_protect(1), so the cycle collector stayed off for the rest of the request (phpGH-10026).

The WSDL and schema loaders now throw the SoapFault with soap_throw_error() and return an error to their callers, which release what they own on the way out. The message and the fault code ("WSDL") are the same as before.

SoapServer and SoapClient with the "exceptions" option disabled keep reporting these errors as fatal errors: the constructor turns the SoapFault back into the same E_ERROR once the loader has cleaned up.

Since nothing bails out of the loaders any more, the error paths that existed only to survive a bailout are gone: the zend_try in load_wsdl_ex(), requestify_string() and the copies of targetNamespace in schema_load_file(). delete_type() now also frees an unresolved "ref", which an error before schema_pass2() can leave behind.

Errors from encoding and decoding values still bail out and are not covered by this change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@likemusic

Copy link
Copy Markdown
Author

Encoding and decoding errors are in a follow-up on top of this one: #24367.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant