Skip to content

Commit a38e70f

Browse files
committed
Reviews
1 parent cc2dbba commit a38e70f

11 files changed

+88
-88
lines changed

ext/dom/document.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ int dom_document_encoding_read(dom_object *obj, zval *retval)
136136
return SUCCESS;
137137
}
138138

139-
int dom_document_encoding_write(dom_object *obj, zval *newval)
139+
zend_result dom_document_encoding_write(dom_object *obj, zval *newval)
140140
{
141141
xmlDoc *docp = (xmlDocPtr) dom_object_get_node(obj);
142142
zend_string *str;
@@ -161,8 +161,8 @@ int dom_document_encoding_write(dom_object *obj, zval *newval)
161161
}
162162
docp->encoding = xmlStrdup((const xmlChar *) ZSTR_VAL(str));
163163
} else {
164-
// Todo convert to error?
165-
php_error_docref(NULL, E_WARNING, "Invalid Document Encoding");
164+
zend_value_error("Invalid Document Encoding");
165+
return FAILURE;
166166
}
167167

168168
zend_string_release_ex(str, 0);

ext/dom/element.c

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -295,9 +295,8 @@ PHP_METHOD(DOMElement, setAttribute)
295295
attr = (xmlNodePtr)xmlSetProp(nodep, (xmlChar *) name, (xmlChar *)value);
296296
}
297297
if (!attr) {
298-
// TODO Convert to error?
299-
php_error_docref(NULL, E_WARNING, "No such attribute '%s'", name);
300-
RETURN_FALSE;
298+
zend_argument_value_error(1, "must be a valid XML attribute");
299+
RETURN_THROWS();
301300
}
302301

303302
DOM_RET_OBJ(attr, &ret, intern);
@@ -426,9 +425,8 @@ PHP_METHOD(DOMElement, setAttributeNode)
426425
DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
427426

428427
if (attrp->type != XML_ATTRIBUTE_NODE) {
429-
// Todo convert to a ValueError?
430-
php_error_docref(NULL, E_WARNING, "Attribute node is required");
431-
RETURN_FALSE;
428+
zend_argument_value_error(1, "must have the node attribute");
429+
RETURN_THROWS();
432430
}
433431

