Make handling of NULL bytes in file paths more consistent (WIP)

Not all extensions consistently throw exceptions when the user passes
a path name containing null bytes. Also, some extensions would throw
a ValueError while others would throw a TypeError. Error messages
also varied.

Now a ValueError is thrown after all failed path checks, at least for
as far as these occur in functions that are exposed to userland.

Closes GH-6216.

diff --git a/ext/dom/document.c b/ext/dom/document.c
index d3a8fa43a5aa7bf8312eca99e98aff1227560d12..23bdcae1e0c1a4d4c85344ddf918c5ecb25fcc3e 100644 (file)
--- a/ext/dom/document.c
+++ b/ext/dom/document.c
@@ -1197,6 +1197,7 @@ static xmlDocPtr dom_document_parser(zval *id, int mode, char *source, size_t so
        if (mode == DOM_LOAD_FILE) {
                char *file_dest;
                if (CHECK_NULL_PATH(source, source_len)) {
+                       zend_value_error("Path to document must not contain any null bytes");
                        return NULL;
                }
                file_dest = _dom_get_valid_file_path(source, resolved_path, MAXPATHLEN);
@@ -1305,8 +1306,8 @@ static void dom_parse_document(INTERNAL_FUNCTION_PARAMETERS, int mode) {
        }
 
        if (!source_len) {
-               php_error_docref(NULL, E_WARNING, "Empty string supplied as input");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "must not be empty");
+               RETURN_THROWS();
        }
        if (ZEND_SIZE_T_INT_OVFL(source_len)) {
                php_error_docref(NULL, E_WARNING, "Input string is too long");
@@ -1388,8 +1389,8 @@ PHP_METHOD(DOMDocument, save)
        }
 
        if (file_len == 0) {
-               php_error_docref(NULL, E_WARNING, "Invalid Filename");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "must not be empty");
+               RETURN_THROWS();
        }
 
        DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
@@ -1623,9 +1624,9 @@ static void _dom_document_schema_validate(INTERNAL_FUNCTION_PARAMETERS, int type
                RETURN_THROWS();
        }
 
