Skip to content

Convert warnings to exceptions in standard lib (reopened)#5004

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
kocsismate wants to merge 16 commits into from
Closed

Convert warnings to exceptions in standard lib (reopened) #5004

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
e6b3915
Promote warning to exception in ini_get_all() function
kocsismate Nov 20, 2019
80250da
Promote warning to exception in parse_ini_file() function
kocsismate Nov 20, 2019
ae0b1f5
Promote warnings to exceptions in dl() function
kocsismate Nov 20, 2019
0a0e75e
Promote warnings to exceptions in password_*() functions
kocsismate Nov 20, 2019
13d8ec6
Promote warning to exception in get_browser() function
kocsismate Nov 20, 2019
2f08249
Promote warning to exception in dns_check_record() function
kocsismate Nov 20, 2019
e1eb864
Promote warning to exception in convert_uudecode() function
kocsismate Nov 20, 2019
f289483
Promote warrnings to exceptions in chgrp() function
kocsismate Nov 20, 2019
f38e877
Promote warning to exception in constant() function
kocsismate Nov 20, 2019
c992c01
Promote warnings to exceptions in proc_open() function
kocsismate Nov 20, 2019
6a4bd43
Promote warning to exception in the levenshtein() function
kocsismate Nov 20, 2019
f53e365
Promote warnings to exceptions in some URL and file related functions
kocsismate Nov 20, 2019
761842d
Promote warnings to exceptions in string search related functions
kocsismate Nov 20, 2019
1bb354d
Promote warning to exception in the convert_uudecode() function
kocsismate Nov 20, 2019
30f05c1
Promote warnings to exceptions in register_shutdown_handler() function
kocsismate Nov 20, 2019
c90cce3
Promote warnings to exceptions in register_tick_function() function
kocsismate Nov 20, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 13 additions & 5 deletions Zend/tests/018.phpt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -3,19 +3,27 @@ constant() tests
--FILE--
<?php

var_dump(constant(""));

define("TEST_CONST", 1);
var_dump(constant("TEST_CONST"));
try {
var_dump(constant(""));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

try {
constant('::');
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}

define("TEST_CONST2", "test");
var_dump(constant("TEST_CONST2"));

echo "Done\n";
?>
--EXPECTF--
Warning: constant(): Couldn't find constant in %s on line %d
NULL
int(1)
Couldn't find constant
Couldn't find constant ::
string(4) "test"
Done
10 changes: 6 additions & 4 deletions Zend/tests/bug41026.phpt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -7,7 +7,7 @@ class try_class
{
static public function main ()
{
register_shutdown_function(array ("self", "on_shutdown"));
register_shutdown_function(array ("self", "on_shutdown"));
}

static public function on_shutdown ()
Expand All@@ -16,11 +16,13 @@ class try_class
}
}

try_class::main ();

try_class::main();
echo "Done\n";
?>
--EXPECT--
Done

Warning: (Registered shutdown functions) Unable to call self::on_shutdown() - function does not exist in Unknown on line 0
Fatal error: Uncaught ValueError: (Registered shutdown functions) Unable to call self::on_shutdown() - function does not exist in [no active file]:0
Stack trace:
#0 {main}
thrown in [no active file] on line 0
11 changes: 7 additions & 4 deletions Zend/tests/bug51791.phpt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,9 +6,12 @@ Bug #51791 (constant() failed to check undefined constant and php interpreter st
class A {
const B = 1;
}
var_dump(constant('A::B1'));

