Skip to content

Commit cc2dbba

Browse files
GirgiasGirgias
committed
Promote warnings to error in DOM ext
1 parent 0444158 commit cc2dbba

16 files changed

+95
-60
lines changed

‎ext/dom/attr.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,14 @@ PHP_METHOD(DOMAttr, __construct)
5050
name_valid = xmlValidateName((xmlChar *) name, 0);
5151
if (name_valid != 0) {
5252
php_dom_throw_error(INVALID_CHARACTER_ERR, 1);
53-
RETURN_FALSE;
53+
RETURN_THROWS();
5454
}
5555

5656
nodep = xmlNewProp(NULL, (xmlChar *) name, (xmlChar *) value);
5757

5858
if (!nodep) {
5959
php_dom_throw_error(INVALID_STATE_ERR, 1);
60-
RETURN_FALSE;
60+
RETURN_THROWS();
6161
}
6262

6363
oldnode = dom_object_get_node(intern);

‎ext/dom/cdatasection.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ PHP_METHOD(DOMCdataSection, __construct)
4646

4747
if (!nodep) {
4848
php_dom_throw_error(INVALID_STATE_ERR, 1);
49-
return;
49+
RETURN_THROWS();
5050
}
5151

5252
intern = Z_DOMOBJ_P(ZEND_THIS);

‎ext/dom/comment.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ PHP_METHOD(DOMComment, __construct)
4646

4747
if (!nodep) {
4848
php_dom_throw_error(INVALID_STATE_ERR, 1);
49-
return;
49+
RETURN_THROWS();
5050
}
5151

5252
intern = Z_DOMOBJ_P(ZEND_THIS);

‎ext/dom/document.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,7 @@ 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?
164165
php_error_docref(NULL, E_WARNING, "Invalid Document Encoding");
165166
}
166167

‎ext/dom/documentfragment.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ PHP_METHOD(DOMDocumentFragment, __construct)
4444

4545
if (!nodep) {
4646
php_dom_throw_error(INVALID_STATE_ERR, 1);
47-
return;
47+
RETURN_THROWS();
4848
}
4949

5050
intern = Z_DOMOBJ_P(ZEND_THIS);
@@ -109,6 +109,7 @@ PHP_METHOD(DOMDocumentFragment, appendXML) {
109109
DOM_GET_OBJ(nodep, id, xmlNodePtr, intern);
110110

111111
if (dom_node_is_read_only(nodep) == SUCCESS) {
112+
// Should always be in strict mode?
112113
php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, dom_get_strict_error(intern->document));
113114
RETURN_FALSE;
114115
}

‎ext/dom/domimplementation.c

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,8 @@ PHP_METHOD(DOMImplementation, createDocumentType)
6767
}
6868

6969
if (name_len == 0) {
70-
php_error_docref(NULL, E_WARNING, "qualifiedName is required");
71-
RETURN_FALSE;
70+
zend_argument_value_error(1, "cannot be empty");
71+
RETURN_THROWS();
7272
}
7373

7474
if (publicid_len > 0) {
@@ -134,12 +134,12 @@ PHP_METHOD(DOMImplementation, createDocument)
134134
if (node != NULL) {
135135
DOM_GET_OBJ(doctype, node, xmlDtdPtr, doctobj);
136136
if (doctype->type == XML_DOCUMENT_TYPE_NODE) {
137-
php_error_docref(NULL, E_WARNING, "Invalid DocumentType object");
138-
RETURN_FALSE;
137+
zend_argument_value_error(3, "is an invalid DocumentType object");
138+
RETURN_THROWS();
139139
}
140140
if (doctype->doc != NULL) {
141141
php_dom_throw_error(WRONG_DOCUMENT_ERR, 1);
142-
RETURN_FALSE;
142+
RETURN_THROWS();
143143
}
144144
} else {
145145
doctobj = NULL;
@@ -163,7 +163,7 @@ PHP_METHOD(DOMImplementation, createDocument)
163163
xmlFree(localname);
164164
}
165165
php_dom_throw_error(errorcode, 1);
166-
RETURN_FALSE;
166+
RETURN_THROWS();
167167
}
168168

