Add libxml_get_external_entity_loader() #7977

tstarling · 2022-01-21T02:17:53Z

Fixes https://bugs.php.net/bug.php?id=76763 . Our motivation is https://phabricator.wikimedia.org/T268847

bukka · 2022-07-19T11:46:43Z

Apology for the late review. Personally I would prefer introducing a new function for this (could be simply libxml_get_external_entity_loader) as it won't break backward compatibility which this does in a way that this will no longer work:

if (libxml_set_external_entity_loader($loader) === true) {
    // do something
}

I actually did a bit of search in GitHub and haven't found such piece of code (just comparisons with false that would still work) but that doesn't mean that there is no such code in private repos. So think it's not worth to risk breaking that.

tstarling · 2022-07-29T04:36:21Z

@bukka Done. Do you want me to squash the commits or write some docs?

ext/libxml/libxml.c

ext/libxml/tests/libxml_get_external_entity_loader.phpt

ext/libxml/libxml.c

tstarling · 2022-08-11T07:06:56Z

Is this ready to go now? I think I have responded to all the comments.

bukka

It looks good to me. Please can you squash it?

tstarling · 2022-08-15T23:12:58Z

@bukka Done, squashed

bukka · 2022-08-18T14:02:41Z

@tstarling Just FYI I messaged internals if this and few other things could be added before the first 8.2 RC. If there are no objection, I will merge it later next week. If you have some spare time and could maybe use that Z_PARAM_FUNC_OR_NULL_WITH_ZVAL instead as discussed above that would be awesome!

jrfnl · 2022-08-18T16:10:57Z

🎗️ If this does go into PHP 8.2, it will need a changelog entry in UPGRADING introducing the new function.

Add libxml_get_external_entity_loader(), which returns the currently installed external entity loader, i.e. the value which was passed to libxml_set_external_entity_loader() or null if no loader was installed and the default entity loader will be used. This allows libraries to save and restore the loader, controlling entity expansion without interfering with the rest of the application. Add macro Z_PARAM_FUNC_OR_NULL_WITH_ZVAL(). This allows us to get the zval for a callable parameter without duplicating callable argument parsing. The saved zval keeps the object needed for fcc/fci alive, simplifying memory management. Fixes #76763.

tstarling · 2022-08-19T03:14:27Z

I added Z_PARAM_FUNC_OR_NULL_WITH_ZVAL() and added an entry to UPGRADING.

tstarling · 2022-08-19T04:49:25Z

AppVeyor says

=====================================================================
FAILED TEST SUMMARY
---------------------------------------------------------------------
Test hrtime() aligns with microtime() [C:\projects\php-src\ext\standard\tests\hrtime\hrtime.phpt]
=====================================================================

Pretty sure that's not my fault.

bukka · 2022-08-28T11:49:31Z

Merged via 1179622 . Thanks!

tstarling force-pushed the return-previous-loader branch from 9e28332 to dfacdbf Compare February 14, 2022 19:46

ramsey added Extension: libxml Waiting on Review labels May 27, 2022

ramsey added this to the PHP 8.2 milestone May 27, 2022

tstarling force-pushed the return-previous-loader branch from dfacdbf to 95f07fe Compare July 29, 2022 04:04

kocsismate reviewed Jul 29, 2022

View reviewed changes