try {
constant('A::B1');
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
?>
--EXPECTF--
Warning: constant(): Couldn't find constant A::B1 in %s on line %d
NULL
--EXPECT--
Couldn't find constant A::B1
14 changes: 6 additions & 8 deletions ext/iconv/tests/iconv_strpos.phpt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,17 +6,16 @@ iconv_strpos()
iconv.internal_charset=ISO-8859-1
--FILE--
<?php
function my_error_handler($errno, $errmsg, $filename, $linenum)
{
echo "$errno: $errmsg\n";
}
set_error_handler('my_error_handler');
function foo($haystk, $needle, $offset, $to_charset = false, $from_charset = false)
{
if ($from_charset !== false) {
$haystk = iconv($from_charset, $to_charset, $haystk);
}
var_dump(strpos($haystk, $needle, $offset));
try {
var_dump(strpos($haystk, $needle, $offset));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
if ($to_charset !== false) {
var_dump(iconv_strpos($haystk, $needle, $offset, $to_charset));
} else {
Expand All@@ -42,8 +41,7 @@ bool(false)
bool(false)
int(5)
int(5)
2: %s
bool(false)
Offset not contained in string
bool(false)
int(7)
int(7)
Expand Down
14 changes: 7 additions & 7 deletions ext/mbstring/tests/bug43841.phpt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -30,7 +30,11 @@ foreach ($offsets as $i) {
echo "mb_strrpos:\n";
var_dump(mb_strrpos('This is na English ta', 'a', $i));
echo "strrpos:\n";
var_dump(strrpos('This is na English ta', 'a', $i));
try {
var_dump(strrpos('This is na English ta', 'a', $i));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
}
?>
--EXPECTF--
Expand All@@ -45,9 +49,7 @@ mb_strrpos:
Warning: mb_strrpos(): Offset is greater than the length of haystack string in %s on line %d
bool(false)
strrpos:

Warning: strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

-- Offset is -24 --
Multibyte String:
Expand All@@ -60,9 +62,7 @@ mb_strrpos:
Warning: mb_strrpos(): Offset is greater than the length of haystack string in %s on line %d
bool(false)
strrpos:

Warning: strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

-- Offset is -13 --
Multibyte String:
Expand Down
38 changes: 13 additions & 25 deletions ext/mbstring/tests/bug45923.phpt
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -10,7 +10,11 @@ function section($func, $haystack, $needle)
echo "\n------- $func -----------\n\n";
foreach(array(0, 3, 6, 9, 11, 12, -1, -3, -6, -20) as $offset) {
echo "> Offset: $offset\n";
var_dump($func($haystack,$needle,$offset));
try {
var_dump($func($haystack,$needle,$offset));
} catch (ValueError $exception) {
echo $exception->getMessage() . "\n";
}
}
}

Expand DownExpand Up@@ -40,19 +44,15 @@ bool(false)
> Offset: 11
bool(false)
> Offset: 12

Warning: strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
> Offset: -1
bool(false)
> Offset: -3
int(8)
> Offset: -6
int(8)
> Offset: -20

Warning: strpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

------- mb_strpos -----------

Expand DownExpand Up@@ -94,19 +94,15 @@ bool(false)
> Offset: 11
bool(false)
> Offset: 12

Warning: stripos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
> Offset: -1
bool(false)
> Offset: -3
int(8)
> Offset: -6
int(8)
> Offset: -20

Warning: stripos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

------- mb_stripos -----------

Expand DownExpand Up@@ -148,19 +144,15 @@ bool(false)
> Offset: 11
bool(false)
> Offset: 12

Warning: strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
> Offset: -1
int(8)
> Offset: -3
int(8)
> Offset: -6
int(4)
> Offset: -20

Warning: strrpos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

------- mb_strrpos -----------

Expand DownExpand Up@@ -202,19 +194,15 @@ bool(false)
> Offset: 11
bool(false)
> Offset: 12

Warning: strripos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string
> Offset: -1
int(8)
> Offset: -3
int(8)
> Offset: -6
int(4)
> Offset: -20

Warning: strripos(): Offset not contained in string in %s on line %d
bool(false)
Offset not contained in string

------- mb_strripos -----------

Expand Down
2 changes: 1 addition & 1 deletion ext/opcache/Optimizer/zend_func_info.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -295,7 +295,7 @@ static const func_info_t func_infos[] = {
F1("highlight_string", MAY_BE_FALSE | MAY_BE_TRUE | MAY_BE_STRING),
F1("php_strip_whitespace", MAY_BE_STRING),
FN("ini_get", MAY_BE_FALSE | MAY_BE_STRING),
F1("ini_get_all", MAY_BE_FALSE | MAY_BE_ARRAY | MAY_BE_ARRAY_KEY_STRING | MAY_BE_ARRAY_OF_NULL | MAY_BE_ARRAY_OF_STRING | MAY_BE_ARRAY_OF_ARRAY),
F1("ini_get_all", MAY_BE_ARRAY | MAY_BE_ARRAY_KEY_STRING | MAY_BE_ARRAY_OF_NULL | MAY_BE_ARRAY_OF_STRING | MAY_BE_ARRAY_OF_ARRAY),
FN("ini_set", MAY_BE_FALSE | MAY_BE_STRING),
F1("ini_alter", MAY_BE_FALSE | MAY_BE_STRING),
F1("get_include_path", MAY_BE_FALSE | MAY_BE_STRING),
Expand Down
21 changes: 10 additions & 11 deletions ext/standard/basic_functions.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -1700,7 +1700,7 @@ PHP_FUNCTION(constant)
}
} else {
if (!EG(exception)) {
php_error_docref(NULL, E_WARNING, "Couldn't find constant %s", ZSTR_VAL(const_name));
zend_value_error("Couldn't find constant %s", ZSTR_VAL(const_name));
}
RETURN_NULL();
}
Expand DownExpand Up@@ -2840,7 +2840,7 @@ static int user_shutdown_function_call(zval *zv) /* {{{ */
if (!zend_is_callable(&shutdown_function_entry->arguments[0], 0, NULL)) {
zend_string *function_name
= zend_get_callable_name(&shutdown_function_entry->arguments[0]);
php_error(E_WARNING, "(Registered shutdown functions) Unable to call %s() - function does not exist", ZSTR_VAL(function_name));
zend_value_error("(Registered shutdown functions) Unable to call %s() - function does not exist", ZSTR_VAL(function_name));
zend_string_release_ex(function_name, 0);
return 0;
}
Expand DownExpand Up@@ -2878,15 +2878,15 @@ static void user_tick_function_call(user_tick_function_entry *tick_fe) /* {{{ */
zval *obj, *method;

if (Z_TYPE_P(function) == IS_STRING) {
php_error_docref(NULL, E_WARNING, "Unable to call %s() - function does not exist", Z_STRVAL_P(function));
zend_value_error("Unable to call %s() - function does not exist", Z_STRVAL_P(function));
} else if ( Z_TYPE_P(function) == IS_ARRAY
&& (obj = zend_hash_index_find(Z_ARRVAL_P(function), 0)) != NULL
&& (method = zend_hash_index_find(Z_ARRVAL_P(function), 1)) != NULL
&& Z_TYPE_P(obj) == IS_OBJECT
&& Z_TYPE_P(method) == IS_STRING) {
php_error_docref(NULL, E_WARNING, "Unable to call %s::%s() - function does not exist", ZSTR_VAL(Z_OBJCE_P(obj)->name), Z_STRVAL_P(method));
zend_value_error("Unable to call %s::%s() - function does not exist", ZSTR_VAL(Z_OBJCE_P(obj)->name), Z_STRVAL_P(method));
} else {
php_error_docref(NULL, E_WARNING, "Unable to call tick function");
zend_value_error("Unable to call tick function");
}
}

Expand DownExpand Up@@ -3194,7 +3194,7 @@ PHP_FUNCTION(ini_get)
}
/* }}} */

/* {{{ proto array|false ini_get_all([string extension[, bool details = true]])
/* {{{ proto array ini_get_all([string extension[, bool details = true]])
Get all configuration options */
PHP_FUNCTION(ini_get_all)
{
Expand All@@ -3205,7 +3205,6 @@ PHP_FUNCTION(ini_get_all)
zend_string *key;
zend_ini_entry *ini_entry;


ZEND_PARSE_PARAMETERS_START(0, 2)
Z_PARAM_OPTIONAL
Z_PARAM_STRING_OR_NULL(extname, extname_len)
Expand All@@ -3216,8 +3215,8 @@ PHP_FUNCTION(ini_get_all)

if (extname) {
if ((module = zend_hash_str_find_ptr(&module_registry, extname, extname_len)) == NULL) {
php_error_docref(NULL, E_WARNING, "Unable to find extension '%s'", extname);
RETURN_FALSE;
zend_value_error("Unable to find extension '%s'", extname);
return;
}
module_number = module->module_number;
}
Expand DownExpand Up@@ -3853,8 +3852,8 @@ PHP_FUNCTION(parse_ini_file)
ZEND_PARSE_PARAMETERS_END();

if (filename_len == 0) {
php_error_docref(NULL, E_WARNING, "Filename cannot be empty!");
RETURN_FALSE;
zend_value_error("Filename cannot be empty!");
return;

@nikicnikicDec 20, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmb69 What do you think about moving this check into Z_PARAM_PATH? It seems to be a pretty common check (and it's also checked in php_stream_open_wrapper), but we probably don't check it everywhere we should be checking it. That would make sure it's treated consistently.

(Conversely: Are there any cases where an empty path should be accepted?)

@cmb69cmb69Dec 20, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are at least some cases where an empty path should be accepted, e.g. for SQLite3::open() (where an empty path is supposed to create a temporary on-disk database). Generally, I think that empty paths should be forbidden right away nonetheless. Maybe change Z_PARAM_PATH to reject empty paths, and cater to the few exceptions by checking for a string and adding a manual NUL byte check? Or maybe introduce another macro (say, Z_PARAM_PATH_MAYBE_EMPTY)? The latter won't work with classic ZPP, though (don't think that would a problem though).

}

/* Set callback function */
Expand Down
2 changes: 1 addition & 1 deletion ext/standard/basic_functions.stub.php
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -311,7 +311,7 @@ function highlight_string(string $string, bool $return = false): string|bool|nul

function ini_get(string $varname): string|false {}

function ini_get_all(?string $extension = null, bool $details = true): array|false {}
function ini_get_all(?string $extension = null, bool $details = true): array {}

function ini_set(string $varname, string $value): string|false {}

Expand Down
2 changes: 1 addition & 1 deletion ext/standard/basic_functions_arginfo.h
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -476,7 +476,7 @@ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_ini_get, 0, 1, MAY_BE_STRING|MAY
ZEND_ARG_TYPE_INFO(0, varname, IS_STRING, 0)
ZEND_END_ARG_INFO()

ZEND_BEGIN_ARG_WITH_RETURN_TYPE_MASK_EX(arginfo_ini_get_all, 0, 0, MAY_BE_ARRAY|MAY_BE_FALSE)
ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_ini_get_all, 0, 0, IS_ARRAY, 0)
ZEND_ARG_TYPE_INFO(0, extension, IS_STRING, 1)
ZEND_ARG_TYPE_INFO(0, details, _IS_BOOL, 0)
ZEND_END_ARG_INFO()
Expand Down
4 changes: 2 additions & 2 deletions ext/standard/browscap.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -701,8 +701,8 @@ PHP_FUNCTION(get_browser)
}
} else {
if (!global_bdata.htab) {
php_error_docref(NULL, E_WARNING, "browscap ini directive not set");
RETURN_FALSE;
zend_throw_error(NULL, "Browscap ini directive not set");
return;

@nikicnikicDec 20, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imho this is again an environment issue: Code may have to deal with the fact that browscap is not available.

}
bdata = &global_bdata;
}
Expand Down
10 changes: 5 additions & 5 deletions ext/standard/dl.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -50,13 +50,13 @@ PHPAPI PHP_FUNCTION(dl)
ZEND_PARSE_PARAMETERS_END();

