Skip to content

Promote warnings to Errors in PostgreSQL extension #6129

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 6 commits into from
Closed

Promote warnings to Errors in PostgreSQL extension #6129

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ec13381
Promote warnings to Errors in PostgreSQL extension
Girgias Sep 13, 2020
5f870f2
Drop TODO Comments
Girgias Sep 13, 2020
1920e45
Correct argnum, correct Oxford comma usage, fix incorrect space inden…
Girgias Sep 13, 2020
6dcec9a
Promote warning about length larger than buffer size
Girgias Sep 13, 2020
296866f
Change check to assertion as it is handled before calling said function
Girgias Sep 13, 2020
a37c41e
Stubs, test case, and indentation
Girgias Sep 14, 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
Next Next commit
Promote warnings to Errors in PostgreSQL extension
  • Loading branch information
@Girgias
Girgias committed Sep 14, 2020
commit ec13381c9cdd2284a4c472906ad30e6e8af4ca33
2 changes: 1 addition & 1 deletion ext/opcache/Optimizer/zend_func_info.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -727,7 +727,7 @@ static const func_info_t func_infos[] = {
F1("pg_result_error", MAY_BE_FALSE | MAY_BE_STRING),
F1("pg_result_error_field", MAY_BE_NULL | MAY_BE_FALSE | MAY_BE_STRING),
F1("pg_get_result", MAY_BE_FALSE | MAY_BE_RESOURCE),
F1("pg_result_status", MAY_BE_FALSE | MAY_BE_LONG | MAY_BE_STRING),
F1("pg_result_status", MAY_BE_LONG | MAY_BE_STRING),
F1("pg_get_notify", MAY_BE_FALSE | MAY_BE_ARRAY | MAY_BE_ARRAY_KEY_ANY | MAY_BE_ARRAY_OF_ANY),
F1("pg_socket", MAY_BE_FALSE | MAY_BE_RESOURCE),
F1("pg_meta_data", MAY_BE_FALSE | MAY_BE_ARRAY | MAY_BE_ARRAY_KEY_STRING | MAY_BE_ARRAY_OF_ARRAY),
Expand Down
178 changes: 123 additions & 55 deletions ext/pgsql/pgsql.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1468,8 +1468,8 @@ PHP_FUNCTION(pg_last_notice)
RETURN_TRUE;
break;
default:
php_error_docref(NULL, E_WARNING,
"Invalid option specified (" ZEND_LONG_FMT ")", option);
zend_argument_value_error(2, "must be one of PGSQL_NOTICE_LAST, PGSQL_NOTICE_ALL, or PGSQL_NOTICE_CLEAR");
RETURN_THROWS();
}
RETURN_FALSE;
}
Expand Down Expand Up @@ -1555,7 +1555,12 @@ PHP_FUNCTION(pg_field_table)
RETURN_THROWS();
}

