soap #73237 - Duplicate zval so it's not overwritten on next iteration#2153
soap #73237 - Duplicate zval so it's not overwritten on next iteration#2153ksmiley wants to merge 3 commits intophp:PHP-7.0from
Conversation
If the response includes both fields with simple types (which get concatenated into an XML string) and a complex type (which is parsed into an object), then the object will parsed into the same zval as the simple types and will overwrite the string.
ksmiley
force-pushed
the
soap/bug73237/parseAnyElement
branch
from
b986be1 to
a44d945
Compare
|
Note that there's a memory leak in ZTS mode. |
|
Thanks @cmb69. I added a commit that seems to fix the leak. |
|
How close are we to merging this -- ran into a pretty nasty situation at work and this seems like it would resolve it. |
nikic
left a comment
nikic
left a comment
There was a problem hiding this comment.
Fix looks right to me. I've left two notes to avoid unnecessary copies/leaks.
| ZVAL_NULL(&val2); | ||
| master_to_zval(&val2, get_conversion(XSD_ANYXML), node->next); | ||
| if (Z_TYPE(val2) != IS_STRING || *Z_STRVAL(val) != '<') { | ||
| Z_TRY_DELREF(val2); |
There was a problem hiding this comment.
This should be zval_ptr_dtor(val2);
There was a problem hiding this comment.
When I change it to zval_ptr_dtor(&val2), I consistently get a segfault. It looks like calling the destructor invalidates an object that was stored in SOAP_GLOBAL(ref_map), and that object is needed (by soap_check_xml_ref) on the next iteration of the outer loop.
There was a problem hiding this comment.
Hrm, in that case I'm wondering why the zval_ptr_dtor(&val2) four lines below cannot cause the same issue.
There was a problem hiding this comment.
This inner loop is peeking ahead in the XML document (node->next), and this if decides whether to consume the next node or leave it for the outer loop. I think the reason the zval_ptr_dtor below doesn't cause a problem is that the inner loop has already consumed the next node, and it won't be needed again.
There was a problem hiding this comment.
Thanks for the explanation, makes sense.
ext/soap/php_encoding.c
Outdated
| } else { | ||
| any = &val; | ||
| ZVAL_DUP(&keepVal, &val); | ||
| zval_dtor(&val); |
There was a problem hiding this comment.
These two lines should be ZVAL_COPY_VALUE(&keepVal, &val);
|
Merged as 2628713, thanks! |
4 participants
If the response includes both fields with simple types (which get concatenated into an XML string) and a complex type (which is parsed into an object), then the object will parsed into the same zval as the simple types and will overwrite the string.
https://bugs.php.net/bug.php?id=73237