Skip to content

Commit f068fbc

Browse files
kocsismatekocsismate
committed
Promote warnings to exceptions in ext/xmlreader
Closes GH-6021
1 parent f4e9d0e commit f068fbc

17 files changed

+202
-124
lines changed

‎ext/xmlreader/php_xmlreader.c

Lines changed: 60 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ zval *xmlreader_write_property(zend_object *object, zend_string *name, zval *val
175175
hnd = zend_hash_find_ptr(obj->prop_handler, name);
176176
}
177177
if (hnd != NULL) {
178-
php_error_docref(NULL, E_WARNING, "Cannot write to read-only property");
178+
zend_throw_error(NULL, "Cannot write to read-only property");
179179
} else {
180180
value = zend_std_write_property(object, name, value, cache_slot);
181181
}
@@ -391,8 +391,8 @@ static void php_xmlreader_string_arg(INTERNAL_FUNCTION_PARAMETERS, xmlreader_rea
391391
}
392392

393393
if (!name_len) {
394-
php_error_docref(NULL, E_WARNING, "Argument cannot be an empty string");
395-
RETURN_FALSE;
394+
zend_argument_value_error(1, "cannot be empty");
395+
RETURN_THROWS();
396396
}
397397

398398
id = ZEND_THIS;
@@ -480,8 +480,8 @@ static void php_xmlreader_set_relaxng_schema(INTERNAL_FUNCTION_PARAMETERS, int t
480480
}
481481

482482
if (source != NULL && !source_len) {
483-
php_error_docref(NULL, E_WARNING, "Schema data source is required");
484-
RETURN_FALSE;
483+
zend_argument_value_error(1, "cannot be empty");
484+
RETURN_THROWS();
485485
}
486486

487487
id = ZEND_THIS;
@@ -506,15 +506,16 @@ static void php_xmlreader_set_relaxng_schema(INTERNAL_FUNCTION_PARAMETERS, int t
506506
intern->schema = schema;
507507

508508
RETURN_TRUE;
509+
} else {
510+
php_error_docref(NULL, E_WARNING, "Schema contains errors");
511+
RETURN_FALSE;
509512
}
513+
} else {
514+
zend_throw_error(NULL, "Schema must be set prior to reading");
515+
RETURN_THROWS();
510516
}
511-
512-
php_error_docref(NULL, E_WARNING, "Unable to set schema. This must be set prior to reading or schema contains errors.");
513-
514-
RETURN_FALSE;
515517
#else
516-
php_error_docref(NULL, E_WARNING, "No Schema support built into libxml.");
517-
518+
php_error_docref(NULL, E_WARNING, "No schema support built into libxml");
518519
RETURN_FALSE;
519520
#endif
520521
}
@@ -585,9 +586,14 @@ PHP_METHOD(XMLReader, getAttributeNs)
585586
RETURN_THROWS();
586587
}
587588

588-
if (name_len == 0 || ns_uri_len == 0) {
589-
php_error_docref(NULL, E_WARNING, "Attribute Name and Namespace URI cannot be empty");
590-
RETURN_FALSE;
589+
if (name_len == 0) {
590+
zend_argument_value_error(1, "cannot be empty");
591+
RETURN_THROWS();
592+
}
593+
594+
if (ns_uri_len == 0) {
595+
zend_argument_value_error(2, "cannot be empty");
596+
RETURN_THROWS();
591597
}
592598

593599
id = ZEND_THIS;
@@ -622,8 +628,8 @@ PHP_METHOD(XMLReader, getParserProperty)
622628
retval = xmlTextReaderGetParserProp(intern->ptr,property);
623629
}
624630
if (retval == -1) {
625-
php_error_docref(NULL, E_WARNING, "Invalid parser property");
626-
RETURN_FALSE;
631+
zend_argument_value_error(1, "must be a valid parser property");
632+
RETURN_THROWS();
627633
}
628634

629635
RETURN_BOOL(retval);
@@ -660,8 +666,8 @@ PHP_METHOD(XMLReader, moveToAttribute)
660666
}
661667

