Skip to content

Add DOMXPath::$enableRegisterNodeNS property #779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Add DOMXPath::$enableRegisterNodeNS property #779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ext/dom/dom_properties.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ int dom_typeinfo_type_namespace_read(dom_object *obj, zval **retval TSRMLS_DC);
#if defined(LIBXML_XPATH_ENABLED)
/* xpath properties */
int dom_xpath_document_read(dom_object *obj, zval **retval TSRMLS_DC);
int dom_xpath_enable_register_node_ns_read(dom_object *obj, zval **retval TSRMLS_DC);
int dom_xpath_enable_register_node_ns_write(dom_object *obj, zval *newval TSRMLS_DC);
#endif

#endif /* DOM_PROPERTIERS_H */
Expand Down
2 changes: 2 additions & 0 deletions ext/dom/php_dom.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,7 @@ PHP_MINIT_FUNCTION(dom)

zend_hash_init(&dom_xpath_prop_handlers, 0, NULL, NULL, 1);
dom_register_prop_handler(&dom_xpath_prop_handlers, "document", dom_xpath_document_read, NULL TSRMLS_CC);
dom_register_prop_handler(&dom_xpath_prop_handlers, "enableRegisterNodeNS", dom_xpath_enable_register_node_ns_read, dom_xpath_enable_register_node_ns_write TSRMLS_CC);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The property handlers need to be exported. The build fails because of undefined identifiers.

zend_hash_add(&classes, ce.name, ce.name_length + 1, &dom_xpath_prop_handlers, sizeof(dom_xpath_prop_handlers), NULL);
#endif

Expand Down Expand Up @@ -1226,6 +1227,7 @@ zend_object_value dom_xpath_objects_new(zend_class_entry *class_type TSRMLS_DC)
intern->registerPhpFunctions = 0;
intern->registered_phpfunctions = NULL;
intern->node_list = NULL;
intern->enable_register_node_ns = 1;

ALLOC_HASHTABLE(intern->registered_phpfunctions);
zend_hash_init(intern->registered_phpfunctions, 0, NULL, ZVAL_PTR_DTOR, 0);
Expand Down
1 change: 1 addition & 0 deletions ext/dom/php_dom.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ typedef struct _dom_xpath_object {
int registerPhpFunctions;
HashTable *registered_phpfunctions;
HashTable *node_list;
zend_bool enable_register_node_ns;
} dom_xpath_object;

typedef struct _dom_nnodemap_object {
Expand Down
34 changes: 34 additions & 0 deletions ext/dom/tests/bug55700.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
--TEST--
Bug #55700 (Disable automatic registration on a DOMXpath object)
--SKIPIF--
<?php require_once('skipif.inc'); ?>
--FILE--
<?php

$dom = new DOMDocument();
$dom->loadXML(
'<foobar><a:foo xmlns:a="urn:a">'.
'<b:bar xmlns:b="urn:b"/></a:foo>'.
'</foobar>'
);
$xpath = new DOMXPath($dom);

// disable automatic namespace registration
var_dump($xpath->enableRegisterNodeNS);
$xpath->enableRegisterNodeNS = FALSE;
var_dump($xpath->enableRegisterNodeNS);

$context = $dom->documentElement->firstChild;
$xpath->registerNamespace('a', 'urn:b');
var_dump(
$xpath->evaluate(
'descendant-or-self::a:*',
$context
)->item(0)->tagName
);

?>
--EXPECT--
bool(true)
bool(false)
string(5) "b:bar"
24 changes: 24 additions & 0 deletions ext/dom/xpath.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,25 @@ int dom_xpath_document_read(dom_object *obj, zval **retval TSRMLS_DC)
}
/* }}} */

/* {{{ enableRegisterNodeNS boolean
readonly=no
*/
int dom_xpath_enable_register_node_ns_read(dom_object *obj, zval **retval TSRMLS_DC)
{
MAKE_STD_ZVAL(*retval);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should show a leak in a debug build, but your test never reads the property.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no leak here (that I can see, checked with a debug build) - note that this is a property accessor and not a method, as such retval is a zval ** and so would be free'd by the caller. Were you reading this as a locally alloc'd and not free'd variable as it would be if this were a method dealing with return_value?

I've added two reads to the test to cover the code path it previously did not hit.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, sorry for picking around, I did not see that dom_read_property() already takes care of that.
Thing is, if you are returning a temporary zval from a property read handler, it should have a recount of 0, because the zval is not stored anywhere else.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe this also is handled by the caller at https://github.com/php/php-src/blob/PHP-5.6/ext/dom/php_dom.c#L367-369

Unless I'm misreading?

ZVAL_BOOL(*retval, ((dom_xpath_object *)obj)->enable_register_node_ns);

return SUCCESS;
}

int dom_xpath_enable_register_node_ns_write(dom_object *obj, zval *newval TSRMLS_DC)
{
((dom_xpath_object *)obj)->enable_register_node_ns = zend_is_true(newval);

return SUCCESS;
}
/* }}} */

/* {{{ proto boolean dom_xpath_register_ns(string prefix, string uri); */
PHP_FUNCTION(dom_xpath_register_ns)
{
Expand Down Expand Up @@ -430,6 +449,11 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */

ctxp->node = nodep;

if (ZEND_NUM_ARGS() < 3) {
/* register_node_ns was not passed, fetch default value from the property */
register_node_ns = intern->enable_register_node_ns;
}

if (register_node_ns) {
/* Register namespaces in the node */
ns = xmlGetNsList(docp, nodep);
Expand Down