From f592f75e9c42c5b5758da3cf68b442bc408e18db Mon Sep 17 00:00:00 2001 From: ndossche Date: Mon, 20 Feb 2023 14:32:45 +0100 Subject: [PATCH 1/2] Add missing error check on tidyLoadConfig Parse errors were not reported for the default config, they were only reported when explicitly another config was loaded. This means that users may not be aware of errors in their configuration and therefore the behaviour of Tidy might not be what they intended. This patch fixes that issue by using a common function. In fact, the check for -1 might be enough for the current implementation of Tidy, but the Tidy docs say that any value other than 0 indicates an error. So future errors might not be caught when just using an error code of -1. Therefore, this also changes the error code checks of == -1 to < 0 and == 1 to > 0. Closes GH-10636 Signed-off-by: George Peter Banyard --- NEWS | 1 + ext/tidy/tests/019.phpt | 6 +++--- ext/tidy/tests/gh10636.phpt | 12 ++++++++++++ ext/tidy/tests/gh10636_config | 1 + ext/tidy/tidy.c | 23 ++++++++++++----------- 5 files changed, 29 insertions(+), 14 deletions(-) create mode 100644 ext/tidy/tests/gh10636.phpt create mode 100644 ext/tidy/tests/gh10636_config diff --git a/NEWS b/NEWS index 26313b462f9..50476cd4fa6 100644 --- a/NEWS +++ b/NEWS @@ -63,6 +63,7 @@ PHP NEWS - Tidy: . Fix memory leaks when attempting to open a non-existing file or a file over 4GB. (Girgias) + . Add missing error check on tidyLoadConfig. (nielsdos) 14 Feb 2023, PHP 8.1.16 diff --git a/ext/tidy/tests/019.phpt b/ext/tidy/tests/019.phpt index 20a15c7018c..444ecc155a4 100644 --- a/ext/tidy/tests/019.phpt +++ b/ext/tidy/tests/019.phpt @@ -28,13 +28,13 @@ tidy_repair_file($l, $l, $l ,$l); // This doesn't emit any warning, TODO look in echo "Done\n"; ?> --EXPECTF-- -Warning: tidy_repair_string(): Could not load configuration file "1" in %s on line %d +Warning: tidy_repair_string(): Could not load the Tidy configuration file "1" in %s on line %d Warning: tidy_repair_string(): Could not set encoding "1" in %s on line %d -Warning: tidy_repair_string(): Could not load configuration file "" in %s on line %d +Warning: tidy_repair_string(): Could not load the Tidy configuration file "" in %s on line %d -Warning: tidy_repair_string(): Could not load configuration file "1" in %s on line %d +Warning: tidy_repair_string(): Could not load the Tidy configuration file "1" in %s on line %d Warning: tidy_repair_string(): Could not set encoding "1" in %s on line %d Path cannot be empty diff --git a/ext/tidy/tests/gh10636.phpt b/ext/tidy/tests/gh10636.phpt new file mode 100644 index 00000000000..ce0aa3c08d1 --- /dev/null +++ b/ext/tidy/tests/gh10636.phpt @@ -0,0 +1,12 @@ +--TEST-- +GH-10636 (Tidy does not output notice when it encountered parse errors in the default configuration file) +--EXTENSIONS-- +tidy +--INI-- +tidy.default_config={PWD}/gh10636_config +--FILE-- + +--EXPECTF-- +Notice: main(): There were errors while parsing the Tidy configuration file "%sgh10636_config" in %s on line %d diff --git a/ext/tidy/tests/gh10636_config b/ext/tidy/tests/gh10636_config new file mode 100644 index 00000000000..d9c4755a2d7 --- /dev/null +++ b/ext/tidy/tests/gh10636_config @@ -0,0 +1 @@ +indent:@ diff --git a/ext/tidy/tidy.c b/ext/tidy/tidy.c index 1d757f44fae..5e9961963e2 100644 --- a/ext/tidy/tidy.c +++ b/ext/tidy/tidy.c @@ -79,14 +79,7 @@ _php_tidy_apply_config_array(_doc, _val_ht); \ } else if (_val_str) { \ TIDY_OPEN_BASE_DIR_CHECK(ZSTR_VAL(_val_str)); \ - switch (tidyLoadConfig(_doc, ZSTR_VAL(_val_str))) { \ - case -1: \ - php_error_docref(NULL, E_WARNING, "Could not load configuration file \"%s\"", ZSTR_VAL(_val_str)); \ - break; \ - case 1: \ - php_error_docref(NULL, E_NOTICE, "There were errors while parsing the configuration file \"%s\"", ZSTR_VAL(_val_str)); \ - break; \ - } \ + php_tidy_load_config(_doc, ZSTR_VAL(_val_str)); \ } @@ -143,9 +136,7 @@ if (php_check_open_basedir(filename)) { \ #define TIDY_SET_DEFAULT_CONFIG(_doc) \ if (TG(default_config) && TG(default_config)[0]) { \ - if (tidyLoadConfig(_doc, TG(default_config)) < 0) { \ - php_error_docref(NULL, E_WARNING, "Unable to load Tidy configuration file at \"%s\"", TG(default_config)); \ - } \ + php_tidy_load_config(_doc, TG(default_config)); \ } /* }}} */ @@ -269,6 +260,16 @@ static void TIDY_CALL php_tidy_panic(ctmbstr msg) php_error_docref(NULL, E_ERROR, "Could not allocate memory for tidy! (Reason: %s)", (char *)msg); } +static void php_tidy_load_config(TidyDoc doc, const char *path) +{ + int ret = tidyLoadConfig(doc, path); + if (ret < 0) { + php_error_docref(NULL, E_WARNING, "Could not load the Tidy configuration file \"%s\"", path); + } else if (ret > 0) { + php_error_docref(NULL, E_NOTICE, "There were errors while parsing the Tidy configuration file \"%s\"", path); + } +} + static int _php_tidy_set_tidy_opt(TidyDoc doc, char *optname, zval *value) { TidyOption opt = tidyGetOptionByName(doc, optname); From 8cac8306c3f5a8bcb1a147249737190e9f037577 Mon Sep 17 00:00:00 2001 From: ndossche Date: Mon, 20 Feb 2023 10:56:10 +0100 Subject: [PATCH 2/2] Fix incorrect error check in browsecap for pcre2_match() pcre2_match() returns error codes < 0, but only the "no match" error code was handled. Fix it by changing the check to >= 0. Closes GH-10632 Signed-off-by: George Peter Banyard --- NEWS | 1 + ext/standard/browscap.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index 50476cd4fa6..49be3090908 100644 --- a/NEWS +++ b/NEWS @@ -59,6 +59,7 @@ PHP NEWS . Fix incorrect check in cs_8559_5 in map_from_unicode(). (nielsdos) . Fix bug GH-9697 for reset/end/next/prev() attempting to move pointer of properties table for certain internal classes such as FFI classes + . Fix incorrect error check in browsecap for pcre2_match(). (nielsdos) - Tidy: . Fix memory leaks when attempting to open a non-existing file or a file over diff --git a/ext/standard/browscap.c b/ext/standard/browscap.c index 2b6cd4e63f6..490acac2f99 100644 --- a/ext/standard/browscap.c +++ b/ext/standard/browscap.c @@ -612,7 +612,7 @@ static int browser_reg_compare(browscap_entry *entry, zend_string *agent_name, b } rc = pcre2_match(re, (PCRE2_SPTR)ZSTR_VAL(agent_name), ZSTR_LEN(agent_name), 0, 0, match_data, php_pcre_mctx()); php_pcre_free_match_data(match_data); - if (PCRE2_ERROR_NOMATCH != rc) { + if (rc >= 0) { /* If we've found a possible browser, we need to do a comparison of the number of characters changed in the user agent being checked versus the previous match found and the current match. */