662668
if (name_len == 0) {
663-
php_error_docref(NULL, E_WARNING, "Attribute Name is required");
664-
RETURN_FALSE;
669+
zend_argument_value_error(1, "cannot be empty");
670+
RETURN_THROWS();
665671
}
666672

667673
id = ZEND_THIS;
@@ -719,9 +725,14 @@ PHP_METHOD(XMLReader, moveToAttributeNs)
719725
RETURN_THROWS();
720726
}
721727

722-
if (name_len == 0 || ns_uri_len == 0) {
723-
php_error_docref(NULL, E_WARNING, "Attribute Name and Namespace URI cannot be empty");
724-
RETURN_FALSE;
728+
if (name_len == 0) {
729+
zend_argument_value_error(1, "cannot be empty");
730+
RETURN_THROWS();
731+
}
732+
733+
if (ns_uri_len == 0) {
734+
zend_argument_value_error(2, "cannot be empty");
735+
RETURN_THROWS();
725736
}
726737

727738
id = ZEND_THIS;
@@ -772,17 +783,17 @@ PHP_METHOD(XMLReader, read)
772783

773784
id = ZEND_THIS;
774785
intern = Z_XMLREADER_P(id);
775-
if (intern != NULL && intern->ptr != NULL) {
776-
retval = xmlTextReaderRead(intern->ptr);
777-
if (retval == -1) {
778-
RETURN_FALSE;
779-
} else {
780-
RETURN_BOOL(retval);
781-
}
786+
if (intern == NULL || intern->ptr == NULL) {
787+
zend_throw_error(NULL, "Data must be loaded before reading");
788+
RETURN_THROWS();
782789
}
783790

784-
php_error_docref(NULL, E_WARNING, "Load Data before trying to read");
785-
RETURN_FALSE;
791+
retval = xmlTextReaderRead(intern->ptr);
792+
if (retval == -1) {
793+
RETURN_FALSE;
794+
} else {
795+
RETURN_BOOL(retval);
796+
}
786797
}
787798
/* }}} */
788799

@@ -816,8 +827,7 @@ PHP_METHOD(XMLReader, next)
816827
}
817828
}
818829

819-
php_error_docref(NULL, E_WARNING, "Load Data before trying to read");
820-
RETURN_FALSE;
830+
zend_throw_error(NULL, "Data must be loaded before reading");
821831
}
822832
/* }}} */
823833

@@ -848,8 +858,8 @@ PHP_METHOD(XMLReader, open)
848858
}
849859

850860
if (!source_len) {
851-
php_error_docref(NULL, E_WARNING, "Empty string supplied as input");
852-
RETURN_FALSE;
861+
zend_argument_value_error(1, "cannot be empty");
862+
RETURN_THROWS();
853863
}
854864

855865
valid_file = _xmlreader_get_valid_file_path(source, resolved_path, MAXPATHLEN );
@@ -920,8 +930,8 @@ PHP_METHOD(XMLReader, setSchema)
920930
}
921931

922932
if (source != NULL && !source_len) {
923-
php_error_docref(NULL, E_WARNING, "Schema data source is required");
924-
RETURN_FALSE;
933+
zend_argument_value_error(1, "cannot be empty");
934+
RETURN_THROWS();
925935
}
926936

927937
id = ZEND_THIS;
@@ -932,15 +942,16 @@ PHP_METHOD(XMLReader, setSchema)
932942

933943
if (retval == 0) {
934944
RETURN_TRUE;
945+
} else {
946+
php_error_docref(NULL, E_WARNING, "Schema contains errors");
947+
RETURN_FALSE;
935948
}
949+
} else {
950+
zend_throw_error(NULL, "Schema must be set prior to reading");
951+
RETURN_THROWS();
936952
}
937-
938-
php_error_docref(NULL, E_WARNING, "Unable to set schema. This must be set prior to reading or schema contains errors.");
939-
940-
RETURN_FALSE;
941953
#else
942-
php_error_docref(NULL, E_WARNING, "No Schema support built into libxml.");
943-
954+
php_error_docref(NULL, E_WARNING, "No schema support built into libxml");
944955
RETURN_FALSE;
945956
#endif
946957
}
@@ -967,8 +978,8 @@ PHP_METHOD(XMLReader, setParserProperty)
967978
retval = xmlTextReaderSetParserProp(intern->ptr,property, value);
968979
}
969980
if (retval == -1) {
970-
php_error_docref(NULL, E_WARNING, "Invalid parser property");
971-
RETURN_FALSE;
981+
zend_argument_value_error(1, "must be a valid parser property");
982+
RETURN_THROWS();
972983
}
973984

