Skip to content

Commit 6dac6a9

Browse files
Dik Takkennikic
Dik Takken
authored and
nikic
committed
Warning promotion: Throw on writing invalid XML tag names
This change throws a ValueError when an invalid tag name is passed to XMLWriter. The rationale is that this indicates a programming error because tag names typically originate from string literals or application code generating them. This translates into either a typo or a flaw in tag generation logic. Closes GH-6233.
1 parent 25f1c40 commit 6dac6a9

File tree

2 files changed

+41
-30
lines changed

2 files changed

+41
-30
lines changed

‎ext/xmlwriter/php_xmlwriter.c

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -88,10 +88,10 @@ static zend_object *xmlwriter_object_new(zend_class_entry *class_type)
8888
}
8989
/* }}} */
9090

91-
#define XMLW_NAME_CHK(__err) \
91+
#define XMLW_NAME_CHK(__arg_no, __subject) \
9292
if (xmlValidateName((xmlChar *) name, 0) != 0) { \
93-
php_error_docref(NULL, E_WARNING, "%s", __err); \
94-
RETURN_FALSE; \
93+
zend_argument_value_error(__arg_no, "must be a valid %s, \"%s\" given", __subject, name); \
94+
RETURN_THROWS(); \
9595
} \
9696

9797
/* {{{ function prototypes */
@@ -199,7 +199,7 @@ static void xmlwriter_objects_clone(void *object, void **object_clone)
199199
}
200200
}}} */
201201

202-
static void php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAMETERS, xmlwriter_read_one_char_t internal_function, char *err_string)
202+
static void php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAMETERS, xmlwriter_read_one_char_t internal_function, char *subject_name)
203203
{
204204
xmlTextWriterPtr ptr;
205205
char *name;
@@ -212,8 +212,8 @@ static void php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAMETERS, xmlwriter_rea
212212
}
213213
XMLWRITER_FROM_OBJECT(ptr, self);
214214

215-
if (err_string != NULL) {
216-
XMLW_NAME_CHK(err_string);
215+
if (subject_name != NULL) {
216+
XMLW_NAME_CHK(2, subject_name);
217217
}
218218

219219
if (ptr) {
@@ -281,7 +281,7 @@ PHP_FUNCTION(xmlwriter_set_indent_string)
281281
/* {{{ Create start attribute - returns FALSE on error */
282282
PHP_FUNCTION(xmlwriter_start_attribute)
283283
{
284-
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartAttribute, "Invalid Attribute Name");
284+
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartAttribute, "attribute name");
285285
}
286286
/* }}} */
287287

@@ -307,7 +307,7 @@ PHP_FUNCTION(xmlwriter_start_attribute_ns)
307307
}
308308
XMLWRITER_FROM_OBJECT(ptr, self);
309309

310-
XMLW_NAME_CHK("Invalid Attribute Name");
310+
XMLW_NAME_CHK(3, "attribute name");
311311

312312
if (ptr) {
313313
retval = xmlTextWriterStartAttributeNS(ptr, (xmlChar *)prefix, (xmlChar *)name, (xmlChar *)uri);
@@ -335,7 +335,7 @@ PHP_FUNCTION(xmlwriter_write_attribute)
335335
}
336336
XMLWRITER_FROM_OBJECT(ptr, self);
337337

338-
XMLW_NAME_CHK("Invalid Attribute Name");
338+
XMLW_NAME_CHK(2, "attribute name");
339339

340340
if (ptr) {
341341
retval = xmlTextWriterWriteAttribute(ptr, (xmlChar *)name, (xmlChar *)content);
@@ -363,7 +363,7 @@ PHP_FUNCTION(xmlwriter_write_attribute_ns)
363363
}
364364
XMLWRITER_FROM_OBJECT(ptr, self);
365365

366-
XMLW_NAME_CHK("Invalid Attribute Name");
366+
XMLW_NAME_CHK(3, "attribute name");
367367

368368
if (ptr) {
369369
retval = xmlTextWriterWriteAttributeNS(ptr, (xmlChar *)prefix, (xmlChar *)name, (xmlChar *)uri, (xmlChar *)content);
@@ -379,7 +379,7 @@ PHP_FUNCTION(xmlwriter_write_attribute_ns)
379379
/* {{{ Create start element tag - returns FALSE on error */
380380
PHP_FUNCTION(xmlwriter_start_element)
381381
{
382-
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartElement, "Invalid Element Name");
382+
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartElement, "element name");
383383
}
384384
/* }}} */
385385

@@ -398,7 +398,7 @@ PHP_FUNCTION(xmlwriter_start_element_ns)
398398
}
399399
XMLWRITER_FROM_OBJECT(ptr, self);
400400

401-
XMLW_NAME_CHK("Invalid Element Name");
401+
XMLW_NAME_CHK(3, "element name");
402402

403403
if (ptr) {
404404
retval = xmlTextWriterStartElementNS(ptr, (xmlChar *)prefix, (xmlChar *)name, (xmlChar *)uri);
@@ -441,7 +441,7 @@ PHP_FUNCTION(xmlwriter_write_element)
441441
}
442442
XMLWRITER_FROM_OBJECT(ptr, self);
443443

444-
XMLW_NAME_CHK("Invalid Element Name");
444+
XMLW_NAME_CHK(2, "element name");
445445

