[php-src] PR #24367: ext/soap: report encoding errors without a bailout
| From: | likemusic | Date: | Thu, 15 Oct 2026 23:32:28 +0000 |
| Subject: | [php-src] PR #24367: ext/soap: report encoding errors without a bailout | ||
| Groups: | php.git-pulls | ||
| Request: | Send a blank email to git-pulls+get-39494@lists.php.net to get a copy of this message | ||
Pull Request: https://github.com/php/php-src/pull/24367
Author: likemusic
Follow-up to #24366, which does the same for WSDL and XML schema errors. This branch is on top of
#24366, so the first commit here is #24366 itself; **only the last commit is new** ("ext/soap:
report encoding errors without a bailout"). I will rebase once #24366 is merged or changed.
With both, #10026 is fixed for SoapClient: an encoding error in a call (for example
SOAP-ERROR: Encoding: object has no 'x' property, or an invalid
xsd:int in the response) no longer leaves
gc_status()['protected'] at true for the rest of the request.
### What changes
- The 47 soap_errorN(E_ERROR, ...) calls in php_encoding.c use
soap_encoding_error(). In a call of a SoapClient that uses exceptions it
throws the same SoapFault as before (same message, fault code Client, also
stored in __soap_fault) and returns. Everywhere else, i.e. SoapServer and
SoapClient with exceptions => false, it is the same fatal error as
before.
- Encoders and decoders are deeply recursive and return xmlNodePtr/zval *,
so instead of propagating an error status through all of them, an encoder that hits an error returns
a placeholder (a BOGUS node or NULL) and the call discards the request or
the response. ext/soap already works like this for exceptions thrown in the middle of encoding, e.g.
the ValueError for recursive arrays in to_xml_map() or for non-backed
enums.
- Only the first error is thrown. Once it is pending, master_to_xml() and
master_to_zval() skip the rest of the value. Without that, a cycle of href
references that fails with Nesting level too deep would be decoded in exponential time,
since nothing stops at the first failure any more.
- After the error the request is not sent (as before, __getLastRequest() stays
null), decoded values are no longer stored into objects (a typed property of a classmap
class would throw a TypeError that hides the SoapFault), decoding of
response headers stops (the trace of the SoapFault holds the array of output headers,
writing to it trips an assertion in zend_hash), and a <Fault> whose
content fails to decode does not replace __soap_fault.
- Places where going on after the error would be unsafe now return: invalid base64, the array index
overflow checks, <boolean> with an element inside, an invalid timestamp,
unresolved references (they return the node itself, not NULL).
get_conversion() can return NULL now, so guess_xml_convert()
(which would otherwise recurse through UNKNOWN_TYPE forever) and
get_array_type() handle that.
One case behaves differently, on purpose: if a callback (for example a typemap to_xml)
has already thrown an exception when the encoding error happens, that exception is now the previous
exception of the SoapFault. Before, the fatal error printed it as an "Uncaught
..." warning and discarded it.
The server keeps its fatal errors: SoapServer::handle() answers with a SOAP fault and
ends the request, so the GC state does not matter there, and changing it would be a much larger
change of handle().
### Testing
Debug build of master (de7ff46337f) with #24366, unless noted otherwise.
- ext/soap: 520 tests pass.
- Two new tests: bugs/gh10026_encoding.phpt (one encoding and one decoding error, then
20000 cycles are collected), and encoding_errors_no_bailout.phpt, which goes through 48
encoding and decoding errors with a SoapClient, including the cases above (first error
wins, output headers, typed classmap property, a fault that fails to decode, a wide
href cycle). Both fail on master without this change, and only on the lines that check
the GC state; the messages are the same.
- A corpus of 59 cases (50 errors, valid values as a control), run on patched and unpatched master
in three modes, one process per run: SoapClient with exceptions,
SoapClient with exceptions => false, and SoapServer:
- with exceptions: the message, fault code, __soap_fault,
__getLastRequest(), output headers and exit code are the same in all 59 cases;
protected is true after the 50 errors on unpatched master and
false after all of them with the patch, and 1000 cycles created afterwards are
collected (0 on unpatched master). The only other difference is the previous exception
described above;
- exceptions => false and SoapServer: the output is byte-for-byte the
same before and after (50 fatal errors / SOAP faults);
- no leaks reported by the debug build, no assertions;
- with gcov, the corpus reaches 40 of the 47 error sites. The other 7 cannot be reached as far as
I can tell: the two Error calling to_xml/from_xml callback
(call_user_function() only fails when the executor is inactive), the two Invalid
value for type 'base64Binary' (php_base64_decode() is not strict and
does not fail), the two Internal Error (the default of a switch over all
type kinds), and Failed to encode list item for a list given as a string.
- Release build with ASAN and UBSAN, USE_ZEND_ALLOC=0: the corpus in all three modes
gives no report, with LeakSanitizer enabled for the SoapClient runs (leak detection off
for the fatal-error modes). run-tests --asan on ext/soap: the new tests
pass; the same 6 tests as on master fail with LeakSanitizer reports (bug37013,
bug55323, gh16318, gh22585, r3_groupE_list_005w,
r3_groupE_list_006w).
While testing I found an existing problem, independent of this PR: if a typemap
from_xml callback throws while a response header is decoded, the next header is written
to the output headers array that the exception trace shares, and a debug build fails with
_zend_hash_str_add_or_update_i: Assertion ... failed (same on master). I can send a
separate fix.
🤖 Generated with [Claude Code](https://claude.com/claude-code)