974985
RETURN_TRUE;
@@ -1022,8 +1033,8 @@ PHP_METHOD(XMLReader, XML)
10221033
}
10231034

10241035
if (!source_len) {
1025-
php_error_docref(NULL, E_WARNING, "Empty string supplied as input");
1026-
RETURN_FALSE;
1036+
zend_argument_value_error(1, "cannot be empty");
1037+
RETURN_THROWS();
10271038
}
10281039

10291040
inputbfr = xmlParserInputBufferCreateMem(source, source_len, XML_CHAR_ENCODING_NONE);
@@ -1105,7 +1116,7 @@ PHP_METHOD(XMLReader, expand)
11051116
node = xmlTextReaderExpand(intern->ptr);
11061117

11071118
if (node == NULL) {
1108-
php_error_docref(NULL, E_WARNING, "An Error Occurred while expanding ");
1119+
php_error_docref(NULL, E_WARNING, "An Error Occurred while expanding");
11091120
RETURN_FALSE;
11101121
} else {
11111122
nodec = xmlDocCopyNode(node, docp, 1);
@@ -1117,8 +1128,8 @@ PHP_METHOD(XMLReader, expand)
11171128
}
11181129
}
11191130
} else {
1120-
php_error_docref(NULL, E_WARNING, "Load Data before trying to expand");
1121-
RETURN_FALSE;
1131+
zend_throw_error(NULL, "Data must be loaded before expanding");
1132+
RETURN_THROWS();
11221133
}
11231134
#else
11241135
php_error(E_WARNING, "DOM support is not enabled");

‎ext/xmlreader/php_xmlreader.stub.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ class XMLReader
77
/** @return bool */
88
public function close() {}
99

10-
/** @return string|null|false */
10+
/** @return string|null */
1111
public function getAttribute(string $name) {}
1212

1313
/** @return string|null */
@@ -22,7 +22,7 @@ public function getParserProperty(int $property) {}
2222
/** @return bool */
2323
public function isValid() {}
2424

25-
/** @return string|null|false */
25+
/** @return string|null */
2626
public function lookupNamespace(string $prefix) {}
2727

2828
/** @return bool */

‎ext/xmlreader/php_xmlreader_arginfo.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/* This is a generated file, edit the .stub.php file instead.
2-
* Stub hash: 8bdec18c4ad8574fb1d3e4baca928949d5ec2438 */
2+
* Stub hash: 90e6d525ba87399c54f36965ebf18dbf65084617 */
33

44
ZEND_BEGIN_ARG_INFO_EX(arginfo_class_XMLReader_close, 0, 0, 0)
55
ZEND_END_ARG_INFO()

‎ext/xmlreader/tests/001.phpt

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,15 @@ while ($reader->read()) {
1717
}
1818
$xmlstring = '';
1919
$reader = new XMLReader();
20-
$reader->XML($xmlstring);
20+
21+
try {
22+
$reader->XML($xmlstring);
23+
} catch (ValueError $exception) {
24+
echo $exception->getMessage() . "\n";
25+
}
26+
2127
?>
22-
--EXPECTF--
28+
--EXPECT--
2329
books
2430
books
25-
26-
Warning: XMLReader::XML(): Empty string supplied as input in %s on line %d
31+
XMLReader::XML(): Argument #1 ($source) cannot be empty

‎ext/xmlreader/tests/002.phpt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ $xmlstring = '<?xml version="1.0" encoding="UTF-8"?>
1010
file_put_contents($filename, $xmlstring);
1111

