Skip to content

Commit 186788f

Browse files
committed
Fix error handling in tidy constructor
1 parent 1d045a5 commit 186788f

File tree

6 files changed

+37
-12
lines changed

6 files changed

+37
-12
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,10 @@ PHP NEWS
3434
. Fix references in request_parse_body() options array. (nielsdos)
3535
. Add RoundingMode enum. (timwolla, saki)
3636

37+
- Tidy:
38+
. Failures in the constructor now throw exceptions rather than emitting
39+
warnings and having a broken object. (nielsdos)
40+
3741
- XSL:
3842
. Fix trampoline leak in xpath callables. (nielsdos)
3943

UPGRADING

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,10 @@ PHP 8.4 UPGRADE NOTES
167167
. strcspn() with empty $characters now returns the length of the string instead
168168
of incorrectly stopping at the first NUL character. See GH-12592.
169169

170+
- Tidy:
171+
. Failures in the constructor now throw exceptions rather than emitting
172+
warnings and having a broken object.
173+
170174
- XML:
171175
. The xml_set_*_handler() functions now declare and check for an effective
172176
signature of callable|string|null for the $handler parameters.

ext/tidy/tests/bug54682.phpt

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,15 @@ tidy
55
--FILE--
66
<?php
77

8-
$nx = new Tidy("*");
8+
$nx = new Tidy();
99
$nx->diagnose();
10+
var_dump($nx);
1011

1112
?>
1213
--EXPECTF--
13-
Warning: tidy::__construct(): Cannot load "*" into memory%win %s on line %d
14+
object(tidy)#%d (2) {
15+
["errorBuffer"]=>
16+
NULL
17+
["value"]=>
18+
NULL
19+
}

ext/tidy/tests/open_basedir_failure_config.phpt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ echo "=== tidy_parse_file ===\n";
1717
tidy_parse_file(__DIR__.'/open_basedir/test.html', 'my_config_file.ini');
1818

1919
echo "=== __construct ===\n";
20-
$tidy = new tidy(__DIR__.'/open_basedir/test.html', 'my_config_file.ini');
20+
try {
21+
$tidy = new tidy(__DIR__.'/open_basedir/test.html', 'my_config_file.ini');
22+
} catch (Exception $e) {
23+
echo $e->getMessage(), "\n";
24+
}
2125

2226
echo "=== parseFile ===\n";
2327
$tidy = new tidy;
@@ -38,8 +42,7 @@ Warning: tidy_parse_string(): open_basedir restriction in effect. File(my_config
3842

3943
Warning: tidy_parse_file(): open_basedir restriction in effect. File(my_config_file.ini) is not within the allowed path(s): (%sopen_basedir) in %s on line %d
4044
=== __construct ===
41-
42-
Warning: tidy::__construct(): open_basedir restriction in effect. File(my_config_file.ini) is not within the allowed path(s): (%sopen_basedir) in %s on line %d
45+
tidy::__construct(): open_basedir restriction in effect. File(my_config_file.ini) is not within the allowed path(s): (%sopen_basedir)
4346
=== parseFile ===
4447

4548
Warning: tidy::parseFile(): open_basedir restriction in effect. File(my_config_file.ini) is not within the allowed path(s): (%sopen_basedir) in %s on line %d

ext/tidy/tests/parsing_inexistent_file.phpt

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,16 @@ var_dump($tidy->parseFile("does_not_exist.html"));
1010

1111
var_dump(tidy_parse_file("does_not_exist.html"));
1212

13-
$tidy = new tidy("does_not_exist.html");
13+
try {
14+
$tidy = new tidy("does_not_exist.html");
15+
} catch (Exception $e) {
16+
echo $e->getMessage(), "\n";
17+
}
1418
?>
1519
--EXPECTF--
1620
Warning: tidy::parseFile(): Cannot load "does_not_exist.html" into memory in %s on line %d
1721
bool(false)
1822

1923
Warning: tidy_parse_file(): Cannot load "does_not_exist.html" into memory in %s on line %d
2024
bool(false)
21-
22-
Warning: tidy::__construct(): Cannot load "does_not_exist.html" into memory in %s on line %d
25+
Cannot load "does_not_exist.html" into memory

ext/tidy/tidy.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@
4040

4141
#include "tidy_arginfo.h"
4242

43+
#include "Zend/zend_exceptions.h"
44+
4345
/* compatibility with older versions of libtidy */
4446
#ifndef TIDY_CALL
4547
#define TIDY_CALL
@@ -1371,8 +1373,8 @@ PHP_METHOD(tidy, __construct)
13711373

13721374
if (inputfile) {
13731375
if (!(contents = php_tidy_file_to_mem(ZSTR_VAL(inputfile), use_include_path))) {
1374-
php_error_docref(NULL, E_WARNING, "Cannot load \"%s\" into memory%s", ZSTR_VAL(inputfile), (use_include_path) ? " (using include path)" : "");
1375-
return;
1376+
zend_throw_error(zend_ce_exception, "Cannot load \"%s\" into memory%s", ZSTR_VAL(inputfile), (use_include_path) ? " (using include path)" : "");
1377+
RETURN_THROWS();
13761378
}
13771379

13781380
if (ZEND_SIZE_T_UINT_OVFL(ZSTR_LEN(contents))) {
@@ -1381,11 +1383,14 @@ PHP_METHOD(tidy, __construct)
13811383
RETURN_THROWS();
13821384
}
13831385

1386+
zend_error_handling error_handling;
1387+
zend_replace_error_handling(EH_THROW, NULL, &error_handling);
13841388
if (php_tidy_apply_config(obj->ptdoc->doc, options_str, options_ht) != SUCCESS) {
1385-
/* TODO: this is the constructor, we should throw probably... */
1389+
zend_restore_error_handling(&error_handling);
13861390
zend_string_release_ex(contents, 0);
1387-
RETURN_FALSE;
1391+
RETURN_THROWS();
13881392
}
1393+
zend_restore_error_handling(&error_handling);
13891394

13901395
php_tidy_parse_string(obj, ZSTR_VAL(contents), (uint32_t)ZSTR_LEN(contents), enc);
13911396

0 commit comments

Comments
 (0)