Skip to content

Commit a068a9a

Browse files
committed
Make XMLDocument::xinclude() return values and error conditions sane
See https://wiki.php.net/rfc/dom_additions_84#api_amendments
1 parent 5236551 commit a068a9a

File tree

5 files changed

+101
-30
lines changed

5 files changed

+101
-30
lines changed

ext/dom/document.c

Lines changed: 51 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1706,17 +1706,36 @@ static void php_dom_remove_xinclude_nodes(xmlNodePtr cur) /* {{{ */
17061706
}
17071707
/* }}} */
17081708

1709-
/* {{{ Substitutues xincludes in a DomDocument */
1709+
/* {{{ Substitutes xincludes in a DomDocument */
1710+
static int dom_perform_xinclude(xmlDocPtr docp, dom_object *intern, zend_long flags)
1711+
{
1712+
PHP_LIBXML_SANITIZE_GLOBALS(xinclude);
1713+
int err = xmlXIncludeProcessFlags(docp, (int)flags);
1714+
PHP_LIBXML_RESTORE_GLOBALS(xinclude);
1715+
1716+
/* XML_XINCLUDE_START and XML_XINCLUDE_END nodes need to be removed as these
1717+
are added via xmlXIncludeProcess to mark beginning and ending of xincluded document
1718+
but are not wanted in resulting document - must be done even if err as it could fail after
1719+
having processed some xincludes */
1720+
xmlNodePtr root = docp->children;
1721+
while (root && root->type != XML_ELEMENT_NODE && root->type != XML_XINCLUDE_START) {
1722+
root = root->next;
1723+
}
1724+
if (root) {
1725+
php_dom_remove_xinclude_nodes(root);
1726+
}
1727+
1728+
php_libxml_invalidate_node_list_cache(intern->document);
1729+
1730+
return err;
1731+
}
1732+
17101733
PHP_METHOD(DOMDocument, xinclude)
17111734
{
1712-
zval *id;
17131735
xmlDoc *docp;
1714-
xmlNodePtr root;
17151736
zend_long flags = 0;
1716-
int err;
17171737
dom_object *intern;
17181738

1719-
id = ZEND_THIS;
17201739
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|l", &flags) == FAILURE) {
17211740
RETURN_THROWS();
17221741
}
@@ -1726,30 +1745,40 @@ PHP_METHOD(DOMDocument, xinclude)
17261745
RETURN_FALSE;
17271746
}
17281747

1729-
DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
1748+
DOM_GET_OBJ(docp, ZEND_THIS, xmlDocPtr, intern);
17301749

1731-
PHP_LIBXML_SANITIZE_GLOBALS(xinclude);
1732-
err = xmlXIncludeProcessFlags(docp, (int)flags);
1733-
PHP_LIBXML_RESTORE_GLOBALS(xinclude);
1750+
int err = dom_perform_xinclude(docp, intern, flags);
17341751

