Skip to content

Commit b3a4a6b

Browse files
nielsdosnielsdos
authored
Resolve TODOs in ext/dom around nullable content (#14999)
It's indeed possible this is NULL. When you create a new text-like node in libxml and pass NULL as content, you do get NULL in the content field instead of the empty string. You can hit this by creating DOMText or DOMComment directly and not passing any argument. This could also be created internally. We refactor the code such that this detail is hidden and we add a test to check that it correctly throws an exception.
1 parent 8d59715 commit b3a4a6b

File tree

4 files changed

+46
-30
lines changed

4 files changed

+46
-30
lines changed

‎ext/dom/characterdata.c

Lines changed: 7 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,6 @@ Modern spec URL: https://dom.spec.whatwg.org/#dom-characterdata-substringdata
104104
PHP_METHOD(DOMCharacterData, substringData)
105105
{
106106
zval *id;
107-
xmlChar *cur;
108107
xmlChar *substring;
109108
xmlNodePtr node;
110109
zend_long offset_input, count_input;
@@ -119,11 +118,7 @@ PHP_METHOD(DOMCharacterData, substringData)
119118

120119
DOM_GET_OBJ(node, id, xmlNodePtr, intern);
121120

122-
cur = node->content;
123-
if (cur == NULL) {
124-
/* TODO: is this even possible? */
125-
cur = BAD_CAST "";
126-
}
121+
const xmlChar *cur = php_dom_get_content_or_empty(node);
127122

128123
length = xmlUTF8Strlen(cur);
129124
if (ZEND_LONG_INT_OVFL(offset_input) || ZEND_LONG_INT_OVFL(count_input)) {
@@ -197,7 +192,7 @@ Modern spec URL: https://dom.spec.whatwg.org/#dom-characterdata-insertdata
197192
static void dom_character_data_insert_data(INTERNAL_FUNCTION_PARAMETERS, bool return_true)
198193
{
199194
zval *id;
200-
xmlChar *cur, *first, *second;
195+
xmlChar *first, *second;
201196
xmlNodePtr node;
202197
char *arg;
203198
zend_long offset_input;
@@ -213,11 +208,7 @@ static void dom_character_data_insert_data(INTERNAL_FUNCTION_PARAMETERS, bool re
213208

214209
DOM_GET_OBJ(node, id, xmlNodePtr, intern);
215210

216-
cur = node->content;
217-
if (cur == NULL) {
218-
/* TODO: is this even possible? */
219-
cur = BAD_CAST "";
220-
}
211+
const xmlChar *cur = php_dom_get_content_or_empty(node);
221212

222213
length = xmlUTF8Strlen(cur);
223214

@@ -268,7 +259,7 @@ Modern spec URL: https://dom.spec.whatwg.org/#dom-characterdata-deletedata
268259
static void dom_character_data_delete_data(INTERNAL_FUNCTION_PARAMETERS, bool return_true)
269260
{
270261
zval *id;
271-
xmlChar *cur, *substring, *second;
262+
xmlChar *substring, *second;
272263
xmlNodePtr node;
273264
zend_long offset, count_input;
274265
unsigned int count;
@@ -282,11 +273,7 @@ static void dom_character_data_delete_data(INTERNAL_FUNCTION_PARAMETERS, bool re
282273

283274
DOM_GET_OBJ(node, id, xmlNodePtr, intern);
284275

285-
cur = node->content;
286-
if (cur == NULL) {
287-
/* TODO: is this even possible? */
288-
cur = BAD_CAST "";
289-
}
276+
const xmlChar *cur = php_dom_get_content_or_empty(node);
290277

291278
length = xmlUTF8Strlen(cur);
292279

@@ -340,7 +327,7 @@ Modern spec URL: https://dom.spec.whatwg.org/#dom-characterdata-replacedata
340327
static void dom_character_data_replace_data(INTERNAL_FUNCTION_PARAMETERS, bool return_true)
341328
{
342329
zval *id;
343-
xmlChar *cur, *substring, *second = NULL;
330+
xmlChar *substring, *second = NULL;
344331
xmlNodePtr node;
345332
char *arg;
346333
zend_long offset, count_input;
@@ -356,11 +343,7 @@ static void dom_character_data_replace_data(INTERNAL_FUNCTION_PARAMETERS, bool r
356343

357344
DOM_GET_OBJ(node, id, xmlNodePtr, intern);
358345

359-
cur = node->content;
360-
if (cur == NULL) {
361-
/* TODO: is this even possible? */
362-
cur = BAD_CAST "";
363-
}
346+
const xmlChar *cur = php_dom_get_content_or_empty(node);
364347

365348
length = xmlUTF8Strlen(cur);
366349

‎ext/dom/php_dom.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -294,6 +294,11 @@ static zend_always_inline xmlNodePtr php_dom_first_child_of_container_node(xmlNo
294294
}
295295
}
296296

297+
static zend_always_inline const xmlChar *php_dom_get_content_or_empty(const xmlNode *node)
298+
{
299+
return node->content ? node->content : BAD_CAST "";
300+
}
301+
297302
PHP_MINIT_FUNCTION(dom);
298303
PHP_MSHUTDOWN_FUNCTION(dom);
299304
PHP_MINFO_FUNCTION(dom);

‎ext/dom/tests/null_text_content_manipulation.phpt

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
--TEST--
2+
NULL text content manipulation
3+
--EXTENSIONS--
4+
dom
5+
--FILE--
6+
<?php
7+
$text = new DOMText();
8+
try {
9+
var_dump($text->substringData(1, 0));
10+
} catch (DOMException $e) {
11+
echo $e->getMessage(), "\n";
12+
}
13+
try {
14+
var_dump($text->insertData(1, ""));
15+
} catch (DOMException $e) {
16+
echo $e->getMessage(), "\n";
17+
}
18+
try {
19+
var_dump($text->deleteData(1, 1));
20+
} catch (DOMException $e) {
21+
echo $e->getMessage(), "\n";
22+
}
23+
try {
24+
var_dump($text->replaceData(1, 1, ""));
25+
} catch (DOMException $e) {
26+
echo $e->getMessage(), "\n";
27+
}
28+
?>
29+
--EXPECT--
30+
Index Size Error
31+
Index Size Error
32+
Index Size Error
33+
Index Size Error

‎ext/dom/text.c

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,6 @@ Modern spec URL: https://dom.spec.whatwg.org/#dom-text-splittext
100100
PHP_METHOD(DOMText, splitText)
101101
{
102102
zval *id;
103-
xmlChar *cur;
104103
xmlChar *first;
105104
xmlChar *second;
106105
xmlNodePtr node;
@@ -120,11 +119,7 @@ PHP_METHOD(DOMText, splitText)
120119
RETURN_THROWS();
121120
}
122121

123-
cur = node->content;
124-
if (cur == NULL) {
125-
/* TODO: is this even possible? */
126-
cur = BAD_CAST "";
127-
}
122+
const xmlChar *cur = php_dom_get_content_or_empty(node);
128123
length = xmlUTF8Strlen(cur);
129124

130125
if (ZEND_LONG_INT_OVFL(offset) || (int)offset > length) {

0 commit comments

Comments
 (0)