Implement request #61105: Support Soap 1.2 SoapFault Reason Text lang attribute #18701
Conversation
ext/soap/soap.c
Outdated
| ZVAL_EMPTY_STRING(Z_FAULT_STRING_P(obj)); | ||
| ZVAL_NULL(Z_FAULT_CODE_P(obj)); | ||
| ZVAL_NULL(Z_FAULT_CODENS_P(obj)); | ||
| ZVAL_NULL(Z_FAULT_ACTOR_P(obj)); | ||
| ZVAL_NULL(Z_FAULT_DETAIL_P(obj)); | ||
| ZVAL_NULL(Z_FAULT_NAME_P(obj)); | ||
| ZVAL_NULL(Z_FAULT_HEADERFAULT_P(obj)); | ||
| ZVAL_NULL(Z_FAULT_LANG_P(obj)); |
There was a problem hiding this comment.
Should this maybe be:
| ZVAL_NULL(Z_FAULT_LANG_P(obj)); | |
| ZVAL_EMPTY_STRING(Z_FAULT_LANG_P(obj)); |
?
There was a problem hiding this comment.
I'm uncertain. The other things are nullable too. The language attribute is actually only applicable if the client requested SOAP 1.2, it's unimportant in SOAP 1.1. I suppose this should be changed to an empty string for simplicity but I'd have to think a bit.
ext/soap/soap.c
Outdated
| add_soap_fault_ex(&exception_object, this_ptr, "Server", ZSTR_VAL(msg), NULL, NULL, SOAP_GLOBAL(lang_en)); | ||
| zend_string_release_ex(msg, 0); | ||
| } else { | ||
| add_soap_fault_ex(&exception_object, this_ptr, "Server", "Internal Error", NULL, NULL); | ||
| add_soap_fault_ex(&exception_object, this_ptr, "Server", "Internal Error", NULL, NULL, SOAP_GLOBAL(lang_en)); |
There was a problem hiding this comment.
Maybe having two new add_soap_fault_en{_ex}() APIs is better than passing that global into every call?
Similar to what you did with soap_server_fault_en()
|
The logic seems reasonable from really quick look and with my limited knowledge of SOAP. :) |
… attribute This is on the border line of a bugfix and a new feature. Anyway, this is necessary to fix compatibility with .NET clients.
|
Rebased + addressed comments |
ext/soap/soap.c
Outdated
| Z_PARAM_ARRAY_HT_OR_STR_OR_NULL(code_ht, code_str) | ||
| Z_PARAM_STRING(fault_string, fault_string_len) | ||
| Z_PARAM_OPTIONAL | ||
| Z_PARAM_STRING_OR_NULL(fault_actor, fault_actor_len) | ||
| Z_PARAM_ZVAL_OR_NULL(details) | ||
| Z_PARAM_STR_OR_NULL(name) | ||
| Z_PARAM_ZVAL_OR_NULL(headerfault) | ||
| Z_PARAM_PATH_STR_OR_NULL(lang) |
ext/soap/soap.c
Outdated
| soapServicePtr service; | ||
| xmlCharEncodingHandlerPtr old_encoding; | ||
|
|
||
| if (zend_parse_parameters(ZEND_NUM_ARGS(), "ss|szS", | ||
| if (zend_parse_parameters(ZEND_NUM_ARGS(), "ss|szSS", |
There was a problem hiding this comment.
You use the PATH specifier for the constructor but only string here for the new lang param
ext/soap/soap.c
Outdated
| @@ -660,18 +668,20 @@ PHP_METHOD(SoapFault, __construct) | |||
| char *fault_string = NULL, *fault_code = NULL, *fault_actor = NULL, *fault_code_ns = NULL; | |||
| size_t fault_string_len, fault_actor_len = 0, fault_code_len = 0; | |||
| zend_string *name = NULL; | |||
| zend_string *lang = NULL; | |||
This is on the border line of a bugfix and a new feature. Anyway, this is necessary to fix compatibility with .NET clients.