ext/soap: setting xml namespace in classmap

[lxShaDoWxl]

Added support setting namespace in classmap. For multiple namespace and not unique name type

[nielsdos]

Thanks for your PR.
I have some remarks for changes to the code that'll simplify this a bit and improve performance.

[lxShaDoWxl]

Thanks for your PR. I have some remarks for changes to the code that'll simplify this a bit and improve performance.

Thanks! 🙏

[nielsdos]

Thanks for the changes, looks pretty good.

One thing I wonder about the classmap is this; if we have:
'classmap' => array('{http://schemas.nothing.com}book'=>'bookNs', 'book'=>'book')
then I expect a book in the namespace http://schemas.nothing.com to return an object of type bookNs. But now it returns an object of type book. That happens because to_zval_object_ex first looks up in SOAP_GLOBAL(class_map) without taking into account the namespace. Only when that returns NULL the namespace lookup happens. I think that's unintuitive and the checks probably should be reversed. What do you think?

[lxShaDoWxl]

maybe change the order?

1. search with namespace
2. if null to search whout namespace
   the initial search order made sure that there was no unnecessary search in the old php code

[nielsdos]

Perhaps changed order. I get that you want to avoid the additional lookup. Makes me think that the zend_string construction could be cached maybe too.
Anyway, let's maybe hear @dstogov 's opinion on this, because he developed the SOAP extension in the first place.

[dstogov]

• I think it's better to change the order (first search in namespace).
• Usage of zend_strpprintf() on each lookup looks inefficient (additional memory allocation/deallocation). It would be great to find a better solution.

[dstogov]

In general this looks good.
I have only minor comments, and I don't see a simple decision to get rid from zend_strprintf(). So I wouldn't object about committing the patch in the current state.

@nielsdos please take care about finalizing and landing this.

[nielsdos]

@lxShaDoWxl Let's change the check order.

Also, an idea to avoid constantly allocating temporary strings:

• Add a field to _encodeType: zend_string *cached_clark_notation;. This will lazily cache the clark notation string.
• If cached_clark_notation is non-NULL, don't create a temporary string, but use that string for lookup.
• If it is NULL you need to make a temp string. When creating a temporary string, you can store it in cached_clark_notation if the sdl is non-persistent.
• Make sure to zend_string_addref at the right locations to copy the string over
• For the const encode defaultEncoding[] table you can set up the strings as persistent in module initialization, and deallocate them in module finalization.

If you're unsure how to do it, I can take it from here if you want :)

[lxShaDoWxl]

@nielsdos please do it 🙏 I didn't understand everything myself how to do it😅

[nielsdos]

Okay no worries, I'll have a look later this week to finish this.

[lxShaDoWxl]

@nielsdos I did it a little differently, please take a look lxShaDoWxl@bbbb545

[nielsdos]

@lxShaDoWxl that looks pretty good, thank you! I left some comments, after addressing those feel free to add it to this PR.
The only thing left is that the builtin convertors inside the defaultEncoding are not supported with the clark notation because their pointer stays NULL.
You can fix that by removing the const from the array, and filling in the zend_strings in the PHP_MINIT_FUNCTION(soap) in soap.c, and releasing them in the PHP_MSHUTDOWN_FUNCTION(soap) function in soap.c

[lxShaDoWxl]

@nielsdos I added the defaultEncoding initialization to php_soap_prepare_globals. Can't do zend_string_release in PHP_MSHUTDOWN_FUNCTION 😰
not work code

int i;
i = 0;
do {
    if(defaultEncoding[i].details.clark_notation){
        zend_string_release_ex(defaultEncoding[i].details.clark_notation, 1);
    }
    i++;
} while (defaultEncoding[i].details.type != END_KNOWN_TYPES);

[nielsdos]

@lxShaDoWxl I've pushed the fix for your issue to your branch. The code you posted for MSHUTDOWN should now work.
The problem was that spprintf & zend_strpprintf allocate request strings, not persistent strings, a simply copy fixes it.

[lxShaDoWxl]

@nielsdos Thanks! I added my code to PHP_MSHUTDOWN_FUNCTION

[nielsdos]

I merged this manually as I had to rebase onto master, and I performed some small style tweaks and added a note to NEWS and UPGRADING.
Thank you very much for your contribution! Good work.
Merged as e58af7c

[dstogov]

It seems some of the last soap patches broke something. See https://github.com/php/php-src/actions/runs/6621079042/job/17984591267

@nielsdos please take care

[nielsdos]

Ok I will be able to check tonight.

[nielsdos]

Fixed here: #12514