169169
/* currently letting libxml2 set the version string */
@@ -195,9 +195,9 @@ PHP_METHOD(DOMImplementation, createDocument)
195195
}
196196
xmlFreeDoc(docp);
197197
xmlFree(localname);
198-
/* Need some type of error here */
199-
php_error_docref(NULL, E_WARNING, "Unexpected Error");
200-
RETURN_FALSE;
198+
/* Need some better type of error here */
199+
php_dom_throw_error(PHP_ERR, 1);
200+
RETURN_THROWS();
201201
}
202202

203203
nodep->nsDef = nsptr;

‎ext/dom/element.c

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ PHP_METHOD(DOMElement, __construct)
4949
name_valid = xmlValidateName((xmlChar *) name, 0);
5050
if (name_valid != 0) {
5151
php_dom_throw_error(INVALID_CHARACTER_ERR, 1);
52-
return;
52+
RETURN_THROWS();
5353
}
5454

5555
/* Namespace logic is separate and only when uri passed in to insure no BC breakage */
@@ -71,7 +71,7 @@ PHP_METHOD(DOMElement, __construct)
7171
xmlFreeNode(nodep);
7272
}
7373
php_dom_throw_error(errorcode, 1);
74-
return;
74+
RETURN_THROWS();
7575
}
7676
} else {
7777
/* If you don't pass a namespace uri, then you can't set a prefix */
@@ -80,14 +80,14 @@ PHP_METHOD(DOMElement, __construct)
8080
xmlFree(localname);
8181
xmlFree(prefix);
8282
php_dom_throw_error(NAMESPACE_ERR, 1);
83-
return;
83+
RETURN_THROWS();
8484
}
8585
nodep = xmlNewNode(NULL, (xmlChar *) name);
8686
}
8787

8888
if (!nodep) {
8989
php_dom_throw_error(INVALID_STATE_ERR, 1);
90-
return;
90+
RETURN_THROWS();
9191
}
9292

9393
if (value_len > 0) {
@@ -255,19 +255,20 @@ PHP_METHOD(DOMElement, setAttribute)
255255
}
256256

257257
if (name_len == 0) {
258-
php_error_docref(NULL, E_WARNING, "Attribute Name is required");
259-
RETURN_FALSE;
258+
zend_argument_value_error(1, "cannot be empty");
259+
RETURN_THROWS();
260260
}
261261

262262
name_valid = xmlValidateName((xmlChar *) name, 0);
263263
if (name_valid != 0) {
264264
php_dom_throw_error(INVALID_CHARACTER_ERR, 1);
265-
RETURN_FALSE;
265+
RETURN_THROWS();
266266
}
267267

268268
DOM_GET_OBJ(nodep, id, xmlNodePtr, intern);
269269

270270
if (dom_node_is_read_only(nodep) == SUCCESS) {
271+
// Todo make it always strict?
271272
php_dom_throw_error(NO_MODIFICATION_ALLOWED_ERR, dom_get_strict_error(intern->document));
272273
RETURN_FALSE;
273274
}
@@ -294,6 +295,7 @@ PHP_METHOD(DOMElement, setAttribute)
294295
attr = (xmlNodePtr)xmlSetProp(nodep, (xmlChar *) name, (xmlChar *)value);
295296
}
296297
if (!attr) {
298+
// TODO Convert to error?
297299
php_error_docref(NULL, E_WARNING, "No such attribute '%s'", name);
298300
RETURN_FALSE;
299301
}
@@ -424,6 +426,7 @@ PHP_METHOD(DOMElement, setAttributeNode)
424426
DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
425427

426428
if (attrp->type != XML_ATTRIBUTE_NODE) {
429+
// Todo convert to a ValueError?
427430
php_error_docref(NULL, E_WARNING, "Attribute node is required");
428431
RETURN_FALSE;
429432
}
@@ -628,8 +631,8 @@ PHP_METHOD(DOMElement, setAttributeNS)
628631
}
629632

630633
if (name_len == 0) {
631-
php_error_docref(NULL, E_WARNING, "Attribute Name is required");
632-
RETURN_FALSE;
634+
zend_argument_value_error(2, "cannot be empty");
635+
RETURN_THROWS();
633636
}
634637

635638
DOM_GET_OBJ(elemp, id, xmlNodePtr, intern);
@@ -641,6 +644,7 @@ PHP_METHOD(DOMElement, setAttributeNS)
641644
RETURN_NULL();
642645
}
643646

