From 8fef83dd3ceb6105f40ab4b0748a5360f1d6adf2 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Sun, 19 Apr 2020 14:35:25 +0200 Subject: [PATCH] Promote warnings to error in DOM extension Closes GH-5418 --- ext/dom/attr.c | 4 +- ext/dom/cdatasection.c | 2 +- ext/dom/comment.c | 2 +- ext/dom/document.c | 5 +- ext/dom/documentfragment.c | 2 +- ext/dom/domimplementation.c | 18 +++--- ext/dom/element.c | 34 +++++------ ext/dom/entityreference.c | 4 +- ext/dom/namednodemap.c | 58 +++++++++---------- ext/dom/node.c | 40 ++++++++----- ext/dom/php_dom.c | 11 ++-- ext/dom/php_dom.h | 4 +- ext/dom/php_dom.stub.php | 4 +- ext/dom/php_dom_arginfo.h | 2 +- ext/dom/processinginstruction.c | 4 +- .../tests/DOMAttr_ownerElement_error_001.phpt | 13 +++-- ext/dom/tests/DOMDocument_adoptNode.phpt | 10 +++- ext/dom/tests/DOMDocument_encoding_basic.phpt | 18 +++--- ext/dom/tests/DOMNode_C14NFile_basic.phpt | 12 ++++ ext/dom/tests/DOMNode_C14N_basic.phpt | 13 +++++ ext/dom/tests/bug36756.phpt | 15 +++-- ext/dom/tests/bug77569.phpt | 10 +++- ext/dom/tests/domxpath.phpt | 15 +++++ ext/dom/text.c | 13 ++++- ext/dom/xpath.c | 38 ++++++------ 25 files changed, 208 insertions(+), 143 deletions(-) diff --git a/ext/dom/attr.c b/ext/dom/attr.c index 99178bfe3e..e348f8fc5a 100644 --- a/ext/dom/attr.c +++ b/ext/dom/attr.c @@ -50,14 +50,14 @@ PHP_METHOD(DOMAttr, __construct) name_valid = xmlValidateName((xmlChar *) name, 0); if (name_valid != 0) { php_dom_throw_error(INVALID_CHARACTER_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } nodep = xmlNewProp(NULL, (xmlChar *) name, (xmlChar *) value); if (!nodep) { php_dom_throw_error(INVALID_STATE_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } oldnode = dom_object_get_node(intern); diff --git a/ext/dom/cdatasection.c b/ext/dom/cdatasection.c index ce62377917..2d6fd8dc05 100644 --- a/ext/dom/cdatasection.c +++ b/ext/dom/cdatasection.c @@ -46,7 +46,7 @@ PHP_METHOD(DOMCdataSection, __construct) if (!nodep) { php_dom_throw_error(INVALID_STATE_ERR, 1); - return; + RETURN_THROWS(); } intern = Z_DOMOBJ_P(ZEND_THIS); diff --git a/ext/dom/comment.c b/ext/dom/comment.c index db2341b725..79588f0c87 100644 --- a/ext/dom/comment.c +++ b/ext/dom/comment.c @@ -46,7 +46,7 @@ PHP_METHOD(DOMComment, __construct) if (!nodep) { php_dom_throw_error(INVALID_STATE_ERR, 1); - return; + RETURN_THROWS(); } intern = Z_DOMOBJ_P(ZEND_THIS); diff --git a/ext/dom/document.c b/ext/dom/document.c index 6ff1750137..b580842667 100644 --- a/ext/dom/document.c +++ b/ext/dom/document.c @@ -136,7 +136,7 @@ int dom_document_encoding_read(dom_object *obj, zval *retval) return SUCCESS; } -int dom_document_encoding_write(dom_object *obj, zval *newval) +zend_result dom_document_encoding_write(dom_object *obj, zval *newval) { xmlDoc *docp = (xmlDocPtr) dom_object_get_node(obj); zend_string *str; @@ -161,7 +161,8 @@ int dom_document_encoding_write(dom_object *obj, zval *newval) } docp->encoding = xmlStrdup((const xmlChar *) ZSTR_VAL(str)); } else { - php_error_docref(NULL, E_WARNING, "Invalid Document Encoding"); + zend_value_error("Invalid document encoding"); + return FAILURE; } zend_string_release_ex(str, 0); diff --git a/ext/dom/documentfragment.c b/ext/dom/documentfragment.c index 641c3f5b34..051416792d 100644 --- a/ext/dom/documentfragment.c +++ b/ext/dom/documentfragment.c @@ -44,7 +44,7 @@ PHP_METHOD(DOMDocumentFragment, __construct) if (!nodep) { php_dom_throw_error(INVALID_STATE_ERR, 1); - return; + RETURN_THROWS(); } intern = Z_DOMOBJ_P(ZEND_THIS); diff --git a/ext/dom/domimplementation.c b/ext/dom/domimplementation.c index 86ec4a2368..b195af5549 100644 --- a/ext/dom/domimplementation.c +++ b/ext/dom/domimplementation.c @@ -67,8 +67,8 @@ PHP_METHOD(DOMImplementation, createDocumentType) } if (name_len == 0) { - php_error_docref(NULL, E_WARNING, "qualifiedName is required"); - RETURN_FALSE; + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); } if (publicid_len > 0) { @@ -134,12 +134,12 @@ PHP_METHOD(DOMImplementation, createDocument) if (node != NULL) { DOM_GET_OBJ(doctype, node, xmlDtdPtr, doctobj); if (doctype->type == XML_DOCUMENT_TYPE_NODE) { - php_error_docref(NULL, E_WARNING, "Invalid DocumentType object"); - RETURN_FALSE; + zend_argument_value_error(3, "is an invalid DocumentType object"); + RETURN_THROWS(); } if (doctype->doc != NULL) { php_dom_throw_error(WRONG_DOCUMENT_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } } else { doctobj = NULL; @@ -163,7 +163,7 @@ PHP_METHOD(DOMImplementation, createDocument) xmlFree(localname); } php_dom_throw_error(errorcode, 1); - RETURN_FALSE; + RETURN_THROWS(); } /* currently letting libxml2 set the version string */ @@ -195,9 +195,9 @@ PHP_METHOD(DOMImplementation, createDocument) } xmlFreeDoc(docp); xmlFree(localname); - /* Need some type of error here */ - php_error_docref(NULL, E_WARNING, "Unexpected Error"); - RETURN_FALSE; + /* Need some better type of error here */ + php_dom_throw_error(PHP_ERR, 1); + RETURN_THROWS(); } nodep->nsDef = nsptr; diff --git a/ext/dom/element.c b/ext/dom/element.c index 41bc7296ca..317619001b 100644 --- a/ext/dom/element.c +++ b/ext/dom/element.c @@ -49,7 +49,7 @@ PHP_METHOD(DOMElement, __construct) name_valid = xmlValidateName((xmlChar *) name, 0); if (name_valid != 0) { php_dom_throw_error(INVALID_CHARACTER_ERR, 1); - return; + RETURN_THROWS(); } /* Namespace logic is separate and only when uri passed in to insure no BC breakage */ @@ -71,7 +71,7 @@ PHP_METHOD(DOMElement, __construct) xmlFreeNode(nodep); } php_dom_throw_error(errorcode, 1); - return; + RETURN_THROWS(); } } else { /* If you don't pass a namespace uri, then you can't set a prefix */ @@ -80,14 +80,14 @@ PHP_METHOD(DOMElement, __construct) xmlFree(localname); xmlFree(prefix); php_dom_throw_error(NAMESPACE_ERR, 1); - return; + RETURN_THROWS(); } nodep = xmlNewNode(NULL, (xmlChar *) name); } if (!nodep) { php_dom_throw_error(INVALID_STATE_ERR, 1); - return; + RETURN_THROWS(); } if (value_len > 0) { @@ -255,14 +255,14 @@ PHP_METHOD(DOMElement, setAttribute) } if (name_len == 0) { - php_error_docref(NULL, E_WARNING, "Attribute Name is required"); - RETURN_FALSE; + zend_argument_value_error(1, "cannot be empty"); + RETURN_THROWS(); } name_valid = xmlValidateName((xmlChar *) name, 0); if (name_valid != 0) { php_dom_throw_error(INVALID_CHARACTER_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } DOM_GET_OBJ(nodep, id, xmlNodePtr, intern); @@ -294,8 +294,8 @@ PHP_METHOD(DOMElement, setAttribute) attr = (xmlNodePtr)xmlSetProp(nodep, (xmlChar *) name, (xmlChar *)value); } if (!attr) { - php_error_docref(NULL, E_WARNING, "No such attribute '%s'", name); - RETURN_FALSE; + zend_argument_value_error(1, "must be a valid XML attribute"); + RETURN_THROWS(); } DOM_RET_OBJ(attr, &ret, intern); @@ -424,8 +424,8 @@ PHP_METHOD(DOMElement, setAttributeNode) DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj); if (attrp->type != XML_ATTRIBUTE_NODE) { - php_error_docref(NULL, E_WARNING, "Attribute node is required"); - RETURN_FALSE; + zend_argument_value_error(1, "must have the node attribute"); + RETURN_THROWS(); } if (!(attrp->doc == NULL || attrp->doc == nodep->doc)) { @@ -628,8 +628,8 @@ PHP_METHOD(DOMElement, setAttributeNS) } if (name_len == 0) { - php_error_docref(NULL, E_WARNING, "Attribute Name is required"); - RETURN_FALSE; + zend_argument_value_error(2, "cannot be empty"); + RETURN_THROWS(); } DOM_GET_OBJ(elemp, id, xmlNodePtr, intern); @@ -873,10 +873,10 @@ PHP_METHOD(DOMElement, setAttributeNodeNS) DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj); - if (attrp->type != XML_ATTRIBUTE_NODE) { - php_error_docref(NULL, E_WARNING, "Attribute node is required"); - RETURN_FALSE; - } + /* ZPP Guarantees that a DOMAttr class is given, as it is converted to a xmlAttr + * to pass to libxml (see http://www.xmlsoft.org/html/libxml-tree.html#xmlAttr) + * if it is not of type XML_ATTRIBUTE_NODE it indicates a bug somewhere */ + ZEND_ASSERT(attrp->type == XML_ATTRIBUTE_NODE); if (!(attrp->doc == NULL || attrp->doc == nodep->doc)) { php_dom_throw_error(WRONG_DOCUMENT_ERR, dom_get_strict_error(intern->document)); diff --git a/ext/dom/entityreference.c b/ext/dom/entityreference.c index 4e45119b48..635cfe7653 100644 --- a/ext/dom/entityreference.c +++ b/ext/dom/entityreference.c @@ -46,14 +46,14 @@ PHP_METHOD(DOMEntityReference, __construct) name_valid = xmlValidateName((xmlChar *) name, 0); if (name_valid != 0) { php_dom_throw_error(INVALID_CHARACTER_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } node = xmlNewReference(NULL, (xmlChar *) name); if (!node) { php_dom_throw_error(INVALID_STATE_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } intern = Z_DOMOBJ_P(ZEND_THIS); diff --git a/ext/dom/namednodemap.c b/ext/dom/namednodemap.c index fd96003e56..26e0551f3b 100644 --- a/ext/dom/namednodemap.c +++ b/ext/dom/namednodemap.c @@ -146,44 +146,42 @@ PHP_METHOD(DOMNamedNodeMap, item) if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) { RETURN_THROWS(); } - if (index >= 0) { - if (ZEND_LONG_INT_OVFL(index)) { - php_error_docref(NULL, E_WARNING, "Invalid index"); - RETURN_NULL(); - } + if (index < 0 || ZEND_LONG_INT_OVFL(index)) { + zend_argument_value_error(1, "must be between 0 and %d", INT_MAX); + RETURN_THROWS(); + } - intern = Z_DOMOBJ_P(id); + intern = Z_DOMOBJ_P(id); - objmap = (dom_nnodemap_object *)intern->ptr; + objmap = (dom_nnodemap_object *)intern->ptr; - if (objmap != NULL) { - if ((objmap->nodetype == XML_NOTATION_NODE) || - objmap->nodetype == XML_ENTITY_NODE) { - if (objmap->ht) { - if (objmap->nodetype == XML_ENTITY_NODE) { - itemnode = php_dom_libxml_hash_iter(objmap->ht, index); - } else { - itemnode = php_dom_libxml_notation_iter(objmap->ht, index); - } + if (objmap != NULL) { + if ((objmap->nodetype == XML_NOTATION_NODE) || + objmap->nodetype == XML_ENTITY_NODE) { + if (objmap->ht) { + if (objmap->nodetype == XML_ENTITY_NODE) { + itemnode = php_dom_libxml_hash_iter(objmap->ht, index); + } else { + itemnode = php_dom_libxml_notation_iter(objmap->ht, index); } - } else { - nodep = dom_object_get_node(objmap->baseobj); - if (nodep) { - curnode = (xmlNodePtr)nodep->properties; - count = 0; - while (count < index && curnode != NULL) { - count++; - curnode = (xmlNodePtr)curnode->next; - } - itemnode = curnode; + } + } else { + nodep = dom_object_get_node(objmap->baseobj); + if (nodep) { + curnode = (xmlNodePtr)nodep->properties; + count = 0; + while (count < index && curnode != NULL) { + count++; + curnode = (xmlNodePtr)curnode->next; } + itemnode = curnode; } } + } - if (itemnode) { - DOM_RET_OBJ(itemnode, &ret, objmap->baseobj); - return; - } + if (itemnode) { + DOM_RET_OBJ(itemnode, &ret, objmap->baseobj); + return; } RETVAL_NULL(); diff --git a/ext/dom/node.c b/ext/dom/node.c index 16a6c6cc54..a6f88b5c0f 100644 --- a/ext/dom/node.c +++ b/ext/dom/node.c @@ -97,8 +97,7 @@ int dom_node_node_name_read(dom_object *obj, zval *retval) case XML_TEXT_NODE: str = "#text"; break; - default: - php_error_docref(NULL, E_WARNING, "Invalid Node Type"); + EMPTY_SWITCH_DEFAULT_CASE(); } if (str != NULL) { @@ -875,6 +874,7 @@ PHP_METHOD(DOMNode, insertBefore) } if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) { + /* TODO Drop Warning? */ php_error_docref(NULL, E_WARNING, "Document Fragment is empty"); RETURN_FALSE; } @@ -981,8 +981,8 @@ PHP_METHOD(DOMNode, insertBefore) } if (NULL == new_child) { - php_error_docref(NULL, E_WARNING, "Couldn't add newnode as the previous sibling of refnode"); - RETURN_FALSE; + zend_throw_error(NULL, "Cannot add newnode as the previous sibling of refnode"); + RETURN_THROWS(); } dom_reconcile_ns(parentp->doc, new_child); @@ -1074,7 +1074,7 @@ PHP_METHOD(DOMNode, replaceChild) DOM_RET_OBJ(oldchild, &ret, intern); return; } else { - php_dom_throw_error(NOT_FOUND_ERR, dom_get_strict_error(intern->document)); + php_dom_throw_error(NOT_FOUND_ERR, stricterror); RETURN_FALSE; } } @@ -1173,6 +1173,7 @@ PHP_METHOD(DOMNode, appendChild) } if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) { + /* TODO Drop Warning? */ php_error_docref(NULL, E_WARNING, "Document Fragment is empty"); RETURN_FALSE; } @@ -1221,6 +1222,7 @@ PHP_METHOD(DOMNode, appendChild) if (new_child == NULL) { new_child = xmlAddChild(nodep, child); if (new_child == NULL) { + // TODO Convert to Error? php_error_docref(NULL, E_WARNING, "Couldn't append node"); RETURN_FALSE; } @@ -1570,8 +1572,8 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{ docp = nodep->doc; if (! docp) { - php_error_docref(NULL, E_WARNING, "Node must be associated with a document"); - RETURN_FALSE; + zend_throw_error(NULL, "Node must be associated with a document"); + RETURN_THROWS(); } if (xpath_array == NULL) { @@ -1587,8 +1589,8 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{ xmlXPathFreeObject(xpathobjp); } xmlXPathFreeContext(ctxp); - php_error_docref(NULL, E_WARNING, "XPath query did not return a nodeset."); - RETURN_FALSE; + zend_throw_error(NULL, "XPath query did not return a nodeset"); + RETURN_THROWS(); } } } else { @@ -1598,12 +1600,17 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{ char *xquery; tmp = zend_hash_str_find(ht, "query", sizeof("query")-1); - if (tmp && Z_TYPE_P(tmp) == IS_STRING) { - xquery = Z_STRVAL_P(tmp); - } else { - php_error_docref(NULL, E_WARNING, "'query' missing from xpath array or is not a string"); - RETURN_FALSE; + if (!tmp) { + /* if mode == 0 then $xpath arg is 3, if mode == 1 then $xpath is 4 */ + zend_argument_value_error(3 + mode, "must have a \"query\" key"); + RETURN_THROWS(); + } + if (Z_TYPE_P(tmp) != IS_STRING) { + /* if mode == 0 then $xpath arg is 3, if mode == 1 then $xpath is 4 */ + zend_argument_type_error(3 + mode, "\"query\" option must be a string, %s given", zend_zval_type_name(tmp)); + RETURN_THROWS(); } + xquery = Z_STRVAL_P(tmp); ctxp = xmlXPathNewContext(docp); ctxp->node = nodep; @@ -1631,8 +1638,8 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{ xmlXPathFreeObject(xpathobjp); } xmlXPathFreeContext(ctxp); - php_error_docref(NULL, E_WARNING, "XPath query did not return a nodeset."); - RETURN_FALSE; + zend_throw_error(NULL, "XPath query did not return a nodeset"); + RETURN_THROWS(); } } @@ -1738,6 +1745,7 @@ PHP_METHOD(DOMNode, getNodePath) value = (char *) xmlGetNodePath(nodep); if (value == NULL) { + /* TODO Research if can return empty string */ RETURN_NULL(); } else { RETVAL_STRING(value); diff --git a/ext/dom/php_dom.c b/ext/dom/php_dom.c index 8eae00fc03..78d22f6a5e 100644 --- a/ext/dom/php_dom.c +++ b/ext/dom/php_dom.c @@ -317,7 +317,9 @@ zval *dom_read_property(zend_object *object, zend_string *name, int type, void * if (obj->prop_handler != NULL) { hnd = zend_hash_find_ptr(obj->prop_handler, name); } else if (instanceof_function(obj->std.ce, dom_node_class_entry)) { - php_error(E_WARNING, "Couldn't fetch %s. Node no longer exists", ZSTR_VAL(obj->std.ce->name)); + zend_throw_error(NULL, "Couldn't fetch %s. Node no longer exists", ZSTR_VAL(obj->std.ce->name)); + retval = &EG(uninitialized_zval); + return retval; } if (hnd) { @@ -466,8 +468,8 @@ PHP_FUNCTION(dom_import_simplexml) if (nodep && nodeobj && (nodep->type == XML_ELEMENT_NODE || nodep->type == XML_ATTRIBUTE_NODE)) { DOM_RET_OBJ((xmlNodePtr) nodep, &ret, (dom_object *)nodeobj); } else { - php_error_docref(NULL, E_WARNING, "Invalid Nodetype to import"); - RETURN_NULL(); + zend_argument_value_error(1, "is not a valid node type"); + RETURN_THROWS(); } } /* }}} */ @@ -1208,7 +1210,8 @@ PHP_DOM_EXPORT zend_bool php_dom_create_object(xmlNodePtr obj, zval *return_valu break; } default: - php_error_docref(NULL, E_WARNING, "Unsupported node type: %d", obj->type); + /* TODO Convert to a ZEND assertion? */ + zend_throw_error(NULL, "Unsupported node type: %d", obj->type); ZVAL_NULL(return_value); return 0; } diff --git a/ext/dom/php_dom.h b/ext/dom/php_dom.h index 285980b646..24e1ea646a 100644 --- a/ext/dom/php_dom.h +++ b/ext/dom/php_dom.h @@ -151,8 +151,8 @@ entry = zend_register_internal_class_ex(&ce, parent_ce); } #define DOM_NOT_IMPLEMENTED() \ - php_error_docref(NULL, E_WARNING, "Not yet implemented"); \ - return; + zend_throw_error(NULL, "Not yet implemented"); \ + RETURN_THROWS(); #define DOM_NODELIST 0 #define DOM_NAMEDNODEMAP 1 diff --git a/ext/dom/php_dom.stub.php b/ext/dom/php_dom.stub.php index 5f15f99e17..bce79c9177 100644 --- a/ext/dom/php_dom.stub.php +++ b/ext/dom/php_dom.stub.php @@ -201,7 +201,7 @@ class DOMElement implements DOMParentNode, DOMChildNode /** @return bool */ public function removeAttribute(string $qualifiedName) {} - /** @return DOMAttr|false */ + /** @return void */ public function removeAttributeNS(?string $namespace, string $localName) {} /** @return DOMAttr|false */ @@ -210,7 +210,7 @@ class DOMElement implements DOMParentNode, DOMChildNode /** @return DOMAttr|bool */ public function setAttribute(string $qualifiedName, string $value) {} - /** @return bool|null */ + /** @return void */ public function setAttributeNS(?string $namespace, string $qualifiedName, string $value) {} /** @return DOMAttr|null|false */ diff --git a/ext/dom/php_dom_arginfo.h b/ext/dom/php_dom_arginfo.h index d5da40e463..fb8aaf59f2 100644 --- a/ext/dom/php_dom_arginfo.h +++ b/ext/dom/php_dom_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit the .stub.php file instead. - * Stub hash: 8ac9356f9b19b84e98d335bc9d091b022a0f549d */ + * Stub hash: 128108b08807ce0b125fc7b963bf3c5b77e6987a */ ZEND_BEGIN_ARG_WITH_RETURN_OBJ_INFO_EX(arginfo_dom_import_simplexml, 0, 1, DOMElement, 1) ZEND_ARG_TYPE_INFO(0, node, IS_OBJECT, 0) diff --git a/ext/dom/processinginstruction.c b/ext/dom/processinginstruction.c index 9255213e71..73367442eb 100644 --- a/ext/dom/processinginstruction.c +++ b/ext/dom/processinginstruction.c @@ -46,14 +46,14 @@ PHP_METHOD(DOMProcessingInstruction, __construct) name_valid = xmlValidateName((xmlChar *) name, 0); if (name_valid != 0) { php_dom_throw_error(INVALID_CHARACTER_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } nodep = xmlNewPI((xmlChar *) name, (xmlChar *) value); if (!nodep) { php_dom_throw_error(INVALID_STATE_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } intern = Z_DOMOBJ_P(ZEND_THIS); diff --git a/ext/dom/tests/DOMAttr_ownerElement_error_001.phpt b/ext/dom/tests/DOMAttr_ownerElement_error_001.phpt index e57c3da913..67a66ae16e 100644 --- a/ext/dom/tests/DOMAttr_ownerElement_error_001.phpt +++ b/ext/dom/tests/DOMAttr_ownerElement_error_001.phpt @@ -14,10 +14,11 @@ $document->appendChild($root); $attr = $root->setAttribute('category', 'books'); $document->removeChild($root); $root = null; -var_dump($attr->ownerElement); +try { + var_dump($attr->ownerElement); +} catch (\Error $e) { + echo get_class($e) . ': ' . $e->getMessage() . \PHP_EOL; +} ?> ---EXPECTF-- -Warning: Couldn't fetch DOMAttr. Node no longer exists in %s on line %d - -Warning: Undefined property: DOMAttr::$ownerElement in %s on line %d -NULL +--EXPECT-- +Error: Couldn't fetch DOMAttr. Node no longer exists diff --git a/ext/dom/tests/DOMDocument_adoptNode.phpt b/ext/dom/tests/DOMDocument_adoptNode.phpt index 5fc8e79d9d..0f0a32ae49 100644 --- a/ext/dom/tests/DOMDocument_adoptNode.phpt +++ b/ext/dom/tests/DOMDocument_adoptNode.phpt @@ -10,7 +10,11 @@ require_once('skipif.inc'); $dom = new DOMDocument(); $dom->loadXML(""); -$dom->adoptNode($dom->documentElement); +try { + $dom->adoptNode($dom->documentElement); +} catch (\Error $e) { + echo $e->getMessage() . \PHP_EOL; +} ?> ---EXPECTF-- -Warning: DOMDocument::adoptNode(): Not yet implemented in %s +--EXPECT-- +Not yet implemented diff --git a/ext/dom/tests/DOMDocument_encoding_basic.phpt b/ext/dom/tests/DOMDocument_encoding_basic.phpt index 6ac405da69..925c84d5f1 100644 --- a/ext/dom/tests/DOMDocument_encoding_basic.phpt +++ b/ext/dom/tests/DOMDocument_encoding_basic.phpt @@ -19,10 +19,14 @@ if( !$dom ) exit; } -echo "Empty Encoding Read: {$dom->encoding}\n"; +echo "Empty Encoding Read: '{$dom->encoding}'\n"; -$ret = $dom->encoding = 'NYPHP DOMinatrix'; -echo "Adding invalid encoding: $ret\n"; +try { + $ret = $dom->encoding = 'NYPHP DOMinatrix'; + echo "Adding invalid encoding: $ret\n"; +} catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; +} $ret = $dom->encoding = 'ISO-8859-1'; echo "Adding ISO-8859-1 encoding: $ret\n"; @@ -38,11 +42,9 @@ echo "UTF-16 Encoding Read: {$dom->encoding}\n"; ?> ---EXPECTF-- -Empty Encoding Read: - -Warning: main(): Invalid Document Encoding in %s on line %d -Adding invalid encoding: NYPHP DOMinatrix +--EXPECT-- +Empty Encoding Read: '' +Invalid document encoding Adding ISO-8859-1 encoding: ISO-8859-1 ISO-8859-1 Encoding Read: ISO-8859-1 Adding UTF-8 encoding: UTF-8 diff --git a/ext/dom/tests/DOMNode_C14NFile_basic.phpt b/ext/dom/tests/DOMNode_C14NFile_basic.phpt index b006e47deb..85a8b6f1a5 100644 --- a/ext/dom/tests/DOMNode_C14NFile_basic.phpt +++ b/ext/dom/tests/DOMNode_C14NFile_basic.phpt @@ -27,6 +27,16 @@ $node = $doc->getElementsByTagName('title')->item(0); var_dump($node->C14NFile($output)); $content = file_get_contents($output); var_dump($content); +try { + var_dump($node->C14NFile($output, false, false, [])); +} catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; +} +try { + var_dump($node->C14NFile($output, false, false, ['query' => []])); +} catch (\TypeError $e) { + echo $e->getMessage() . \PHP_EOL; +} ?> --CLEAN-- The Grapes of Wrath" +DOMNode::C14NFile(): Argument #4 ($xpath) must have a "query" key +DOMNode::C14NFile(): Argument #4 ($xpath) "query" option must be a string, array given diff --git a/ext/dom/tests/DOMNode_C14N_basic.phpt b/ext/dom/tests/DOMNode_C14N_basic.phpt index bbfd7bcefb..79a5366e22 100644 --- a/ext/dom/tests/DOMNode_C14N_basic.phpt +++ b/ext/dom/tests/DOMNode_C14N_basic.phpt @@ -24,6 +24,19 @@ $doc = new DOMDocument(); $doc->loadXML($xml); $node = $doc->getElementsByTagName('title')->item(0); var_dump($node->C14N()); + +try { + var_dump($node->C14N(false, false, [])); +} catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; +} +try { + var_dump($node->C14N(false, false, ['query' => []])); +} catch (\TypeError $e) { + echo $e->getMessage() . \PHP_EOL; +} ?> --EXPECT-- string(34) "The Grapes of Wrath" +DOMNode::C14N(): Argument #3 ($xpath) must have a "query" key +DOMNode::C14N(): Argument #3 ($xpath) "query" option must be a string, array given diff --git a/ext/dom/tests/bug36756.phpt b/ext/dom/tests/bug36756.phpt index ccf9ba5049..bf7aa20276 100644 --- a/ext/dom/tests/bug36756.phpt +++ b/ext/dom/tests/bug36756.phpt @@ -13,7 +13,6 @@ $node = $xpath->query('/root')->item(0); echo $node->nodeName . "\n"; $dom->removeChild($GLOBALS['dom']->firstChild); echo "nodeType: " . $node->nodeType . "\n"; - /* Node gets destroyed during removeChild */ $dom->loadXML(''); $xpath = new DOMXpath($dom); @@ -21,15 +20,15 @@ $node = $xpath->query('//child')->item(0); echo $node->nodeName . "\n"; $GLOBALS['dom']->removeChild($GLOBALS['dom']->firstChild); -echo "nodeType: " . $node->nodeType . "\n"; +try { + echo "nodeType: " . $node->nodeType . "\n"; +} catch (\Error $e) { + echo get_class($e) . ': ' . $e->getMessage() .\PHP_EOL; +} ?> ---EXPECTF-- +--EXPECT-- root nodeType: 1 child - -Warning: Couldn't fetch DOMElement. Node no longer exists in %sbug36756.php on line %d - -Warning: Undefined property: DOMElement::$nodeType in %s on line %d -nodeType: +Error: Couldn't fetch DOMElement. Node no longer exists diff --git a/ext/dom/tests/bug77569.phpt b/ext/dom/tests/bug77569.phpt index 9eef2af65a..4c63e263e4 100644 --- a/ext/dom/tests/bug77569.phpt +++ b/ext/dom/tests/bug77569.phpt @@ -8,7 +8,11 @@ if (!extension_loaded('dom')) die('skip dom extension not available'); createDocument("", ""); -$dom->encoding = null; +try { + $dom->encoding = null; +} catch (\ValueError $e) { + echo $e->getMessage() . \PHP_EOL; +} ?> ---EXPECTF-- -Warning: main(): Invalid Document Encoding in %s on line %d +--EXPECT-- +Invalid document encoding diff --git a/ext/dom/tests/domxpath.phpt b/ext/dom/tests/domxpath.phpt index f67b2584aa..32aaffee01 100644 --- a/ext/dom/tests/domxpath.phpt +++ b/ext/dom/tests/domxpath.phpt @@ -50,9 +50,24 @@ $root->appendChild($dom->createElementNS("urn::default", "testnode", 5)); $avg = $xpath->evaluate('number(php:function("MyAverage", //def:testnode))'); var_dump($avg); + +try { + $xpath->registerPHPFunctions('non_existent'); + $avg = $xpath->evaluate('number(php:function("non_existent", //def:testnode))'); +} catch (\Error $e) { + echo $e->getMessage() . \PHP_EOL; +} +try { + $xpath->registerPhpFunctions(['non_existant']); + $avg = $xpath->evaluate('number(php:function("non_existent", //def:testnode))'); +} catch (\Error $e) { + echo $e->getMessage() . \PHP_EOL; +} ?> --EXPECT-- myval float(1) bool(true) float(4) +Unable to call handler non_existent() +Unable to call handler non_existent() diff --git a/ext/dom/text.c b/ext/dom/text.c index b1053b2039..7a45be2463 100644 --- a/ext/dom/text.c +++ b/ext/dom/text.c @@ -47,7 +47,7 @@ PHP_METHOD(DOMText, __construct) if (!nodep) { php_dom_throw_error(INVALID_STATE_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } intern = Z_DOMOBJ_P(ZEND_THIS); @@ -122,17 +122,25 @@ PHP_METHOD(DOMText, splitText) } DOM_GET_OBJ(node, id, xmlNodePtr, intern); + if (offset < 0) { + zend_argument_value_error(1, "must be greater than or equal to 0"); + RETURN_THROWS(); + } + if (node->type != XML_TEXT_NODE && node->type != XML_CDATA_SECTION_NODE) { + /* TODO Add warning? */ RETURN_FALSE; } cur = xmlNodeGetContent(node); if (cur == NULL) { + /* TODO Add warning? */ RETURN_FALSE; } length = xmlUTF8Strlen(cur); - if (ZEND_LONG_INT_OVFL(offset) || (int)offset > length || offset < 0) { + if (ZEND_LONG_INT_OVFL(offset) || (int)offset > length) { + /* TODO Add warning? */ xmlFree(cur); RETURN_FALSE; } @@ -149,6 +157,7 @@ PHP_METHOD(DOMText, splitText) xmlFree(second); if (nnode == NULL) { + /* TODO Add warning? */ RETURN_FALSE; } diff --git a/ext/dom/xpath.c b/ext/dom/xpath.c index cc3cbf8433..1f97601c42 100644 --- a/ext/dom/xpath.c +++ b/ext/dom/xpath.c @@ -136,15 +136,9 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, obj = valuePop(ctxt); if (obj->stringval == NULL) { - php_error_docref(NULL, E_WARNING, "Handler name must be a string"); + zend_type_error("Handler name must be a string"); xmlXPathFreeObject(obj); - if (fci.param_count > 0) { - for (i = 0; i < nargs - 1; i++) { - zval_ptr_dtor(&fci.params[i]); - } - efree(fci.params); - } - return; + goto cleanup; } ZVAL_STRING(&fci.function_name, (char *) obj->stringval); xmlXPathFreeObject(obj); @@ -154,11 +148,11 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, fci.retval = &retval; if (!zend_make_callable(&fci.function_name, &callable)) { - php_error_docref(NULL, E_WARNING, "Unable to call handler %s()", ZSTR_VAL(callable)); + zend_throw_error(NULL, "Unable to call handler %s()", ZSTR_VAL(callable)); + goto cleanup; } else if (intern->registerPhpFunctions == 2 && zend_hash_exists(intern->registered_phpfunctions, callable) == 0) { - php_error_docref(NULL, E_WARNING, "Not allowed to call handler '%s()'.", ZSTR_VAL(callable)); - /* Push an empty string, so that we at least have an xslt result... */ - valuePush(ctxt, xmlXPathNewString((xmlChar *)"")); + zend_throw_error(NULL, "Not allowed to call handler '%s()'.", ZSTR_VAL(callable)); + goto cleanup; } else { result = zend_call_function(&fci, NULL); if (result == SUCCESS && Z_TYPE(retval) != IS_UNDEF) { @@ -176,8 +170,8 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, } else if (Z_TYPE(retval) == IS_FALSE || Z_TYPE(retval) == IS_TRUE) { valuePush(ctxt, xmlXPathNewBoolean(Z_TYPE(retval) == IS_TRUE)); } else if (Z_TYPE(retval) == IS_OBJECT) { - php_error_docref(NULL, E_WARNING, "A PHP Object cannot be converted to a XPath-string"); - valuePush(ctxt, xmlXPathNewString((xmlChar *)"")); + zend_type_error("A PHP Object cannot be converted to a XPath-string"); + return; } else { zend_string *str = zval_get_string(&retval); valuePush(ctxt, xmlXPathNewString((xmlChar *) ZSTR_VAL(str))); @@ -186,6 +180,7 @@ static void dom_xpath_ext_function_php(xmlXPathParserContextPtr ctxt, int nargs, zval_ptr_dtor(&retval); } } +cleanup: zend_string_release_ex(callable, 0); zval_ptr_dtor_str(&fci.function_name); if (fci.param_count > 0) { @@ -228,7 +223,7 @@ PHP_METHOD(DOMXPath, __construct) ctx = xmlXPathNewContext(docp); if (ctx == NULL) { php_dom_throw_error(INVALID_STATE_ERR, 1); - RETURN_FALSE; + RETURN_THROWS(); } intern = Z_XPATHOBJ_P(ZEND_THIS); @@ -308,8 +303,8 @@ PHP_METHOD(DOMXPath, registerNamespace) ctxp = (xmlXPathContextPtr) intern->dom.ptr; if (ctxp == NULL) { - php_error_docref(NULL, E_WARNING, "Invalid XPath Context"); - RETURN_FALSE; + zend_throw_error(NULL, "Invalid XPath Context"); + RETURN_THROWS(); } if (xmlXPathRegisterNs(ctxp, prefix, ns_uri) != 0) { @@ -352,8 +347,8 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */ ctxp = (xmlXPathContextPtr) intern->dom.ptr; if (ctxp == NULL) { - php_error_docref(NULL, E_WARNING, "Invalid XPath Context"); - RETURN_FALSE; + zend_throw_error(NULL, "Invalid XPath Context"); + RETURN_THROWS(); } docp = (xmlDocPtr) ctxp->doc; @@ -371,8 +366,8 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */ } if (nodep && docp != nodep->doc) { - php_error_docref(NULL, E_WARNING, "Node From Wrong Document"); - RETURN_FALSE; + zend_throw_error(NULL, "Node from wrong document"); + RETURN_THROWS(); } ctxp->node = nodep; @@ -401,6 +396,7 @@ static void php_xpath_eval(INTERNAL_FUNCTION_PARAMETERS, int type) /* {{{ */ } if (! xpathobjp) { + /* TODO Add Warning? */ RETURN_FALSE; } -- 2.40.0