1735-
/* XML_XINCLUDE_START and XML_XINCLUDE_END nodes need to be removed as these
1736-
are added via xmlXIncludeProcess to mark beginning and ending of xincluded document
1737-
but are not wanted in resulting document - must be done even if err as it could fail after
1738-
having processed some xincludes */
1739-
root = (xmlNodePtr) docp->children;
1740-
while(root && root->type != XML_ELEMENT_NODE && root->type != XML_XINCLUDE_START) {
1741-
root = root->next;
1752+
if (err) {
1753+
RETVAL_LONG(err);
1754+
} else {
1755+
RETVAL_FALSE;
17421756
}
1743-
if (root) {
1744-
php_dom_remove_xinclude_nodes(root);
1757+
}
1758+
1759+
PHP_METHOD(Dom_XMLDocument, xinclude)
1760+
{
1761+
xmlDoc *docp;
1762+
zend_long flags = 0;
1763+
dom_object *intern;
1764+
1765+
if (zend_parse_parameters(ZEND_NUM_ARGS(), "|l", &flags) == FAILURE) {
1766+
RETURN_THROWS();
17451767
}
17461768

1747-
php_libxml_invalidate_node_list_cache(intern->document);
1769+
if (ZEND_LONG_EXCEEDS_INT(flags)) {
1770+
zend_argument_value_error(1, "is too large");
1771+
RETURN_THROWS();
1772+
}
17481773

1749-
if (err) {
1750-
RETVAL_LONG(err);
1774+
DOM_GET_OBJ(docp, ZEND_THIS, xmlDocPtr, intern);
1775+
1776+
int err = dom_perform_xinclude(docp, intern, flags);
1777+
1778+
if (err < 0) {
1779+
php_dom_throw_error(INVALID_MODIFICATION_ERR, /* strict */ true);
17511780
} else {
1752-
RETVAL_FALSE;
1781+
RETURN_LONG(err);
17531782
}
17541783
}
17551784
/* }}} */

ext/dom/php_dom.stub.php

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1617,8 +1617,7 @@ public function createEntityReference(string $name): EntityReference {}
16171617
/** @implementation-alias DOMDocument::validate */
16181618
public function validate(): bool {}
16191619

1620-
/** @implementation-alias DOMDocument::xinclude */
1621-
public function xinclude(int $options = 0): int|false {}
1620+
public function xinclude(int $options = 0): int {}
16221621

16231622
public function saveXml(?Node $node = null, int $options = 0): string|false {}
16241623

ext/dom/php_dom_arginfo.h

Lines changed: 4 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ext/dom/tests/dom_xinclude.phpt

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,27 @@ in_array('compress.zlib', stream_get_wrappers()) or die('skip compress.zlib wrap
88
?>
99
--FILE--
1010
<?php
11-
$dom = new domdocument;
12-
1311
$data = file_get_contents(__DIR__."/xinclude.xml");
1412
$reldir = str_replace(getcwd(),".",__DIR__);
1513
if (DIRECTORY_SEPARATOR == '\\') {
1614
$reldir = str_replace('\\',"/", $reldir);
1715
}
1816
$data = str_replace('compress.zlib://ext/dom/tests/','compress.zlib://'.$reldir."/", $data);
1917

20-
18+
$dom = new domdocument;
2119
$dom->loadXML($data);
2220
$dom->xinclude();
2321
print $dom->saveXML()."\n";
2422
foreach ($dom->documentElement->childNodes as $node) {
2523
print $node->nodeName."\n";
2624
}
25+
26+
$dom = Dom\XMLDocument::createFromString($data);
27+
$dom->xinclude();
28+
print $dom->saveXML()."\n";
29+
foreach ($dom->documentElement->childNodes as $node) {
30+
print $node->nodeName."\n";
31+
}
2732
?>
2833
--EXPECTF--
2934
<?xml version="1.0"?>
@@ -41,3 +46,17 @@ foreach ($dom->documentElement->childNodes as $node) {
4146
book
4247
book
4348
#text
49+
<?xml version="1.0" encoding="UTF-8"?>
50+
<foo xmlns:xi="http://www.w3.org/2001/XInclude">
51+
<book xml:base="compress.zlib://./ext/dom/tests/book.xml">
52+
<title>The Grapes of Wrath</title>
53+
<author>John Steinbeck</author>
54+
</book><book xml:base="compress.zlib://./ext/dom/tests/book.xml">
55+
<title>The Pearl</title>
56+
<author>John Steinbeck</author>
57+
</book>
58+
</foo>
59+
#text
60+
book
61+
book
62+
#text
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
--TEST--
2+
Test: Xinclude errors
3+
--EXTENSIONS--
4+
dom
5+
--SKIPIF--
6+
<?php
7+
if (PHP_INT_SIZE != 8) die("skip for 64bit platforms only");
8+
?>
9+
--FILE--
10+
<?php
11+
$dom = new DOMDocument;
12+
$dom->xinclude(PHP_INT_MAX);
13+
14+
$dom = Dom\XMLDocument::createEmpty();
15+
try {
16+
$dom->xinclude(PHP_INT_MAX);
17+
} catch (ValueError $e) {
18+
echo $e->getMessage();
19+
}
20+
?>
21+
--EXPECTF--
22+
Warning: DOMDocument::xinclude(): Invalid flags in %s on line %d
23+
Dom\XMLDocument::xinclude(): Argument #1 ($options) is too large

0 commit comments

Comments
 (0)