Skip to content

Commit 59ac05e

Browse files
committed
Address review
assertion for C programming error, normalize another warning case to error + test
1 parent 81022bc commit 59ac05e

File tree

4 files changed

+37
-12
lines changed

4 files changed

+37
-12
lines changed

ext/dom/element.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -873,10 +873,10 @@ PHP_METHOD(DOMElement, setAttributeNodeNS)
873873

874874
DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
875875

876-
if (attrp->type != XML_ATTRIBUTE_NODE) {
877-
zend_argument_value_error(1, "must have node attribute");
878-
RETURN_THROWS();
879-
}
876+
/* ZPP Guarantees that a DOMAttr class is given, as it is converted to a xmlAttr
877+
* to pass to libxml (see http://www.xmlsoft.org/html/libxml-tree.html#xmlAttr)
878+
* if it is not of type XML_ATTRIBUTE_NODE it indicates a bug somewhere */
879+
ZEND_ASSERT(attrp->type == XML_ATTRIBUTE_NODE);
880880

881881
if (!(attrp->doc == NULL || attrp->doc == nodep->doc)) {
882882
php_dom_throw_error(WRONG_DOCUMENT_ERR, dom_get_strict_error(intern->document));

ext/dom/node.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ readonly=yes
3535
URL: http://www.w3.org/TR/2003/WD-DOM-Level-3-Core-20030226/DOM3-Core.html#core-ID-F68D095
3636
Since:
3737
*/
38-
zend_result dom_node_node_name_read(dom_object *obj, zval *retval)
38+
int dom_node_node_name_read(dom_object *obj, zval *retval)
3939
{
4040
xmlNode *nodep;
4141
xmlNsPtr ns;
@@ -97,9 +97,7 @@ zend_result dom_node_node_name_read(dom_object *obj, zval *retval)
9797
case XML_TEXT_NODE:
9898
str = "#text";
9999
break;
100-
default:
101-
zend_value_error("XML Node type must be one of XML_*_NODE");
102-
return FAILURE;
100+
EMPTY_SWITCH_DEFAULT_CASE();
103101
}
104102

105103
if (str != NULL) {
@@ -1603,11 +1601,13 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
16031601

16041602
tmp = zend_hash_str_find(ht, "query", sizeof("query")-1);
16051603
if (!tmp) {
1606-
zend_argument_value_error(3, "\"query\" option must be in XPath array");
1604+
/* if mode == 0 then $xpath arg is 3, if mode == 1 then $xpath is 4 */
1605+
zend_argument_value_error(3 + mode, "must have a \"query\" key");
16071606
RETURN_THROWS();
16081607
}
16091608
if (Z_TYPE_P(tmp) != IS_STRING) {
1610-
zend_argument_type_error(3, "\"query\" option must be a string, %s given", zend_zval_type_name(tmp));
1609+
/* if mode == 0 then $xpath arg is 3, if mode == 1 then $xpath is 4 */
1610+
zend_argument_type_error(3 + mode, "\"query\" option must be a string, %s given", zend_zval_type_name(tmp));
16111611
RETURN_THROWS();
16121612
}
16131613
xquery = Z_STRVAL_P(tmp);
@@ -1638,8 +1638,8 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
16381638
xmlXPathFreeObject(xpathobjp);
16391639
}
16401640
xmlXPathFreeContext(ctxp);
1641-
php_error_docref(NULL, E_WARNING, "XPath query did not return a nodeset.");
1642-
RETURN_FALSE;
1641+
zend_throw_error(NULL, "XPath query did not return a nodeset");
1642+
RETURN_THROWS();
16431643
}
16441644
}
16451645

ext/dom/tests/DOMNode_C14NFile_basic.phpt

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,16 @@ $node = $doc->getElementsByTagName('title')->item(0);
2727
var_dump($node->C14NFile($output));
2828
$content = file_get_contents($output);
2929
var_dump($content);
30+
try {
31+
var_dump($node->C14NFile($output, false, false, []));
32+
} catch (\ValueError $e) {
33+
echo $e->getMessage() . \PHP_EOL;
34+
}
35+
try {
36+
var_dump($node->C14NFile($output, false, false, ['query' => []]));
37+
} catch (\TypeError $e) {
38+
echo $e->getMessage() . \PHP_EOL;
39+
}
3040
?>
3141
--CLEAN--
3242
<?php
@@ -36,3 +46,5 @@ unlink($output);
3646
--EXPECT--
3747
int(34)
3848
string(34) "<title>The Grapes of Wrath</title>"
49+
DOMNode::C14NFile(): Argument #4 ($xpath) must have a "query" key
50+
DOMNode::C14NFile(): Argument #4 ($xpath) "query" option must be a string, array given

ext/dom/tests/DOMNode_C14N_basic.phpt

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,19 @@ $doc = new DOMDocument();
2424
$doc->loadXML($xml);
2525
$node = $doc->getElementsByTagName('title')->item(0);
2626
var_dump($node->C14N());
27+
28+
try {
29+
var_dump($node->C14N(false, false, []));
30+
} catch (\ValueError $e) {
31+
echo $e->getMessage() . \PHP_EOL;
32+
}
33+
try {
34+
var_dump($node->C14N(false, false, ['query' => []]));
35+
} catch (\TypeError $e) {
36+
echo $e->getMessage() . \PHP_EOL;
37+
}
2738
?>
2839
--EXPECT--
2940
string(34) "<title>The Grapes of Wrath</title>"
41+
DOMNode::C14N(): Argument #3 ($xpath) must have a "query" key
42+
DOMNode::C14N(): Argument #3 ($xpath) "query" option must be a string, array given

0 commit comments

Comments
 (0)