Skip to content

Commit

Permalink
Add missing error check on tidyLoadConfig
Browse files Browse the repository at this point in the history
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 <girgias@php.net>
  • Loading branch information
nielsdos authored and Girgias committed Feb 21, 2023
1 parent ed4dc39 commit f592f75
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 14 deletions.
1 change: 1 addition & 0 deletions NEWS
Expand Up @@ -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

Expand Down
6 changes: 3 additions & 3 deletions ext/tidy/tests/019.phpt
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions 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--
<?php
$a = new tidy(__DIR__."/007.html");
?>
--EXPECTF--
Notice: main(): There were errors while parsing the Tidy configuration file "%sgh10636_config" in %s on line %d
1 change: 1 addition & 0 deletions ext/tidy/tests/gh10636_config
@@ -0,0 +1 @@
indent:@
23 changes: 12 additions & 11 deletions ext/tidy/tidy.c
Expand Up @@ -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)); \
}


Expand Down Expand Up @@ -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)); \
}
/* }}} */

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit f592f75

Please sign in to comment.