434432
if (!(attrp->doc == NULL || attrp->doc == nodep->doc)) {
@@ -878,9 +876,8 @@ PHP_METHOD(DOMElement, setAttributeNodeNS)
878876
DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
879877

880878
if (attrp->type != XML_ATTRIBUTE_NODE) {
881-
// Todo convert to error?
882-
php_error_docref(NULL, E_WARNING, "Attribute node is required");
883-
RETURN_FALSE;
879+
zend_argument_value_error(1, "must have node attribute");
880+
RETURN_THROWS();
884881
}
885882

886883
if (!(attrp->doc == NULL || attrp->doc == nodep->doc)) {

ext/dom/node.c

Lines changed: 19 additions & 18 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-
int dom_node_node_name_read(dom_object *obj, zval *retval)
38+
zend_result dom_node_node_name_read(dom_object *obj, zval *retval)
3939
{
4040
xmlNode *nodep;
4141
xmlNsPtr ns;
@@ -98,8 +98,8 @@ int dom_node_node_name_read(dom_object *obj, zval *retval)
9898
str = "#text";
9999
break;
100100
default:
101-
// Todo convert to error?
102-
php_error_docref(NULL, E_WARNING, "Invalid Node Type");
101+
zend_value_error("XML Node type must be one of XML_*_NODE");
102+
return FAILURE;
103103
}
104104

105105
if (str != NULL) {
@@ -984,9 +984,8 @@ PHP_METHOD(DOMNode, insertBefore)
984984
}
985985

986986
if (NULL == new_child) {
987-
// Todo convert to error?
988-
php_error_docref(NULL, E_WARNING, "Couldn't add newnode as the previous sibling of refnode");
989-
RETURN_FALSE;
987+
zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode");
988+
RETURN_THROWS();
990989
}
991990

992991
dom_reconcile_ns(parentp->doc, new_child);
@@ -1227,6 +1226,7 @@ PHP_METHOD(DOMNode, appendChild)
12271226
if (new_child == NULL) {
12281227
new_child = xmlAddChild(nodep, child);
12291228
if (new_child == NULL) {
1229+
// TODO Convert to Error?
12301230
php_error_docref(NULL, E_WARNING, "Couldn't append node");
12311231
RETURN_FALSE;
12321232
}
@@ -1577,9 +1577,8 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
15771577
docp = nodep->doc;
15781578

15791579
if (! docp) {
1580-
// TOdo convert to error?
1581-
php_error_docref(NULL, E_WARNING, "Node must be associated with a document");
1582-
RETURN_FALSE;
1580+
zend_throw_error(NULL, "Node must be associated with a document");
1581+
RETURN_THROWS();
15831582
}
15841583

15851584
if (xpath_array == NULL) {
@@ -1595,9 +1594,8 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
15951594
xmlXPathFreeObject(xpathobjp);
15961595
}
15971596
xmlXPathFreeContext(ctxp);
1598-
// Todo convert to error?
1599-
php_error_docref(NULL, E_WARNING, "XPath query did not return a nodeset.");
1600-
RETURN_FALSE;
1597+
zend_throw_error(NULL, "XPath query did not return a nodeset.");
1598+
RETURN_THROWS();
16011599
}
16021600
}
16031601
} else {
@@ -1607,13 +1605,16 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
16071605
char *xquery;
16081606

16091607
tmp = zend_hash_str_find(ht, "query", sizeof("query")-1);
1610-
if (tmp && Z_TYPE_P(tmp) == IS_STRING) {
1611-
xquery = Z_STRVAL_P(tmp);
1612-
} else {
1613-
// Todo convert to error?
1614-
php_error_docref(NULL, E_WARNING, "'query' missing from xpath array or is not a string");
1615-
RETURN_FALSE;
1608+
if (!tmp) {
1609+
// TODO Use argument version after checking which num arg it is
1610+
zend_value_error("\"query\" option must be in XPath array");
1611+
RETURN_THROWS();
1612+
}
1613+
if (Z_TYPE_P(tmp) != IS_STRING) {
1614+
zend_type_error("\"query\" option must be a string, %s given", zend_zval_type_name(tmp));
1615+
RETURN_THROWS();
16161616
}
1617+
xquery = Z_STRVAL_P(tmp);
16171618

16181619
ctxp = xmlXPathNewContext(docp);
16191620
ctxp->node = nodep;

ext/dom/php_dom.c

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -317,8 +317,9 @@ zval *dom_read_property(zend_object *object, zend_string *name, int type, void *
317317
if (obj->prop_handler != NULL) {
318318
hnd = zend_hash_find_ptr(obj->prop_handler, name);
319319
} else if (instanceof_function(obj->std.ce, dom_node_class_entry)) {
320-
// Should this be an error?
321-
php_error(E_WARNING, "Couldn't fetch %s. Node no longer exists", ZSTR_VAL(obj->std.ce->name));
320+
zend_throw_error(NULL, "Couldn't fetch %s. Node no longer exists", ZSTR_VAL(obj->std.ce->name));
321+
retval = &EG(uninitialized_zval);
322+
return retval;
322323
}
323324

324325
if (hnd) {
@@ -467,9 +468,8 @@ PHP_FUNCTION(dom_import_simplexml)
467468
if (nodep && nodeobj && (nodep->type == XML_ELEMENT_NODE || nodep->type == XML_ATTRIBUTE_NODE)) {
468469
DOM_RET_OBJ((xmlNodePtr) nodep, &ret, (dom_object *)nodeobj);
469470
} else {
470-
// Should this be an error?
471-
php_error_docref(NULL, E_WARNING, "Invalid Nodetype to import");
472-
RETURN_NULL();
471+
zend_argument_value_error(1, "cannot import invalid node type");
472+
RETURN_THROWS();
473473
}
474474
}
475475
/* }}} */
@@ -1210,8 +1210,8 @@ PHP_DOM_EXPORT zend_bool php_dom_create_object(xmlNodePtr obj, zval *return_valu
12101210
break;
12111211
}
12121212
default:
1213-
// Todo convert to value error?
1214-
php_error_docref(NULL, E_WARNING, "Unsupported node type: %d", obj->type);
1213+
/* TODO Convert to a ZEND assertion? */
1214+
zend_throw_error(NULL, "Unsupported node type: %d", obj->type);
12151215
ZVAL_NULL(return_value);
12161216
return 0;
12171217
}

ext/dom/php_dom.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,10 +150,9 @@ entry = zend_register_internal_class_ex(&ce, parent_ce);
150150
RETURN_THROWS(); \
151151
}
152152

153-
// Make this an error?
154153
#define DOM_NOT_IMPLEMENTED() \
155-
php_error_docref(NULL, E_WARNING, "Not yet implemented"); \
156-
return;
154+
zend_throw_error(NULL, "Not yet implemented"); \
155+
RETURN_THROWS();
157156

158157
#define DOM_NODELIST 0
159158
#define DOM_NAMEDNODEMAP 1

ext/dom/tests/DOMAttr_ownerElement_error_001.phpt

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,11 @@ $document->appendChild($root);
1414
$attr = $root->setAttribute('category', 'books');
1515
$document->removeChild($root);
1616
$root = null;
17-
var_dump($attr->ownerElement);
17+
try {
18+
var_dump($attr->ownerElement);
19+
} catch (\Error $e) {
20+
echo get_class($e) . ': ' . $e->getMessage() . \PHP_EOL;
21+
}
1822
?>
19-
--EXPECTF--
20-
Warning: Couldn't fetch DOMAttr. Node no longer exists in %s on line %d
21-
22-
Warning: Undefined property: DOMAttr::$ownerElement in %s on line %d
23-
NULL
23+
--EXPECT--
24+
Error: Couldn't fetch DOMAttr. Node no longer exists

ext/dom/tests/DOMDocument_adoptNode.phpt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ require_once('skipif.inc');
1010
$dom = new DOMDocument();
1111
$dom->loadXML("<root />");
1212

13-
$dom->adoptNode($dom->documentElement);
13+
try {
14+
$dom->adoptNode($dom->documentElement);
15+
} catch (\Error $e) {
16+
echo $e->getMessage() . \PHP_EOL;
17+
}
1418
?>
15-
--EXPECTF--
16-
Warning: DOMDocument::adoptNode(): Not yet implemented in %s
19+
--EXPECT--
20+
Not yet implemented

ext/dom/tests/DOMDocument_encoding_basic.phpt

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,14 @@ if( !$dom )
1919
exit;
2020
}
2121

