Skip to content

Promote E_NOTICE to Error in PostgreSQL extension #6226

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
wants to merge 4 commits into from
Closed

Promote E_NOTICE to Error in PostgreSQL extension #6226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f31d08d
Promote E_NOTICE to Error in PostgreSQL extension
Girgias Sep 28, 2020
ae1001a
Tests + nits
Girgias Sep 28, 2020
e327070
Review
Girgias Sep 29, 2020
5ccf9c0
Indentation...
Girgias Sep 29, 2020
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
105 changes: 59 additions & 46 deletions ext/pgsql/pgsql.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2300,6 +2300,7 @@ PHP_FUNCTION(pg_lo_create)
RETURN_THROWS();
}

/* Overloaded method uses default link if arg 1 is not a resource, set oid pointer */
if ((argc == 1) && (Z_TYPE_P(pgsql_link) != IS_RESOURCE)) {
oid = pgsql_link;
pgsql_link = NULL;
Expand All @@ -2322,25 +2323,26 @@ PHP_FUNCTION(pg_lo_create)
switch (Z_TYPE_P(oid)) {
case IS_STRING:
{
/* TODO: Use subroutine? */
char *end_ptr;
wanted_oid = (Oid)strtoul(Z_STRVAL_P(oid), &end_ptr, 10);
if ((Z_STRVAL_P(oid)+Z_STRLEN_P(oid)) != end_ptr) {
/* wrong integer format */
php_error_docref(NULL, E_NOTICE, "Invalid OID value passed");
RETURN_FALSE;
/* wrong integer format */
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
}
break;
case IS_LONG:
if (Z_LVAL_P(oid) < (zend_long)InvalidOid) {
php_error_docref(NULL, E_NOTICE, "Invalid OID value passed");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
wanted_oid = (Oid)Z_LVAL_P(oid);
break;
default:
php_error_docref(NULL, E_NOTICE, "Invalid OID value passed");
RETURN_FALSE;
zend_type_error("OID value must be of type string|int, %s given", zend_zval_type_name(oid));
RETURN_THROWS();
}
if ((pgsql_oid = lo_create(pgsql, wanted_oid)) == InvalidOid) {
php_error_docref(NULL, E_WARNING, "Unable to create PostgreSQL large object");
Expand Down Expand Up @@ -2374,39 +2376,41 @@ PHP_FUNCTION(pg_lo_unlink)
/* accept string type since Oid type is unsigned int */
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"rs", &pgsql_link, &oid_string, &oid_strlen) == SUCCESS) {
/* TODO: Use subroutine? */
oid = (Oid)strtoul(oid_string, &end_ptr, 10);
if ((oid_string+oid_strlen) != end_ptr) {
/* wrong integer format */
php_error_docref(NULL, E_NOTICE, "Wrong OID value passed");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
link = Z_RES_P(pgsql_link);
}
else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"rl", &pgsql_link, &oid_long) == SUCCESS) {
if (oid_long <= (zend_long)InvalidOid) {
php_error_docref(NULL, E_NOTICE, "Invalid OID specified");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
oid = (Oid)oid_long;
link = Z_RES_P(pgsql_link);
}
else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"s", &oid_string, &oid_strlen) == SUCCESS) {
/* TODO: subroutine? */
oid = (Oid)strtoul(oid_string, &end_ptr, 10);
if ((oid_string+oid_strlen) != end_ptr) {
/* wrong integer format */
php_error_docref(NULL, E_NOTICE, "Wrong OID value passed");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
link = FETCH_DEFAULT_LINK();
CHECK_DEFAULT_LINK(link);
}
else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"l", &oid_long) == SUCCESS) {
if (oid_long <= (zend_long)InvalidOid) {
php_error_docref(NULL, E_NOTICE, "Invalid OID is specified");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
oid = (Oid)oid_long;
link = FETCH_DEFAULT_LINK();
Expand Down Expand Up @@ -2447,39 +2451,41 @@ PHP_FUNCTION(pg_lo_open)
/* accept string type since Oid is unsigned int */
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"rss", &pgsql_link, &oid_string, &oid_strlen, &mode_string, &mode_strlen) == SUCCESS) {
/* TODO: Use subroutine? */
oid = (Oid)strtoul(oid_string, &end_ptr, 10);
if ((oid_string+oid_strlen) != end_ptr) {
/* wrong integer format */
php_error_docref(NULL, E_NOTICE, "Wrong OID value passed");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
link = Z_RES_P(pgsql_link);
}
else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"rls", &pgsql_link, &oid_long, &mode_string, &mode_strlen) == SUCCESS) {
if (oid_long <= (zend_long)InvalidOid) {
php_error_docref(NULL, E_NOTICE, "Invalid OID specified");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
oid = (Oid)oid_long;
link = Z_RES_P(pgsql_link);
}
else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"ss", &oid_string, &oid_strlen, &mode_string, &mode_strlen) == SUCCESS) {
/* TODO: Use subroutine? */
oid = (Oid)strtoul(oid_string, &end_ptr, 10);
if ((oid_string+oid_strlen) != end_ptr) {
/* wrong integer format */
php_error_docref(NULL, E_NOTICE, "Wrong OID value passed");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
link = FETCH_DEFAULT_LINK();
CHECK_DEFAULT_LINK(link);
}
else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"ls", &oid_long, &mode_string, &mode_strlen) == SUCCESS) {
if (oid_long <= (zend_long)InvalidOid) {
php_error_docref(NULL, E_NOTICE, "Invalid OID specified");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
oid = (Oid)oid_long;
link = FETCH_DEFAULT_LINK();
Expand Down Expand Up @@ -2717,25 +2723,26 @@ PHP_FUNCTION(pg_lo_import)
switch (Z_TYPE_P(oid)) {
case IS_STRING:
{
/* TODO: Use subroutine? */
char *end_ptr;
wanted_oid = (Oid)strtoul(Z_STRVAL_P(oid), &end_ptr, 10);
if ((Z_STRVAL_P(oid)+Z_STRLEN_P(oid)) != end_ptr) {
/* wrong integer format */
php_error_docref(NULL, E_NOTICE, "Invalid OID value passed");
RETURN_FALSE;
/* wrong integer format */
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
}
break;
case IS_LONG:
if (Z_LVAL_P(oid) < (zend_long)InvalidOid) {
php_error_docref(NULL, E_NOTICE, "Invalid OID value passed");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
wanted_oid = (Oid)Z_LVAL_P(oid);
break;
default:
php_error_docref(NULL, E_NOTICE, "Invalid OID value passed");
RETURN_FALSE;
zend_type_error("OID value must be of type string|int, %s given", zend_zval_type_name(oid));
RETURN_THROWS();
}

returned_oid = lo_import_with_oid(pgsql, file_in, wanted_oid);
Expand Down Expand Up @@ -2773,39 +2780,41 @@ PHP_FUNCTION(pg_lo_export)
if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"rlp", &pgsql_link, &oid_long, &file_out, &name_len) == SUCCESS) {
if (oid_long <= (zend_long)InvalidOid) {
php_error_docref(NULL, E_NOTICE, "Invalid OID specified");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
oid = (Oid)oid_long;
link = Z_RES_P(pgsql_link);
}
else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"rsp", &pgsql_link, &oid_string, &oid_strlen, &file_out, &name_len) == SUCCESS) {
/* TODO: Use subroutine? */
oid = (Oid)strtoul(oid_string, &end_ptr, 10);
if ((oid_string+oid_strlen) != end_ptr) {
/* wrong integer format */
php_error_docref(NULL, E_NOTICE, "Wrong OID value passed");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
link = Z_RES_P(pgsql_link);
}
else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"lp", &oid_long, &file_out, &name_len) == SUCCESS) {
if (oid_long <= (zend_long)InvalidOid) {
php_error_docref(NULL, E_NOTICE, "Invalid OID specified");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
oid = (Oid)oid_long;
link = FETCH_DEFAULT_LINK();
CHECK_DEFAULT_LINK(link);
}
else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc,
"sp", &oid_string, &oid_strlen, &file_out, &name_len) == SUCCESS) {
/* TODO: Use subroutine? */
oid = (Oid)strtoul(oid_string, &end_ptr, 10);
if ((oid_string+oid_strlen) != end_ptr) {
/* wrong integer format */
php_error_docref(NULL, E_NOTICE, "Wrong OID value passed");
RETURN_FALSE;
zend_value_error("Invalid OID value passed");
RETURN_THROWS();
}
link = FETCH_DEFAULT_LINK();
CHECK_DEFAULT_LINK(link);
Expand Down Expand Up @@ -4264,6 +4273,7 @@ PHP_PGSQL_API int php_pgsql_meta_data(PGconn *pg_link, const char *table_name, z
src = estrdup(table_name);
tmp_name = php_strtok_r(src, ".", &tmp_name2);
if (!tmp_name) {
// TODO ValueError (empty table name)?
efree(src);
php_error_docref(NULL, E_WARNING, "The table name must be specified");
return FAILURE;
Expand Down Expand Up @@ -4557,6 +4567,7 @@ static int php_pgsql_add_quotes(zval *src, zend_bool should_free)
}
/* }}} */

/* Raise E_NOTICE to E_WARNING or Error? */
#define PGSQL_CONV_CHECK_IGNORE() \
if (!err && Z_TYPE(new_val) == IS_STRING && !strcmp(Z_STRVAL(new_val), "NULL")) { \
/* if new_value is string "NULL" and field has default value, remove element to use default value */ \
Expand Down Expand Up @@ -4600,8 +4611,10 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
skip_field = 0;
ZVAL_NULL(&new_val);

/* TODO: Check when meta data can be broken and see if can use assertions instead */

if (!err && field == NULL) {
php_error_docref(NULL, E_WARNING, "Accepts only string key for values");
zend_value_error("Array of values must be an associative array with string keys");
err = 1;
}

Expand All @@ -4625,8 +4638,8 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
php_error_docref(NULL, E_NOTICE, "Detected broken meta data. Missing 'is enum'");
err = 1;
}
if (!err && (Z_TYPE_P(val) == IS_ARRAY || Z_TYPE_P(val) == IS_OBJECT)) {
php_error_docref(NULL, E_NOTICE, "Expects scalar values as field values");
if (!err && (Z_TYPE_P(val) == IS_ARRAY || Z_TYPE_P(val) == IS_OBJECT || Z_TYPE_P(val) == IS_RESOURCE)) {
zend_type_error("Values must be of type string|int|float|bool|null, %s given", zend_zval_type_name(val));
err = 1;
}
if (err) {
Expand All @@ -4641,6 +4654,7 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
data_type = php_pgsql_get_data_type(Z_STRVAL_P(type), Z_STRLEN_P(type));
}

/* TODO: Should E_NOTICE be converted to type error if PHP type cannot be converted to field type? */
switch(data_type)
{
case PG_BOOL:
Expand Down Expand Up @@ -5358,7 +5372,7 @@ PHP_PGSQL_API int php_pgsql_insert(PGconn *pg_link, const char *table, zval *var

ZEND_HASH_FOREACH_STR_KEY(Z_ARRVAL_P(var_array), fld) {
if (fld == NULL) {
php_error_docref(NULL, E_NOTICE, "Expects associative array for values to be inserted");
zend_value_error("Array of values must be an associative array with string keys");
goto cleanup;
}
if (opt & PGSQL_DML_ESCAPE) {
Expand Down Expand Up @@ -5401,9 +5415,8 @@ PHP_PGSQL_API int php_pgsql_insert(PGconn *pg_link, const char *table, zval *var
smart_str_appendl(&querystr, "NULL", sizeof("NULL")-1);
break;
default:
php_error_docref(NULL, E_WARNING, "Expects scaler values. type = %d", Z_TYPE_P(val));
zend_type_error("Value must be of type string|int|float|null, %s given", zend_zval_type_name(val));
goto cleanup;
break;
}
smart_str_appendc(&querystr, ',');
} ZEND_HASH_FOREACH_END();
Expand Down Expand Up @@ -5532,7 +5545,7 @@ static inline int build_assignment_string(PGconn *pg_link, smart_str *querystr,

ZEND_HASH_FOREACH_STR_KEY_VAL(ht, fld, val) {
if (fld == NULL) {
php_error_docref(NULL, E_NOTICE, "Expects associative array for values to be inserted");
zend_value_error("Array of values must be an associative array with string keys");
return -1;
}
if (opt & PGSQL_DML_ESCAPE) {
Expand Down Expand Up @@ -5573,7 +5586,7 @@ static inline int build_assignment_string(PGconn *pg_link, smart_str *querystr,
smart_str_appendl(querystr, "NULL", sizeof("NULL")-1);
break;
default:
php_error_docref(NULL, E_WARNING, "Expects scaler values. type=%d", Z_TYPE_P(val));
zend_type_error("Value must be of type string|int|float|null, %s given", zend_zval_type_name(val));
return -1;
}
smart_str_appendl(querystr, pad, pad_len);
Expand Down
26 changes: 26 additions & 0 deletions ext/pgsql/tests/05large_object.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,28 @@ if (!file_exists($path . 'php.gif.exported')) {
@unlink($path . 'php.gif.exported');
pg_query($db, 'commit');

/* invalid OID values */
try {
pg_lo_create(-15);
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}
try {
pg_lo_create($db, -15);
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}
try {
pg_lo_create('giberrish');
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}
try {
pg_lo_create($db, 'giberrish');
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}

echo "OK";
?>
--EXPECT--
Expand All @@ -79,4 +101,8 @@ unlink LO
Test without connection
Test with string oid value
import/export LO
Invalid OID value passed
Invalid OID value passed
Invalid OID value passed
Invalid OID value passed
OK
33 changes: 33 additions & 0 deletions ext/pgsql/tests/10pg_convert_9.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,34 @@ $fields = array('num'=>'1234', 'str'=>'AAA', 'bin'=>'BBB');
$converted = pg_convert($db, $table_name, $fields);

var_dump($converted);

/* Invalid values */
try {
$converted = pg_convert($db, $table_name, [5 => 'AAA']);
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}
try {
$converted = pg_convert($db, $table_name, ['AAA']);
} catch (\ValueError $e) {
echo $e->getMessage(), \PHP_EOL;
}
try {
$converted = pg_convert($db, $table_name, ['num' => []]);
} catch (\TypeError $e) {
echo $e->getMessage(), \PHP_EOL;
}
try {
$converted = pg_convert($db, $table_name, ['num' => new stdClass()]);
} catch (\TypeError $e) {
echo $e->getMessage(), \PHP_EOL;
}
try {
$converted = pg_convert($db, $table_name, ['num' => $db]);
var_dump($converted);
} catch (\TypeError $e) {
echo $e->getMessage(), \PHP_EOL;
}
?>
--EXPECT--
array(3) {
Expand All @@ -28,3 +56,8 @@ array(3) {
[""bin""]=>
string(12) "E'\\x424242'"
}
Array of values must be an associative array with string keys
Array of values must be an associative array with string keys
Values must be of type string|int|float|bool|null, array given
Values must be of type string|int|float|bool|null, stdClass given
Values must be of type string|int|float|bool|null, resource given
Loading