-       if (source_len == 0) {
-               php_error_docref(NULL, E_WARNING, "Invalid Schema source");
-               RETURN_FALSE;
+       if (!source_len) {
+               zend_argument_value_error(1, "must not be empty");
+               RETURN_THROWS();
        }
 
        DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
@@ -1633,8 +1634,8 @@ static void _dom_document_schema_validate(INTERNAL_FUNCTION_PARAMETERS, int type
        switch (type) {
        case DOM_LOAD_FILE:
                if (CHECK_NULL_PATH(source, source_len)) {
-                       php_error_docref(NULL, E_WARNING, "Invalid Schema file source");
-                       RETURN_FALSE;
+                       zend_argument_value_error(1, "must not contain any null bytes");
+                       RETURN_THROWS();
                }
                valid_file = _dom_get_valid_file_path(source, resolved_path, MAXPATHLEN);
                if (!valid_file) {
@@ -1724,9 +1725,9 @@ static void _dom_document_relaxNG_validate(INTERNAL_FUNCTION_PARAMETERS, int typ
                RETURN_THROWS();
        }
 
-       if (source_len == 0) {
-               php_error_docref(NULL, E_WARNING, "Invalid Schema source");
-               RETURN_FALSE;
+       if (!source_len) {
+               zend_argument_value_error(1, "must not be empty");
+               RETURN_THROWS();
        }
 
        DOM_GET_OBJ(docp, id, xmlDocPtr, intern);
@@ -1734,8 +1735,8 @@ static void _dom_document_relaxNG_validate(INTERNAL_FUNCTION_PARAMETERS, int typ
        switch (type) {
        case DOM_LOAD_FILE:
                if (CHECK_NULL_PATH(source, source_len)) {
-                       php_error_docref(NULL, E_WARNING, "Invalid RelaxNG file source");
-                       RETURN_FALSE;
+                       zend_argument_value_error(1, "must not contain any null bytes");
+                       RETURN_THROWS();
                }
                valid_file = _dom_get_valid_file_path(source, resolved_path, MAXPATHLEN);
                if (!valid_file) {
@@ -1823,8 +1824,8 @@ static void dom_load_html(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{ */
        }
 
        if (!source_len) {
-               php_error_docref(NULL, E_WARNING, "Empty string supplied as input");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "must not be empty");
+               RETURN_THROWS();
        }
 
        if (ZEND_LONG_EXCEEDS_INT(options)) {
@@ -1834,8 +1835,8 @@ static void dom_load_html(INTERNAL_FUNCTION_PARAMETERS, int mode) /* {{{ */
 
        if (mode == DOM_LOAD_FILE) {
                if (CHECK_NULL_PATH(source, source_len)) {
-                       php_error_docref(NULL, E_WARNING, "Invalid file source");
-                       RETURN_FALSE;
+                       zend_argument_value_error(1, "must not contain any null bytes");
+                       RETURN_THROWS();
                }
                ctxt = htmlCreateFileParserCtxt(source, NULL);
        } else {
@@ -1930,8 +1931,8 @@ PHP_METHOD(DOMDocument, saveHTMLFile)
        }
 
        if (file_len == 0) {
-               php_error_docref(NULL, E_WARNING, "Invalid Filename");
-               RETURN_FALSE;
+               zend_argument_value_error(1, "must not be empty");
+               RETURN_THROWS();
        }
 
        DOM_GET_OBJ(docp, id, xmlDocPtr, intern);

diff --git a/ext/dom/tests/DOMDocument_loadHTML_error2.phpt b/ext/dom/tests/DOMDocument_loadHTML_error2.phpt
index 3167c0189034a3366b4b1ef50c4c3327791df88a..695f58868d5b20788ca3c84cb2542500204c9f4c 100644 (file)
--- a/ext/dom/tests/DOMDocument_loadHTML_error2.phpt
+++ b/ext/dom/tests/DOMDocument_loadHTML_error2.phpt
@@ -9,7 +9,11 @@ require_once('skipif.inc');
 --FILE--
 <?php
 $doc = new DOMDocument();
-$doc->loadHTML('');
+try {
+    $doc->loadHTML('');
+} catch (ValueError $e) {
+    echo $e->getMessage() . "\n";
+}
 ?>
---EXPECTF--
-Warning: DOMDocument::loadHTML(): Empty string supplied as input in %s on line %d
+--EXPECT--
+DOMDocument::loadHTML(): Argument #1 ($source) must not be empty

diff --git a/ext/dom/tests/DOMDocument_loadHTMLfile_error2.phpt b/ext/dom/tests/DOMDocument_loadHTMLfile_error2.phpt
index 55b36e712d18a056ccc6068cb9b5a1f6ad45db6c..492bedda1027acc16f2e11bba3c99002402c53ee 100644 (file)
--- a/ext/dom/tests/DOMDocument_loadHTMLfile_error2.phpt
+++ b/ext/dom/tests/DOMDocument_loadHTMLfile_error2.phpt
@@ -11,13 +11,19 @@ assert.bail=true
 --FILE--
 <?php
 $doc = new DOMDocument();
-$result = $doc->loadHTMLFile("");
-assert($result === false);
+try {
+    $result = $doc->loadHTMLFile("");
+} catch (ValueError $e) {
+    echo $e->getMessage() . "\n";
+}
+
 $doc = new DOMDocument();
-$result = $doc->loadHTMLFile("text.html\0something");
-assert($result === false);
+try {
+    $result = $doc->loadHTMLFile("text.html\0something");
+} catch (ValueError $e) {
+    echo $e->getMessage() . "\n";
+}
 ?>
---EXPECTF--
-%r(PHP ){0,1}%rWarning: DOMDocument::loadHTMLFile(): Empty string supplied as input %s
-
-%r(PHP ){0,1}%rWarning: DOMDocument::loadHTMLFile(): Invalid file source %s
+--EXPECT--
+DOMDocument::loadHTMLFile(): Argument #1 ($filename) must not be empty
+DOMDocument::loadHTMLFile(): Argument #1 ($filename) must not contain any null bytes

diff --git a/ext/dom/tests/DOMDocument_loadXML_error6.phpt b/ext/dom/tests/DOMDocument_loadXML_error6.phpt
new file mode 100644 (file)
index 0000000..9f22692
--- /dev/null
+++ b/ext/dom/tests/DOMDocument_loadXML_error6.phpt
@@ -0,0 +1,16 @@
+--TEST--
+Test DOMDocument::loadXML() with empty file path
+--SKIPIF--
+<?php require_once('skipif.inc'); ?>
+--FILE--
+<?php
+// create dom document
+$dom = new DOMDocument();
+try {
+    $dom->loadXML("");
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+?>
+--EXPECT--
+DOMDocument::loadXML(): Argument #1 ($source) must not be empty

diff --git a/ext/dom/tests/DOMDocument_load_error6.phpt b/ext/dom/tests/DOMDocument_load_error6.phpt
new file mode 100644 (file)
index 0000000..70e2706
--- /dev/null
+++ b/ext/dom/tests/DOMDocument_load_error6.phpt
@@ -0,0 +1,27 @@
+--TEST--
+Test DOMDocument::load() with invalid paths
+--SKIPIF--
+<?php require_once('skipif.inc'); ?>
+--FILE--
+<?php
+// create dom document
+$dom = new DOMDocument();
+try {
+    $dom->load("");
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+try {
+    $dom->load("/path/with/\0/byte");
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
+
+// Path is too long
+var_dump($dom->load(str_repeat(" ", PHP_MAXPATHLEN + 1)));
+?>
+--EXPECT--
+DOMDocument::load(): Argument #1 ($filename) must not be empty
+Path to document must not contain any null bytes
+bool(false)

diff --git a/ext/dom/tests/DOMDocument_saveHTMLFile_invalid_filename.phpt b/ext/dom/tests/DOMDocument_saveHTMLFile_invalid_filename.phpt
index 6ca66730eb588b65a58c025f39106fccc5158e2b..12b45a491bd045cfe5adc28dda510a7caa441bd4 100644 (file)
--- a/ext/dom/tests/DOMDocument_saveHTMLFile_invalid_filename.phpt
+++ b/ext/dom/tests/DOMDocument_saveHTMLFile_invalid_filename.phpt
@@ -19,7 +19,11 @@ $title = $doc->createElement('title');
 $title = $head->appendChild($title);
 $text = $doc->createTextNode('This is the title');
 $text = $title->appendChild($text);
-$bytes = $doc->saveHTMLFile($filename);
+try {
+    $doc->saveHTMLFile($filename);
+} catch (ValueError $exception) {
+    echo $exception->getMessage() . "\n";
+}
 ?>
---EXPECTF--
-Warning: DOMDocument::saveHTMLFile(): Invalid Filename in %s on line %d
+--EXPECT--
+DOMDocument::saveHTMLFile(): Argument #1 ($filename) must not be empty

diff --git a/ext/dom/tests/DOMDocument_schemaValidateSource_error3.phpt b/ext/dom/tests/DOMDocument_schemaValidateSource_error3.phpt
index 41f9015b92b7a0d03b5804bef1b938577f8781d0..dc7a1db9012739e48be4d2e4995496a151277d6e 100644 (file)
--- a/ext/dom/tests/DOMDocument_schemaValidateSource_error3.phpt
+++ b/ext/dom/tests/DOMDocument_schemaValidateSource_error3.phpt
@@ -12,10 +12,12 @@ $doc = new DOMDocument;
 
 $doc->load(__DIR__."/book.xml");
 
-$result = $doc->schemaValidateSource('');
-var_dump($result);
+try {
+    $doc->schemaValidateSource('');
+} catch (ValueError $e) {
+    echo $e->getMessage() . "\n";
+}
 
 ?>
---EXPECTF--
-Warning: DOMDocument::schemaValidateSource(): Invalid Schema source in %s.php on line %d
-bool(false)
+--EXPECT--
+DOMDocument::schemaValidateSource(): Argument #1 ($source) must not be empty

diff --git a/ext/dom/tests/DOMDocument_schemaValidate_error3.phpt b/ext/dom/tests/DOMDocument_schemaValidate_error3.phpt
index 2ff2ce9487e6990eb28d026eae4bac25d7d0061d..0add6b2442a6d3794df8203ba5912fead3511c3e 100644 (file)
--- a/ext/dom/tests/DOMDocument_schemaValidate_error3.phpt
+++ b/ext/dom/tests/DOMDocument_schemaValidate_error3.phpt
@@ -12,10 +12,12 @@ $doc = new DOMDocument;
 
 $doc->load(__DIR__."/book.xml");
 
-$result = $doc->schemaValidate('');
-var_dump($result);
+try {
+    $doc->schemaValidate('');
+} catch (ValueError $e) {
+    echo $e->getMessage() . "\n";
+}
 
 ?>
---EXPECTF--
-Warning: DOMDocument::schemaValidate(): Invalid Schema source in %s.php on line %d
-bool(false)
+--EXPECT--
+DOMDocument::schemaValidate(): Argument #1 ($filename) must not be empty

diff --git a/ext/dom/tests/DOMDocument_schemaValidate_error6.phpt b/ext/dom/tests/DOMDocument_schemaValidate_error6.phpt
new file mode 100644 (file)
index 0000000..1dbfb9a
--- /dev/null
+++ b/ext/dom/tests/DOMDocument_schemaValidate_error6.phpt
@@ -0,0 +1,25 @@
+--TEST--
+DomDocument::schemaValidate() - invalid path to schema
+--SKIPIF--
+<?php require_once('skipif.inc'); ?>
+--FILE--
+<?php
+
+$doc = new DOMDocument;
+
+$doc->load(__DIR__."/book.xml");
+
+try {
+    $doc->schemaValidate("/path/with/\0/byte");
+} catch (ValueError $e) {
+    echo $e->getMessage() . "\n";
+}
+
+var_dump($doc->schemaValidate(str_repeat(" ", PHP_MAXPATHLEN + 1)));
+
+?>
+--EXPECTF--
+DOMDocument::schemaValidate(): Argument #1 ($filename) must not contain any null bytes
+
+Warning: DOMDocument::schemaValidate(): Invalid Schema file source in %s on line %d
+bool(false)

diff --git a/ext/fileinfo/fileinfo.c b/ext/fileinfo/fileinfo.c
index 1b26736ad0c4547e9bb655a0285b5f47133c618c..0a4ee3c34eebb5eb4a4ef9dadd5b924369bc286f 100644 (file)
--- a/ext/fileinfo/fileinfo.c
+++ b/ext/fileinfo/fileinfo.c
@@ -444,7 +444,7 @@ static void _php_finfo_get_type(INTERNAL_FUNCTION_PARAMETERS, int mode, int mime
                                goto clean;
                        }
                        if (CHECK_NULL_PATH(buffer, buffer_len)) {
-                               zend_argument_type_error(1, "must not contain null bytes");
+                               zend_argument_type_error(1, "must not contain any null bytes");
                                goto clean;
                        }
 

diff --git a/ext/fileinfo/tests/finfo_file_001.phpt b/ext/fileinfo/tests/finfo_file_001.phpt
index 726103e02d666f750c03b9d15637a63fb8c0574e..71d88bbd43f03627c2f53673330c9018010b52e8 100644 (file)
--- a/ext/fileinfo/tests/finfo_file_001.phpt
+++ b/ext/fileinfo/tests/finfo_file_001.phpt
@@ -26,7 +26,7 @@ var_dump(finfo_file($fp, '&'));
 
 ?>
 --EXPECTF--
-finfo_file(): Argument #1 ($finfo) must not contain null bytes
+finfo_file(): Argument #1 ($finfo) must not contain any null bytes
 finfo_file(): Argument #1 ($finfo) cannot be empty
 finfo_file(): Argument #1 ($finfo) cannot be empty
 string(9) "directory"

diff --git a/ext/fileinfo/tests/finfo_file_basic.phpt b/ext/fileinfo/tests/finfo_file_basic.phpt
index 08c296ac8be3b4e328dbc6d4818d661b4728dfef..821405b48cd392619b0c7d0f75c11ea72e74df15 100644 (file)
--- a/ext/fileinfo/tests/finfo_file_basic.phpt
+++ b/ext/fileinfo/tests/finfo_file_basic.phpt
@@ -25,4 +25,4 @@ try {
 string(28) "text/x-php; charset=us-ascii"
 string(22) "PHP script, ASCII text"
 string(25) "text/plain; charset=utf-8"
-finfo_file(): Argument #1 ($finfo) must not contain null bytes
+finfo_file(): Argument #1 ($finfo) must not contain any null bytes

diff --git a/ext/fileinfo/tests/mime_content_type_001.phpt b/ext/fileinfo/tests/mime_content_type_001.phpt
index 9e3cb603986cddb93b209b33bc7ad977b29db771..76897cca7f9a6d7f1af3fa7d68ef133d21dc36dd 100644 (file)
--- a/ext/fileinfo/tests/mime_content_type_001.phpt
+++ b/ext/fileinfo/tests/mime_content_type_001.phpt
@@ -48,4 +48,4 @@ mime_content_type(): Argument #2 must be of type resource|string, array given
 
 Warning: mime_content_type(foo/inexistent): Failed to open stream: No such file or directory in %s on line %d
 mime_content_type(): Argument #1 ($filename) cannot be empty
-mime_content_type(): Argument #1 ($filename) must not contain null bytes
+mime_content_type(): Argument #1 ($filename) must not contain any null bytes

diff --git a/ext/sqlite3/sqlite3.c b/ext/sqlite3/sqlite3.c
index 464df5ae21aa5b8041bb3210d91cf0d2a21dbf08..cd91e68fd3e9a0257a21a499260a2d1dc6a0a693 100644 (file)
--- a/ext/sqlite3/sqlite3.c
+++ b/ext/sqlite3/sqlite3.c
@@ -1243,12 +1243,7 @@ PHP_METHOD(SQLite3, openBlob)
 
        db_obj = Z_SQLITE3_DB_P(object);
 
-       if (zend_parse_parameters(ZEND_NUM_ARGS(), "ssl|sl", &table, &table_len, &column, &column_len, &rowid, &dbname, &dbname_len, &flags) == FAILURE) {
-               RETURN_THROWS();
-       }
-
-       if (ZEND_NUM_ARGS() >= 4 && CHECK_NULL_PATH(dbname, dbname_len)) {
-               zend_argument_type_error(4, "must not contain null bytes");
+       if (zend_parse_parameters(ZEND_NUM_ARGS(), "ssl|pl", &table, &table_len, &column, &column_len, &rowid, &dbname, &dbname_len, &flags) == FAILURE) {
                RETURN_THROWS();
        }
 
@@ -1346,17 +1341,7 @@ PHP_METHOD(SQLite3, backup)
        sqlite3_backup *dbBackup;
        int rc; // Return code
 
-       if (zend_parse_parameters(ZEND_NUM_ARGS(), "O|ss", &destination_zval, php_sqlite3_sc_entry, &source_dbname, &source_dbname_length, &destination_dbname, &destination_dbname_length) == FAILURE) {
-               RETURN_THROWS();
-       }
-
-       if (ZEND_NUM_ARGS() >= 2 && CHECK_NULL_PATH(source_dbname, source_dbname_length)) {
-               zend_argument_type_error(2, "must not contain null bytes");
-               RETURN_THROWS();
-       }
-
-       if (ZEND_NUM_ARGS() >= 3 && CHECK_NULL_PATH(destination_dbname, destination_dbname_length)) {
-               zend_argument_type_error(3, "must not contain null bytes");
+       if (zend_parse_parameters(ZEND_NUM_ARGS(), "O|pp", &destination_zval, php_sqlite3_sc_entry, &source_dbname, &source_dbname_length, &destination_dbname, &destination_dbname_length) == FAILURE) {
                RETURN_THROWS();
        }