446446
if (ptr) {
447447
if (!content) {
@@ -480,7 +480,7 @@ PHP_FUNCTION(xmlwriter_write_element_ns)
480480
}
481481
XMLWRITER_FROM_OBJECT(ptr, self);
482482

483-
XMLW_NAME_CHK("Invalid Element Name");
483+
XMLW_NAME_CHK(3, "element name");
484484

485485
if (ptr) {
486486
if (!content) {
@@ -507,7 +507,7 @@ PHP_FUNCTION(xmlwriter_write_element_ns)
507507
/* {{{ Create start PI tag - returns FALSE on error */
508508
PHP_FUNCTION(xmlwriter_start_pi)
509509
{
510-
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartPI, "Invalid PI Target");
510+
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartPI, "PI target");
511511
}
512512
/* }}} */
513513

@@ -533,7 +533,7 @@ PHP_FUNCTION(xmlwriter_write_pi)
533533
}
534534
XMLWRITER_FROM_OBJECT(ptr, self);
535535

536-
XMLW_NAME_CHK("Invalid PI Target");
536+
XMLW_NAME_CHK(2, "PI target");
537537

538538
if (ptr) {
539539
retval = xmlTextWriterWritePI(ptr, (xmlChar *)name, (xmlChar *)content);
@@ -726,7 +726,7 @@ PHP_FUNCTION(xmlwriter_write_dtd)
726726
/* {{{ Create start DTD element - returns FALSE on error */
727727
PHP_FUNCTION(xmlwriter_start_dtd_element)
728728
{
729-
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartDTDElement, "Invalid Element Name");
729+
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartDTDElement, "element name");
730730
}
731731
/* }}} */
732732

@@ -752,7 +752,7 @@ PHP_FUNCTION(xmlwriter_write_dtd_element)
752752
}
753753
XMLWRITER_FROM_OBJECT(ptr, self);
754754

755-
XMLW_NAME_CHK("Invalid Element Name");
755+
XMLW_NAME_CHK(2, "element name");
756756

757757
if (ptr) {
758758
retval = xmlTextWriterWriteDTDElement(ptr, (xmlChar *)name, (xmlChar *)content);
@@ -768,7 +768,7 @@ PHP_FUNCTION(xmlwriter_write_dtd_element)
768768
/* {{{ Create start DTD AttList - returns FALSE on error */
769769
PHP_FUNCTION(xmlwriter_start_dtd_attlist)
770770
{
771-
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartDTDAttlist, "Invalid Element Name");
771+
php_xmlwriter_string_arg(INTERNAL_FUNCTION_PARAM_PASSTHRU, xmlTextWriterStartDTDAttlist, "element name");
772772
}
773773
/* }}} */
774774

@@ -794,7 +794,7 @@ PHP_FUNCTION(xmlwriter_write_dtd_attlist)
794794
}
795795
XMLWRITER_FROM_OBJECT(ptr, self);
796796

797-
XMLW_NAME_CHK("Invalid Element Name");
797+
XMLW_NAME_CHK(2, "element name");
798798

799799
if (ptr) {
800800
retval = xmlTextWriterWriteDTDAttlist(ptr, (xmlChar *)name, (xmlChar *)content);
@@ -822,7 +822,7 @@ PHP_FUNCTION(xmlwriter_start_dtd_entity)
822822
}
823823
XMLWRITER_FROM_OBJECT(ptr, self);
824824

825-
XMLW_NAME_CHK("Invalid Attribute Name");
825+
XMLW_NAME_CHK(2, "attribute name");
826826

827827
if (ptr) {
828828
retval = xmlTextWriterStartDTDEntity(ptr, isparm, (xmlChar *)name);
@@ -862,7 +862,7 @@ PHP_FUNCTION(xmlwriter_write_dtd_entity)
862862
}
863863
XMLWRITER_FROM_OBJECT(ptr, self);
864864

865-
XMLW_NAME_CHK("Invalid Element Name");
865+
XMLW_NAME_CHK(2, "element name");
866866

867867
if (ptr) {
868868
retval = xmlTextWriterWriteDTDEntity(ptr, pe, (xmlChar *)name, (xmlChar *)pubid, (xmlChar *)sysid, (xmlChar *)ndataid, (xmlChar *)content);

‎ext/xmlwriter/tests/010.phpt

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,27 @@ $xw = xmlwriter_open_uri($file);
1414
var_dump(xmlwriter_start_element($xw, "tag"));
1515
var_dump(xmlwriter_start_attribute($xw, "attr"));
1616
var_dump(xmlwriter_end_attribute($xw));
17-
var_dump(xmlwriter_start_attribute($xw, "-1"));
17+
18+
try {
19+
xmlwriter_start_attribute($xw, "-1");
20+
} catch (ValueError $e) {
21+
echo $e->getMessage(), "\n";
22+
}
23+
1824
var_dump(xmlwriter_end_attribute($xw));
19-
var_dump(xmlwriter_start_attribute($xw, "\""));
25+
26+
try {
27+
xmlwriter_start_attribute($xw, "\"");
28+
} catch (ValueError $e) {
29+
echo $e->getMessage(), "\n";
30+
}
31+
2032
var_dump(xmlwriter_end_attribute($xw));
2133
var_dump(xmlwriter_end_element($xw));
2234

35+
// Force to write and empty the buffer
36+
xmlwriter_flush($xw, empty: true);
37+
2338
unset($xw);
2439

2540
var_dump(file_get_contents($file));
@@ -32,13 +47,9 @@ echo "Done\n";
3247
bool(true)
3348
bool(true)
3449
bool(true)
35-
36-
Warning: xmlwriter_start_attribute(): Invalid Attribute Name in %s on line %d
37-
bool(false)
38-
bool(false)
39-
40-
Warning: xmlwriter_start_attribute(): Invalid Attribute Name in %s on line %d
50+
xmlwriter_start_attribute(): Argument #2 ($name) must be a valid attribute name, "-1" given
4151
bool(false)
52+
xmlwriter_start_attribute(): Argument #2 ($name) must be a valid attribute name, """ given
4253
bool(false)
4354
bool(true)
4455
string(14) "<tag attr=""/>"

0 commit comments

Comments
 (0)