647+
// Todo should always be strict?
644648
errorcode = dom_check_qname(name, &localname, &prefix, uri_len, name_len);
645649

646650
if (errorcode == 0) {
@@ -874,6 +878,7 @@ PHP_METHOD(DOMElement, setAttributeNodeNS)
874878
DOM_GET_OBJ(attrp, node, xmlAttrPtr, attrobj);
875879

876880
if (attrp->type != XML_ATTRIBUTE_NODE) {
881+
// Todo convert to error?
877882
php_error_docref(NULL, E_WARNING, "Attribute node is required");
878883
RETURN_FALSE;
879884
}

‎ext/dom/entityreference.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,14 +46,14 @@ PHP_METHOD(DOMEntityReference, __construct)
4646
name_valid = xmlValidateName((xmlChar *) name, 0);
4747
if (name_valid != 0) {
4848
php_dom_throw_error(INVALID_CHARACTER_ERR, 1);
49-
RETURN_FALSE;
49+
RETURN_THROWS();
5050
}
5151

5252
node = xmlNewReference(NULL, (xmlChar *) name);
5353

5454
if (!node) {
5555
php_dom_throw_error(INVALID_STATE_ERR, 1);
56-
RETURN_FALSE;
56+
RETURN_THROWS();
5757
}
5858

5959
intern = Z_DOMOBJ_P(ZEND_THIS);

‎ext/dom/namednodemap.c

Lines changed: 28 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -146,44 +146,42 @@ PHP_METHOD(DOMNamedNodeMap, item)
146146
if (zend_parse_parameters(ZEND_NUM_ARGS(), "l", &index) == FAILURE) {
147147
RETURN_THROWS();
148148
}
149-
if (index >= 0) {
150-
if (ZEND_LONG_INT_OVFL(index)) {
151-
php_error_docref(NULL, E_WARNING, "Invalid index");
152-
RETURN_NULL();
153-
}
149+
if (index < 0 || ZEND_LONG_INT_OVFL(index)) {
150+
zend_argument_value_error(1, "must be between 0 and %d", INT_MAX);
151+
RETURN_THROWS();
152+
}
154153

155-
intern = Z_DOMOBJ_P(id);
154+
intern = Z_DOMOBJ_P(id);
156155

157-
objmap = (dom_nnodemap_object *)intern->ptr;
156+
objmap = (dom_nnodemap_object *)intern->ptr;
158157

159-
if (objmap != NULL) {
160-
if ((objmap->nodetype == XML_NOTATION_NODE) ||
161-
objmap->nodetype == XML_ENTITY_NODE) {
162-
if (objmap->ht) {
163-
if (objmap->nodetype == XML_ENTITY_NODE) {
164-
itemnode = php_dom_libxml_hash_iter(objmap->ht, index);
165-
} else {
166-
itemnode = php_dom_libxml_notation_iter(objmap->ht, index);
167-
}
158+
if (objmap != NULL) {
159+
if ((objmap->nodetype == XML_NOTATION_NODE) ||
160+
objmap->nodetype == XML_ENTITY_NODE) {
161+
if (objmap->ht) {
162+
if (objmap->nodetype == XML_ENTITY_NODE) {
163+
itemnode = php_dom_libxml_hash_iter(objmap->ht, index);
164+
} else {
165+
itemnode = php_dom_libxml_notation_iter(objmap->ht, index);
168166
}
169-
} else {
170-
nodep = dom_object_get_node(objmap->baseobj);
171-
if (nodep) {
172-
curnode = (xmlNodePtr)nodep->properties;
173-
count = 0;
174-
while (count < index && curnode != NULL) {
175-
count++;
176-
curnode = (xmlNodePtr)curnode->next;
177-
}
178-
itemnode = curnode;
167+
}
168+
} else {
169+
nodep = dom_object_get_node(objmap->baseobj);
170+
if (nodep) {
171+
curnode = (xmlNodePtr)nodep->properties;
172+
count = 0;
173+
while (count < index && curnode != NULL) {
174+
count++;
175+
curnode = (xmlNodePtr)curnode->next;
179176
}
177+
itemnode = curnode;
180178
}
181179
}
180+
}
182181

183-
if (itemnode) {
184-
DOM_RET_OBJ(itemnode, &ret, objmap->baseobj);
185-
return;
186-
}
182+
if (itemnode) {
183+
DOM_RET_OBJ(itemnode, &ret, objmap->baseobj);
184+
return;
187185
}
188186

189187
RETVAL_NULL();

‎ext/dom/node.c

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ int dom_node_node_name_read(dom_object *obj, zval *retval)
9898
str = "#text";
9999
break;
100100
default:
101+
// Todo convert to error?
101102
php_error_docref(NULL, E_WARNING, "Invalid Node Type");
102103
}
103104

@@ -856,6 +857,7 @@ PHP_METHOD(DOMNode, insertBefore)
856857

857858
new_child = NULL;
858859

860+
// Todo should always be strict?
859861
stricterror = dom_get_strict_error(intern->document);
860862

861863
if (dom_node_is_read_only(parentp) == SUCCESS ||
@@ -875,6 +877,7 @@ PHP_METHOD(DOMNode, insertBefore)
875877
}
876878

877879
if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
880+
// Todo convert to Error?
878881
php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
879882
RETURN_FALSE;
880883
}
@@ -981,6 +984,7 @@ PHP_METHOD(DOMNode, insertBefore)
981984
}
982985

983986
if (NULL == new_child) {
987+
// Todo convert to error?
984988
php_error_docref(NULL, E_WARNING, "Couldn't add newnode as the previous sibling of refnode");
985989
RETURN_FALSE;
986990
}
@@ -1023,6 +1027,7 @@ PHP_METHOD(DOMNode, replaceChild)
10231027
RETURN_FALSE;
10241028
}
10251029

1030+
// Todo make alway strict?
10261031
stricterror = dom_get_strict_error(intern->document);
10271032

10281033
if (dom_node_is_read_only(nodep) == SUCCESS ||
@@ -1074,7 +1079,7 @@ PHP_METHOD(DOMNode, replaceChild)
10741079
DOM_RET_OBJ(oldchild, &ret, intern);
10751080
return;
10761081
} else {
1077-
php_dom_throw_error(NOT_FOUND_ERR, dom_get_strict_error(intern->document));
1082+
php_dom_throw_error(NOT_FOUND_ERR, stricterror);
10781083
RETURN_FALSE;
10791084
}
10801085
}
@@ -1173,6 +1178,7 @@ PHP_METHOD(DOMNode, appendChild)
11731178
}
11741179