22-
echo "Empty Encoding Read: {$dom->encoding}\n";
22+
echo "Empty Encoding Read: '{$dom->encoding}'\n";
2323

24-
$ret = $dom->encoding = 'NYPHP DOMinatrix';
25-
echo "Adding invalid encoding: $ret\n";
24+
try {
25+
$ret = $dom->encoding = 'NYPHP DOMinatrix';
26+
echo "Adding invalid encoding: $ret\n";
27+
} catch (\ValueError $e) {
28+
echo $e->getMessage() . \PHP_EOL;
29+
}
2630

2731
$ret = $dom->encoding = 'ISO-8859-1';
2832
echo "Adding ISO-8859-1 encoding: $ret\n";
@@ -38,11 +42,9 @@ echo "UTF-16 Encoding Read: {$dom->encoding}\n";
3842

3943

4044
?>
41-
--EXPECTF--
42-
Empty Encoding Read:
43-
44-
Warning: main(): Invalid Document Encoding in %s on line %d
45-
Adding invalid encoding: NYPHP DOMinatrix
45+
--EXPECT--
46+
Empty Encoding Read: ''
47+
Invalid Document Encoding
4648
Adding ISO-8859-1 encoding: ISO-8859-1
4749
ISO-8859-1 Encoding Read: ISO-8859-1
4850
Adding UTF-8 encoding: UTF-8

ext/dom/tests/bug36756.phpt

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,22 @@ $node = $xpath->query('/root')->item(0);
1313
echo $node->nodeName . "\n";
1414
$dom->removeChild($GLOBALS['dom']->firstChild);
1515
echo "nodeType: " . $node->nodeType . "\n";
16-
1716
/* Node gets destroyed during removeChild */
1817
$dom->loadXML('<root><child/></root>');
1918
$xpath = new DOMXpath($dom);
2019
$node = $xpath->query('//child')->item(0);
2120
echo $node->nodeName . "\n";
2221
$GLOBALS['dom']->removeChild($GLOBALS['dom']->firstChild);
2322

24-
echo "nodeType: " . $node->nodeType . "\n";
23+
try {
24+
echo "nodeType: " . $node->nodeType . "\n";
25+
} catch (\Error $e) {
26+
echo get_class($e) . ': ' . $e->getMessage() .\PHP_EOL;
27+
}
2528

2629
?>
27-
--EXPECTF--
30+
--EXPECT--
2831
root
2932
nodeType: 1
3033
child
31-
32-
Warning: Couldn't fetch DOMElement. Node no longer exists in %sbug36756.php on line %d
33-
34-
Warning: Undefined property: DOMElement::$nodeType in %s on line %d
35-
nodeType:
34+
Error: Couldn't fetch DOMElement. Node no longer exists

