From 266ebbf3860f90ad6f24895cb58324b855383b14 Mon Sep 17 00:00:00 2001 From: ndossche Date: Mon, 20 Feb 2023 14:32:45 +0100 Subject: [PATCH] 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. --- ext/tidy/tests/019.phpt | 6 +++--- ext/tidy/tests/gh10636.phpt | 12 ++++++++++++ ext/tidy/tests/gh10636_config | 1 + ext/tidy/tidy.c | 23 ++++++++++++----------- 4 files changed, 28 insertions(+), 14 deletions(-) create mode 100644 ext/tidy/tests/gh10636.phpt create mode 100644 ext/tidy/tests/gh10636_config diff --git a/ext/tidy/tests/019.phpt b/ext/tidy/tests/019.phpt index 20a15c7018ce2..444ecc155a45c 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 0000000000000..ce0aa3c08d1b2 --- /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 0000000000000..d9c4755a2d744 --- /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 1d757f44fae76..5e9961963e231 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);