if (fnum < 0 || fnum >= PQnfields(pg_result->result)) {
if (fnum < 0) {
zend_argument_value_error(2, "must be greater than or equal to 0");
RETURN_THROWS();
}

if (fnum >= PQnfields(pg_result->result)) {
php_error_docref(NULL, E_WARNING, "Bad field offset specified");
RETURN_FALSE;
}
Expand Down Expand Up @@ -1638,10 +1643,14 @@ static void php_pgsql_get_field_info(INTERNAL_FUNCTION_PARAMETERS, int entry_typ
RETURN_THROWS();
}

if (field < 0) {
zend_argument_value_error(2, "must be greater than or equal to 0");
RETURN_THROWS();
}

pgsql_result = pg_result->result;

if (field < 0 || field >= PQnfields(pgsql_result)) {
if (field >= PQnfields(pgsql_result)) {
php_error_docref(NULL, E_WARNING, "Bad field offset specified");
RETURN_FALSE;
}
Expand Down Expand Up @@ -1758,7 +1767,11 @@ PHP_FUNCTION(pg_fetch_result)
}
pg_result->row++;
} else {
if (row < 0 || row >= PQntuples(pgsql_result)) {
if (row < 0) {
zend_argument_value_error(2, "must be greater than or equal to 0");
RETURN_THROWS();
}
if (row >= PQntuples(pgsql_result)) {
php_error_docref(NULL, E_WARNING, "Unable to jump to row " ZEND_LONG_FMT " on PostgreSQL result index " ZEND_LONG_FMT,
row, Z_LVAL_P(result));
RETURN_FALSE;
Expand All @@ -1767,12 +1780,17 @@ PHP_FUNCTION(pg_fetch_result)
}
if (field_name) {
field_offset = PQfnumber(pgsql_result, ZSTR_VAL(field_name));
// TODO Split into 2 and ValueError for negative index?
if (field_offset < 0 || field_offset >= PQnfields(pgsql_result)) {
php_error_docref(NULL, E_WARNING, "Bad column offset specified");
RETURN_FALSE;
}
} else {
if (field_offset < 0 || field_offset >= PQnfields(pgsql_result)) {
if (field_offset < 0) {
zend_argument_value_error(argc, "must be greater than or equal to 0");
RETURN_THROWS();
}
if (field_offset >= PQnfields(pgsql_result)) {
php_error_docref(NULL, E_WARNING, "Bad column offset specified");
RETURN_FALSE;
}
Expand Down Expand Up @@ -1815,13 +1833,13 @@ static void php_pgsql_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, zend_long result_
}

if (!row_is_null && row < 0) {
php_error_docref(NULL, E_WARNING, "The row parameter must be greater or equal to zero");
RETURN_FALSE;
zend_argument_value_error(2, "must be greater than or equal to 0");
RETURN_THROWS();
}

if (!(result_type & PGSQL_BOTH)) {
php_error_docref(NULL, E_WARNING, "Invalid result type");
RETURN_FALSE;
zend_argument_value_error(3, "must be one of PGSQL_ASSOC, PGSQL_NUM, or PGSQL_BOTH");
RETURN_THROWS();
}

if ((pg_result = (pgsql_result_handle *)zend_fetch_resource(Z_RES_P(result), "PostgreSQL result", le_result)) == NULL) {
Expand All @@ -1831,7 +1849,7 @@ static void php_pgsql_fetch_hash(INTERNAL_FUNCTION_PARAMETERS, zend_long result_
pgsql_result = pg_result->result;

if (!row_is_null) {
if (row < 0 || row >= PQntuples(pgsql_result)) {
if (row >= PQntuples(pgsql_result)) {
php_error_docref(NULL, E_WARNING, "Unable to jump to row " ZEND_LONG_FMT " on PostgreSQL result index " ZEND_LONG_FMT,
row, Z_LVAL_P(result));
RETURN_FALSE;
Expand Down Expand Up @@ -1977,8 +1995,8 @@ PHP_FUNCTION(pg_fetch_all)
}

if (!(result_type & PGSQL_BOTH)) {
php_error_docref(NULL, E_WARNING, "Invalid result type");
RETURN_FALSE;
zend_argument_value_error(4, "must be one of PGSQL_ASSOC, PGSQL_NUM, or PGSQL_BOTH");
RETURN_THROWS();
}

if ((pg_result = (pgsql_result_handle *)zend_fetch_resource(Z_RES_P(result), "PostgreSQL result", le_result)) == NULL) {
Expand Down Expand Up @@ -2012,10 +2030,15 @@ PHP_FUNCTION(pg_fetch_all_columns)
RETURN_THROWS();
}

if (colno < 0) {
zend_argument_value_error(2, "must be greater than or equal to 0");
RETURN_THROWS();
}

pgsql_result = pg_result->result;

num_fields = PQnfields(pgsql_result);
if (colno >= (zend_long)num_fields || colno < 0) {
if (colno >= (zend_long)num_fields) {
php_error_docref(NULL, E_WARNING, "Invalid column number '" ZEND_LONG_FMT "'", colno);
RETURN_FALSE;
}
Expand Down Expand Up @@ -2097,11 +2120,16 @@ static void php_pgsql_data_info(INTERNAL_FUNCTION_PARAMETERS, int entry_type)
pg_result->row = 0;
}
pgsql_row = pg_result->row;
// TODO Split into 2 and ValueError for negative index?
if (pgsql_row < 0 || pgsql_row >= PQntuples(pgsql_result)) {
RETURN_FALSE;
}
} else {
if (row < 0 || row >= PQntuples(pgsql_result)) {
if (row < 0) {
zend_argument_value_error(2, "must be greater than or equal to 0");
RETURN_THROWS();
}
if (row >= PQntuples(pgsql_result)) {
php_error_docref(NULL, E_WARNING, "Unable to jump to row " ZEND_LONG_FMT " on PostgreSQL result index " ZEND_LONG_FMT,
row, Z_LVAL_P(result));
RETURN_FALSE;
Expand All @@ -2111,12 +2139,16 @@ static void php_pgsql_data_info(INTERNAL_FUNCTION_PARAMETERS, int entry_type)

if (field_name) {
field_offset = PQfnumber(pgsql_result, ZSTR_VAL(field_name));
// TODO Split into 2 and ValueError for negative index?
if (field_offset < 0 || field_offset >= PQnfields(pgsql_result)) {
php_error_docref(NULL, E_WARNING, "Bad column offset specified");
RETURN_FALSE;
}
} else {
if (field_offset < 0 || field_offset >= PQnfields(pgsql_result)) {
if (field_offset < 0) {
zend_argument_value_error(argc, "must be greater than or equal to 0");
}
if (field_offset >= PQnfields(pgsql_result)) {
php_error_docref(NULL, E_WARNING, "Bad column offset specified");
RETURN_FALSE;
}
Expand Down Expand Up @@ -2603,14 +2635,15 @@ PHP_FUNCTION(pg_lo_write)
}

if (argc > 2) {
if (z_len < 0) {
zend_argument_value_error(3, "must be greater than or equal to 0");
RETURN_THROWS();
}
if (z_len > (zend_long)str_len) {
/* TODO Promote to ValueError? */
php_error_docref(NULL, E_WARNING, "Cannot write more than buffer size %zu. Tried to write " ZEND_LONG_FMT, str_len, z_len);
RETURN_FALSE;
}
if (z_len < 0) {
php_error_docref(NULL, E_WARNING, "Buffer size must be larger than 0, but " ZEND_LONG_FMT " was specified", z_len);
RETURN_FALSE;
}
len = z_len;
}
else {
Expand Down Expand Up @@ -2816,9 +2849,10 @@ PHP_FUNCTION(pg_lo_seek)
if (zend_parse_parameters(argc, "rl|l", &pgsql_id, &offset, &whence) == FAILURE) {
RETURN_THROWS();
}
/* TODO Error for < 0 offset? */
if (whence != SEEK_SET && whence != SEEK_CUR && whence != SEEK_END) {
php_error_docref(NULL, E_WARNING, "Invalid whence parameter");
return;
zend_argument_value_error(3, "must be one of PGSQL_SEEK_SET, PGSQL_SEEK_CUR, or PGSQL_SEEK_END");
RETURN_THROWS();
}

if ((pgsql = (pgLofp *)zend_fetch_resource(Z_RES_P(pgsql_id), "PostgreSQL large object", le_lofp)) == NULL) {
Expand Down Expand Up @@ -3352,6 +3386,7 @@ PHP_FUNCTION(pg_unescape_bytea)
to = estrndup(tmp, to_len);
PQfreemem(tmp);
if (!to) {
/* TODO Promote to Error? */
php_error_docref(NULL, E_WARNING,"Invalid parameter");
RETURN_FALSE;
}
Expand Down Expand Up @@ -4020,10 +4055,9 @@ PHP_FUNCTION(pg_result_status)
}
else if (result_type == PGSQL_STATUS_STRING) {
RETURN_STRING(PQcmdStatus(pgsql_result));
}
else {
php_error_docref(NULL, E_WARNING, "Optional 2nd parameter should be PGSQL_STATUS_LONG or PGSQL_STATUS_STRING");
RETURN_FALSE;
} else {
zend_argument_value_error(2, "must be either PGSQL_STATUS_LONG or PGSQL_STATUS_STRING");
RETURN_THROWS();
}
}
/* }}} */
Expand All @@ -4045,8 +4079,8 @@ PHP_FUNCTION(pg_get_notify)
}

if (!(result_type & PGSQL_BOTH)) {
php_error_docref(NULL, E_WARNING, "Invalid result type");
RETURN_FALSE;
zend_argument_value_error(2, "must be one of PGSQL_ASSOC, PGSQL_NUM, or PGSQL_BOTH");
RETURN_THROWS();
}

PQconsumeInput(pgsql);
Expand Down Expand Up @@ -4246,6 +4280,7 @@ PHP_PGSQL_API int php_pgsql_meta_data(PGconn *pg_link, const char *table_name, z
zval elem;

if (!*table_name) {
// CHeck if can be TODO
php_error_docref(NULL, E_WARNING, "The table name must be specified");
return FAILURE;
}
Expand Down Expand Up @@ -4364,6 +4399,12 @@ PHP_FUNCTION(pg_meta_data)
RETURN_THROWS();
}

/* php_pgsql_meta_data() warns on empty table_name */
if (table_name_len == 0) {
zend_argument_value_error(2, "cannot be empty");
RETURN_THROWS();
}

array_init(return_value);
if (php_pgsql_meta_data(pgsql, table_name, return_value, extended) == FAILURE) {
zend_array_destroy(Z_ARR_P(return_value)); /* destroy array */
Expand Down Expand Up @@ -4564,14 +4605,11 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con
int err = 0, skip_field;
php_pgsql_data_type data_type;

assert(pg_link != NULL);
assert(Z_TYPE_P(values) == IS_ARRAY);
assert(Z_TYPE_P(result) == IS_ARRAY);
assert(!(opt & ~PGSQL_CONV_OPTS));

if (!table_name) {
return FAILURE;
}
ZEND_ASSERT(pg_link != NULL);
ZEND_ASSERT(Z_TYPE_P(values) == IS_ARRAY);
ZEND_ASSERT(Z_TYPE_P(result) == IS_ARRAY);
ZEND_ASSERT(!(opt & ~PGSQL_CONV_OPTS));
ZEND_ASSERT(table_name);

array_init(&meta);
/* table_name is escaped by php_pgsql_meta_data */
Expand Down Expand Up @@ -5222,13 +5260,16 @@ PHP_FUNCTION(pg_convert)
"rsa|l", &pgsql_link, &table_name, &table_name_len, &values, &option) == FAILURE) {
RETURN_THROWS();
}
if (option & ~PGSQL_CONV_OPTS) {
php_error_docref(NULL, E_WARNING, "Invalid option is specified");
RETURN_FALSE;

if (table_name_len == 0) {
zend_argument_value_error(2, "cannot be empty");
RETURN_THROWS();
}
if (!table_name_len) {
php_error_docref(NULL, E_NOTICE, "Table name is invalid");
RETURN_FALSE;

if (option & ~PGSQL_CONV_OPTS) {
zend_argument_value_error(4, "must be a valid bit mask of PGSQL_CONV_IGNORE_DEFAULT, "
"PGSQL_CONV_FORCE_NULL, or PGSQL_CONV_IGNORE_NOT_NULL");
RETURN_THROWS();
}

if ((pg_link = (PGconn *)zend_fetch_resource2(Z_RES_P(pgsql_link), "PostgreSQL link", le_link, le_plink)) == NULL) {
Expand Down Expand Up @@ -5433,9 +5474,16 @@ PHP_FUNCTION(pg_insert)
&pgsql_link, &table, &table_len, &values, &option) == FAILURE) {
RETURN_THROWS();
}

if (table_len == 0) {
zend_argument_value_error(2, "cannot be empty");
RETURN_THROWS();
}

if (option & ~(PGSQL_CONV_OPTS|PGSQL_DML_NO_CONV|PGSQL_DML_EXEC|PGSQL_DML_ASYNC|PGSQL_DML_STRING|PGSQL_DML_ESCAPE)) {
php_error_docref(NULL, E_WARNING, "Invalid option is specified");
RETURN_FALSE;
zend_argument_value_error(4, "must be a valid bit mask of PGSQL_CONV_FORCE_NULL, PGSQL_DML_NO_CONV, "
"PGSQL_DML_ESCAPE, PGSQL_DML_EXEC, PGSQL_DML_ASYNC or PGSQL_DML_STRING");
RETURN_THROWS();
}

if ((pg_link = (PGconn *)zend_fetch_resource2(Z_RES_P(pgsql_link), "PostgreSQL link", le_link, le_plink)) == NULL) {
Expand Down Expand Up @@ -5643,9 +5691,16 @@ PHP_FUNCTION(pg_update)
&pgsql_link, &table, &table_len, &values, &ids, &option) == FAILURE) {
RETURN_THROWS();
}

if (table_len == 0) {
zend_argument_value_error(2, "cannot be empty");
RETURN_THROWS();
}

if (option & ~(PGSQL_CONV_OPTS|PGSQL_DML_NO_CONV|PGSQL_DML_EXEC|PGSQL_DML_STRING|PGSQL_DML_ESCAPE)) {
php_error_docref(NULL, E_WARNING, "Invalid option is specified");
RETURN_FALSE;
zend_argument_value_error(5, "must be a valid bit mask of PGSQL_CONV_FORCE_NULL, PGSQL_DML_NO_CONV, "
"PGSQL_DML_ESCAPE, PGSQL_DML_EXEC, PGSQL_DML_ASYNC or PGSQL_DML_STRING");
RETURN_THROWS();
}

if ((pg_link = (PGconn *)zend_fetch_resource2(Z_RES_P(pgsql_link), "PostgreSQL link", le_link, le_plink)) == NULL) {
Expand Down Expand Up @@ -5733,9 +5788,16 @@ PHP_FUNCTION(pg_delete)
&pgsql_link, &table, &table_len, &ids, &option) == FAILURE) {
RETURN_THROWS();
}

if (table_len == 0) {
zend_argument_value_error(2, "cannot be empty");
RETURN_THROWS();
}

if (option & ~(PGSQL_CONV_FORCE_NULL|PGSQL_DML_NO_CONV|PGSQL_DML_EXEC|PGSQL_DML_STRING|PGSQL_DML_ESCAPE)) {
php_error_docref(NULL, E_WARNING, "Invalid option is specified");
RETURN_FALSE;
zend_argument_value_error(4, "must be a valid bit mask of PGSQL_CONV_FORCE_NULL, PGSQL_DML_NO_CONV, "
"PGSQL_DML_ESCAPE, PGSQL_DML_EXEC, PGSQL_DML_ASYNC or PGSQL_DML_STRING");
RETURN_THROWS();
}

if ((pg_link = (PGconn *)zend_fetch_resource2(Z_RES_P(pgsql_link), "PostgreSQL link", le_link, le_plink)) == NULL) {
Expand Down Expand Up @@ -5865,20 +5927,26 @@ PHP_FUNCTION(pg_select)
long result_type = PGSQL_ASSOC;
PGconn *pg_link;
zend_string *sql = NULL;
int argc = ZEND_NUM_ARGS();

// TODO: result_type is unused by zpp!
if (zend_parse_parameters(argc, "rsa|l",
/* TODO Document result_type param on php.net (apparently it was added in PHP 7.1) */
if (zend_parse_parameters(ZEND_NUM_ARGS(), "rsa|ll",
&pgsql_link, &table, &table_len, &ids, &option, &result_type) == FAILURE) {
RETURN_THROWS();
}

if (table_len == 0) {
zend_argument_value_error(2, "cannot be empty");
RETURN_THROWS();
}

if (option & ~(PGSQL_CONV_FORCE_NULL|PGSQL_DML_NO_CONV|PGSQL_DML_EXEC|PGSQL_DML_ASYNC|PGSQL_DML_STRING|PGSQL_DML_ESCAPE)) {
php_error_docref(NULL, E_WARNING, "Invalid option is specified");
RETURN_FALSE;
zend_argument_value_error(4, "must be a valid bit mask of PGSQL_CONV_FORCE_NULL, PGSQL_DML_NO_CONV, "
"PGSQL_DML_ESCAPE, PGSQL_DML_EXEC, PGSQL_DML_ASYNC or PGSQL_DML_STRING");
RETURN_THROWS();
}
if (!(result_type & PGSQL_BOTH)) {
php_error_docref(NULL, E_WARNING, "Invalid result type");
RETURN_FALSE;
zend_argument_value_error(5, "must be one of PGSQL_ASSOC, PGSQL_NUM, or PGSQL_BOTH");
RETURN_THROWS();
}

if ((pg_link = (PGconn *)zend_fetch_resource2(Z_RES_P(pgsql_link), "PostgreSQL link", le_link, le_plink)) == NULL) {
Expand Down
Loading