11751180
if (child->type == XML_DOCUMENT_FRAG_NODE && child->children == NULL) {
1181+
// Todo convert to error?
11761182
php_error_docref(NULL, E_WARNING, "Document Fragment is empty");
11771183
RETURN_FALSE;
11781184
}
@@ -1462,6 +1468,7 @@ PHP_METHOD(DOMNode, lookupPrefix)
14621468
}
14631469
}
14641470

1471+
// Todo return empty string?
14651472
RETURN_NULL();
14661473
}
14671474
/* }}} end dom_node_lookup_prefix */
@@ -1570,6 +1577,7 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
15701577
docp = nodep->doc;
15711578

15721579
if (! docp) {
1580+
// TOdo convert to error?
15731581
php_error_docref(NULL, E_WARNING, "Node must be associated with a document");
15741582
RETURN_FALSE;
15751583
}
@@ -1587,6 +1595,7 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
15871595
xmlXPathFreeObject(xpathobjp);
15881596
}
15891597
xmlXPathFreeContext(ctxp);
1598+
// Todo convert to error?
15901599
php_error_docref(NULL, E_WARNING, "XPath query did not return a nodeset.");
15911600
RETURN_FALSE;
15921601
}
@@ -1601,6 +1610,7 @@ static void dom_canonicalization(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{
16011610
if (tmp && Z_TYPE_P(tmp) == IS_STRING) {
16021611
xquery = Z_STRVAL_P(tmp);
16031612
} else {
1613+
// Todo convert to error?
16041614
php_error_docref(NULL, E_WARNING, "'query' missing from xpath array or is not a string");
16051615
RETURN_FALSE;
16061616
}
@@ -1738,6 +1748,7 @@ PHP_METHOD(DOMNode, getNodePath)
17381748

17391749
value = (char *) xmlGetNodePath(nodep);
17401750
if (value == NULL) {
1751+
// Todo return empty string?
17411752
RETURN_NULL();
17421753
} else {
17431754
RETVAL_STRING(value);

0 commit comments

Comments
 (0)