1212
$reader = new XMLReader();
13-
if ($reader->open('')) exit();
13+
try {
14+
$reader->open('');
15+
} catch (ValueError $exception) {
16+
echo $exception->getMessage() . "\n";
17+
}
1418

1519
$reader = new XMLReader();
1620
if (!$reader->open($filename)) {
@@ -31,7 +35,7 @@ $reader->close();
3135
unlink($filename);
3236

3337
?>
34-
--EXPECTF--
35-
Warning: XMLReader::open(): Empty string supplied as input in %s on line %d
38+
--EXPECT--
39+
XMLReader::open(): Argument #1 ($URI) cannot be empty
3640
books
3741
books

‎ext/xmlreader/tests/003-get-errors.phpt

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,12 @@ while ($reader->read()) {
3131
echo $reader->value . "\n";
3232

3333
// Test for call with an empty string argument
34-
$attr = $reader->getAttribute('');
35-
var_dump($attr);
34+
try {
35+
$reader->getAttribute('');
36+
} catch (ValueError $exception) {
37+
echo $exception->getMessage() . "\n";
38+
}
39+
3640
// Ensure that node pointer has not changed position
3741
echo $reader->name . ": ";
3842
echo $reader->value . "\n";
@@ -61,13 +65,11 @@ $reader->close();
6165
<?php
6266
unlink(__DIR__.'/003-get-errors.xml');
6367
?>
64-
--EXPECTF--
68+
--EXPECT--
6569
book
6670
bool(true)
6771
num: 1
68-
69-
Warning: XMLReader::getAttribute(): Argument cannot be an empty string in %s on line %d
70-
bool(false)
72+
XMLReader::getAttribute(): Argument #1 ($name) cannot be empty
7173
num: 1
7274
NULL
7375
num: 1

‎ext/xmlreader/tests/003-move-errors.phpt

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,12 @@ while ($reader->read()) {
3131
echo $reader->value . "\n";
3232

3333
// Test for call with an empty string argument
34-
$attr = $reader->moveToAttribute('');
35-
var_dump($attr);
34+
try {
35+
$reader->moveToAttribute('');
36+
} catch (ValueError $exception) {
37+
echo $exception->getMessage() . "\n";
38+
}
39+
3640
// Ensure that node pointer has not changed position
3741
echo $reader->name . ": ";
3842
echo $reader->value . "\n";
@@ -60,13 +64,11 @@ $reader->close();
6064
<?php
6165
unlink(__DIR__.'/003-move-errors.xml');
6266
?>
63-
--EXPECTF--
67+
--EXPECT--
6468
book
6569
bool(true)
6670
num: 1
67-
68-
Warning: XMLReader::moveToAttribute(): Attribute Name is required in %s on line %d
69-
bool(false)
71+
XMLReader::moveToAttribute(): Argument #1 ($name) cannot be empty
7072
num: 1
7173
bool(false)
7274
num: 1

‎ext/xmlreader/tests/003.phpt

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,14 +68,18 @@ while ($reader->read()) {
6868

6969
var_dump($reader->moveToAttributeNo(20));
7070
var_dump($reader->moveToAttribute('missing-attribute'));
71-
var_dump($reader->moveToAttribute(''));
71+
try {
72+
$reader->moveToAttribute('');
73+
} catch (ValueError $exception) {
74+
echo $exception->getMessage() . "\n";
75+
}
7276
}
7377
}
7478
}
7579
$reader->close();
7680
unlink($filename);
7781
?>
78-
--EXPECTF--
82+
--EXPECT--
7983
num: 1
8084
idx: 2
8185
num: 1
@@ -84,6 +88,4 @@ num: 1
8488
idx: 2
8589
bool(false)
8690
bool(false)
87-
88-
Warning: XMLReader::moveToAttribute(): Attribute Name is required in %s on line %d
89-
bool(false)
91+
XMLReader::moveToAttribute(): Argument #1 ($name) cannot be empty

0 commit comments

Comments
 (0)