if (!PG(enable_dl)) {
php_error_docref(NULL, E_WARNING, "Dynamically loaded extensions aren't enabled");
RETURN_FALSE;
zend_throw_error(NULL, "Dynamically loaded extensions aren't enabled");
return;

@nikicnikicDec 20, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar here, environment issue.

}

if (filename_len >= MAXPATHLEN) {
php_error_docref(NULL, E_WARNING, "File name exceeds the maximum allowed length of %d characters", MAXPATHLEN);
RETURN_FALSE;
zend_value_error("File name exceeds the maximum allowed length of %d characters", MAXPATHLEN);
return;
}

php_dl(filename, MODULE_TEMPORARY, return_value, 0);
Expand DownExpand Up@@ -265,7 +265,7 @@ PHP_MINFO_FUNCTION(dl)

PHPAPI void php_dl(char *file, int type, zval *return_value, int start_now)
{
php_error_docref(NULL, E_WARNING, "Cannot dynamically load %s - dynamic modules are not supported", file);
zend_throw_error(NULL, "Cannot dynamically load %s - dynamic modules are not supported", file);
RETVAL_FALSE;
}

Expand Down
4 changes: 2 additions & 2 deletions ext/standard/dns.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -383,8 +383,8 @@ PHP_FUNCTION(dns_check_record)
ZEND_PARSE_PARAMETERS_END();

if (hostname_len == 0) {
php_error_docref(NULL, E_WARNING, "Host cannot be empty");
RETURN_FALSE;
zend_value_error("Host cannot be empty");
return;

@nikicnikicDec 20, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine :)

}

if (rectype) {
Expand Down
Loading