ext/dom/tests/bug77569.phpt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@ if (!extension_loaded('dom')) die('skip dom extension not available');
88
<?php
99
$imp = new DOMImplementation;
1010
$dom = $imp->createDocument("", "");
11-
$dom->encoding = null;
11+
try {
12+
$dom->encoding = null;
13+
} catch (\ValueError $e) {
14+
echo $e->getMessage() . \PHP_EOL;
15+
}
1216
?>
13-
--EXPECTF--
14-
Warning: main(): Invalid Document Encoding in %s on line %d
17+
--EXPECT--
18+
Invalid Document Encoding

ext/dom/xpath.c

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
136136

137137
obj = valuePop(ctxt);
138138
if (obj->stringval == NULL) {
139-
// Todo convert to type error?
140-
php_error_docref(NULL, E_WARNING, "Handler name must be a string");
139+
zend_type_error("Handler name must be a string");
141140
xmlXPathFreeObject(obj);
142141
if (fci.param_count > 0) {
143142
for (i = 0; i < nargs - 1; i++) {
@@ -155,13 +154,11 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
155154
fci.retval = &retval;
156155

157156
if (!zend_make_callable(&fci.function_name, &callable)) {
158-
// TODO make this an error?
159-
php_error_docref(NULL, E_WARNING, "Unable to call handler %s()", ZSTR_VAL(callable));
157+
zend_throw_error(NULL, "Unable to call handler %s()", ZSTR_VAL(callable));
158+
return;
160159
} else if (intern->registerPhpFunctions == 2 && zend_hash_exists(intern->registered_phpfunctions, callable) == 0) {
161-
// TODO make this an error?
162-
php_error_docref(NULL, E_WARNING, "Not allowed to call handler '%s()'.", ZSTR_VAL(callable));
163-
/* Push an empty string, so that we at least have an xslt result... */
164-
valuePush(ctxt, xmlXPathNewString((xmlChar *)""));
160+
zend_throw_error(NULL, "Not allowed to call handler '%s()'.", ZSTR_VAL(callable));
161+
return;
165162
} else {
166163
result = zend_call_function(&fci, NULL);
167164
if (result == SUCCESS && Z_TYPE(retval) != IS_UNDEF) {
@@ -179,9 +176,8 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs,
179176
} else if (Z_TYPE(retval) == IS_FALSE || Z_TYPE(retval) == IS_TRUE) {
180177
valuePush(ctxt, xmlXPathNewBoolean(Z_TYPE(retval) == IS_TRUE));
181178
} else if (Z_TYPE(retval) == IS_OBJECT) {
182-
// TODO make this an error?
183-
php_error_docref(NULL, E_WARNING, "A PHP Object cannot be converted to a XPath-string");
184-
valuePush(ctxt, xmlXPathNewString((xmlChar *)""));
179+
zend_type_error("A PHP Object cannot be converted to a XPath-string");
180+
return;
185181
} else {
186182
zend_string *str = zval_get_string(&retval);
187183
valuePush(ctxt, xmlXPathNewString((xmlChar *) ZSTR_VAL(str)));
@@ -312,9 +308,8 @@ PHP_METHOD(DOMXPath, registerNamespace)
312308

313309
ctxp = (xmlXPathContextPtr) intern->dom.ptr;
314310
if (ctxp == NULL) {
315-
// Todo convert to error?
316-
php_error_docref(NULL, E_WARNING, "Invalid XPath Context");
317-
RETURN_FALSE;
311+
zend_throw_error(NULL, "Invalid XPath Context");
312+
RETURN_THROWS();
318313
}
319314

320315
if (xmlXPathRegisterNs(ctxp, prefix, ns_uri) != 0) {
@@ -357,9 +352,8 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
357352

358353
ctxp = (xmlXPathContextPtr) intern->dom.ptr;
359354
if (ctxp == NULL) {
360-
// Todo convert to error?
361-
php_error_docref(NULL, E_WARNING, "Invalid XPath Context");
362-
RETURN_FALSE;
355+
zend_throw_error(NULL, "Invalid XPath Context");
356+
RETURN_THROWS();
363357
}
364358

365359
docp = (xmlDocPtr) ctxp->doc;
@@ -378,9 +372,8 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */
378372
}
379373

380374
if (nodep && docp != nodep->doc) {
381-
// Todo convert to error?
382-
php_error_docref(NULL, E_WARNING, "Node From Wrong Document");
383-
RETURN_FALSE;
375+
zend_throw_error(NULL, "Node From Wrong Document");
376+
RETURN_THROWS();
384377
}
385378

386379
ctxp->node = nodep;

0 commit comments

Comments
 (0)