From f31d08de36fd4859aa4853345cb5ab846dbf1ca8 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 18:31:02 +0100 Subject: [PATCH 1/4] Promote E_NOTICE to Error in PostgreSQL extension --- ext/pgsql/pgsql.c | 100 ++++++++++++++++++++++++++-------------------- 1 file changed, 57 insertions(+), 43 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index c08ed40d01d22..1b5ee683f2fd7 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -2300,6 +2300,7 @@ PHP_FUNCTION(pg_lo_create) RETURN_THROWS(); } + /* Overloaded method use default link if arg 1 is not a ressource, set oid pointer */ if ((argc == 1) && (Z_TYPE_P(pgsql_link) != IS_RESOURCE)) { oid = pgsql_link; pgsql_link = NULL; @@ -2322,25 +2323,26 @@ PHP_FUNCTION(pg_lo_create) switch (Z_TYPE_P(oid)) { case IS_STRING: { + /* TODO: Use zend_is_numeric_string/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; + 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"); @@ -2374,30 +2376,32 @@ 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 zend_is_numeric_string/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: Use zend_is_numeric_string/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); @@ -2405,8 +2409,8 @@ PHP_FUNCTION(pg_lo_unlink) 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(); @@ -2447,30 +2451,32 @@ 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 zend_is_numeric_string/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 zend_is_numeric_string/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); @@ -2478,8 +2484,8 @@ PHP_FUNCTION(pg_lo_open) 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(); @@ -2525,6 +2531,7 @@ PHP_FUNCTION(pg_lo_open) if ((pgsql_lofd = lo_open(pgsql, oid, pgsql_mode)) == -1) { if (lo_unlink(pgsql, oid) == -1) { efree(pgsql_lofp); + /* TODO: Throw error? */ php_error_docref(NULL, E_WARNING, "Something is really messed up! Your database is badly corrupted in a way NOT related to PHP"); RETURN_FALSE; } @@ -2717,25 +2724,26 @@ PHP_FUNCTION(pg_lo_import) switch (Z_TYPE_P(oid)) { case IS_STRING: { + /* TODO: Use zend_is_numeric_string/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; + 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); @@ -2773,27 +2781,28 @@ 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 zend_is_numeric_string/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(); @@ -2801,11 +2810,12 @@ PHP_FUNCTION(pg_lo_export) } else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc, "sp", &oid_string, &oid_strlen, &file_out, &name_len) == SUCCESS) { + /* TODO: Use zend_is_numeric_string/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); @@ -4264,6 +4274,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; @@ -4557,6 +4568,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 */ \ @@ -4600,8 +4612,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 associated array with string keys"); err = 1; } @@ -4626,7 +4640,7 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con 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"); + zend_type_error("Values must be of type string|int|float|bool|null, %s given", zend_zval_type_name(val)); err = 1; } if (err) { @@ -4641,6 +4655,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: @@ -5358,7 +5373,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 associated array of string keys"); goto cleanup; } if (opt & PGSQL_DML_ESCAPE) { @@ -5401,9 +5416,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(); @@ -5532,7 +5546,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 associated array of string keys"); return -1; } if (opt & PGSQL_DML_ESCAPE) { @@ -5573,7 +5587,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); From ae1001a25e6a3968c3a1424e7b94ded4c251905b Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Mon, 28 Sep 2020 22:59:21 +0100 Subject: [PATCH 2/4] Tests + nits --- ext/pgsql/pgsql.c | 8 +-- ext/pgsql/tests/05large_object.phpt | 26 ++++++++++ ext/pgsql/tests/10pg_convert_9.phpt | 33 +++++++++++++ ext/pgsql/tests/12pg_insert_9.phpt | 33 +++++++++++++ ext/pgsql/tests/13pg_select_9.phpt | 34 +++++++++++++ .../tests/28large_object_import_oid.phpt | 49 +++++++++++++++++++ 6 files changed, 179 insertions(+), 4 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index 1b5ee683f2fd7..e36f676812ceb 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -2300,7 +2300,7 @@ PHP_FUNCTION(pg_lo_create) RETURN_THROWS(); } - /* Overloaded method use default link if arg 1 is not a ressource, set oid pointer */ + /* 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; @@ -4615,7 +4615,7 @@ PHP_PGSQL_API int php_pgsql_convert(PGconn *pg_link, const char *table_name, con /* TODO: Check when meta data can be broken and see if can use assertions instead */ if (!err && field == NULL) { - zend_value_error("Array of values must be an associated array with string keys"); + zend_value_error("Array of values must be an associative array with string keys"); err = 1; } @@ -4639,7 +4639,7 @@ 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)) { + 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; } @@ -5373,7 +5373,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) { - zend_value_error("Array of values must be an associated array of string keys"); + zend_value_error("Array of values must be an associative array of string keys"); goto cleanup; } if (opt & PGSQL_DML_ESCAPE) { diff --git a/ext/pgsql/tests/05large_object.phpt b/ext/pgsql/tests/05large_object.phpt index 3a4a40eb0865f..b493edd221123 100644 --- a/ext/pgsql/tests/05large_object.phpt +++ b/ext/pgsql/tests/05large_object.phpt @@ -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-- @@ -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 diff --git a/ext/pgsql/tests/10pg_convert_9.phpt b/ext/pgsql/tests/10pg_convert_9.phpt index a8395315c6510..0a2828a247da4 100644 --- a/ext/pgsql/tests/10pg_convert_9.phpt +++ b/ext/pgsql/tests/10pg_convert_9.phpt @@ -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) { @@ -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 diff --git a/ext/pgsql/tests/12pg_insert_9.phpt b/ext/pgsql/tests/12pg_insert_9.phpt index 275afc55e1df8..11a401f3580d5 100644 --- a/ext/pgsql/tests/12pg_insert_9.phpt +++ b/ext/pgsql/tests/12pg_insert_9.phpt @@ -21,10 +21,43 @@ echo pg_insert($db, $table_name, $fields, PGSQL_DML_STRING)."\n"; echo pg_insert($db, $table_name, $fields, PGSQL_DML_STRING|PGSQL_DML_ESCAPE)."\n"; var_dump( pg_insert($db, $table_name, $fields, PGSQL_DML_EXEC) ); // Return resource +/* Invalid values */ +try { + $converted = pg_insert($db, $table_name, [5 => 'AAA']); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + $converted = pg_insert($db, $table_name, ['AAA']); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + $converted = pg_insert($db, $table_name, ['num' => []]); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + $converted = pg_insert($db, $table_name, ['num' => new stdClass()]); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + $converted = pg_insert($db, $table_name, ['num' => $db]); + var_dump($converted); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} + echo "Ok\n"; ?> --EXPECTF-- INSERT INTO "php_pgsql_test" ("num","str","bin") VALUES (1234,E'AAA',E'\\x424242'); INSERT INTO "php_pgsql_test" ("num","str","bin") VALUES ('1234','AAA','BBB'); resource(%d) of type (pgsql result) +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 Ok diff --git a/ext/pgsql/tests/13pg_select_9.phpt b/ext/pgsql/tests/13pg_select_9.phpt index d5e661e5e0105..82a59a1cf5638 100644 --- a/ext/pgsql/tests/13pg_select_9.phpt +++ b/ext/pgsql/tests/13pg_select_9.phpt @@ -21,6 +21,35 @@ $res = pg_select($db, $table_name, $ids) or print "Error\n"; var_dump($res); echo pg_select($db, $table_name, $ids, PGSQL_DML_STRING)."\n"; echo pg_select($db, $table_name, $ids, PGSQL_DML_STRING|PGSQL_DML_ESCAPE)."\n"; + +/* Invalid values */ +try { + $converted = pg_select($db, $table_name, [5 => 'AAA']); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + $converted = pg_select($db, $table_name, ['AAA']); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + $converted = pg_select($db, $table_name, ['num' => []]); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + $converted = pg_select($db, $table_name, ['num' => new stdClass()]); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + $converted = pg_select($db, $table_name, ['num' => $db]); + var_dump($converted); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} + echo "Ok\n"; ?> @@ -47,4 +76,9 @@ array(2) { } SELECT * FROM "php_pgsql_test" WHERE "num"=1234; SELECT * FROM "php_pgsql_test" WHERE "num"='1234'; +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 Ok diff --git a/ext/pgsql/tests/28large_object_import_oid.phpt b/ext/pgsql/tests/28large_object_import_oid.phpt index 9ffb96123e9d0..8209cc2932432 100644 --- a/ext/pgsql/tests/28large_object_import_oid.phpt +++ b/ext/pgsql/tests/28large_object_import_oid.phpt @@ -38,6 +38,47 @@ if ($oid != 21005) echo ("pg_lo_import() wrong id\n"); pg_lo_unlink ($oid); pg_exec('commit'); +/* Invalide OID */ +try { + pg_lo_import(__FILE__, -15); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + pg_lo_import($db, __FILE__, -15); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + pg_lo_import(__FILE__, 'giberrish'); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + pg_lo_import($db, __FILE__, 'giberrish'); +} catch (\ValueError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + pg_lo_import(__FILE__, true); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + pg_lo_import($db, __FILE__, []); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + pg_lo_import($db, __FILE__, new stdClass()); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} +try { + pg_lo_import($db, __FILE__, $db); +} catch (\TypeError $e) { + echo $e->getMessage(), \PHP_EOL; +} echo "OK"; ?> @@ -45,4 +86,12 @@ echo "OK"; import LO from int import LO from string import LO using default connection +Invalid OID value passed +Invalid OID value passed +Invalid OID value passed +Invalid OID value passed +OID value must be of type string|int, bool given +OID value must be of type string|int, array given +OID value must be of type string|int, stdClass given +OID value must be of type string|int, resource given OK From e327070b34e8fb586a32b3e47e7b068c1b213242 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Tue, 29 Sep 2020 14:59:46 +0100 Subject: [PATCH 3/4] Review Also realized this doesn't test the value error code path in build_assignment_string() because the same error condition seems to fail early --- ext/pgsql/pgsql.c | 97 +++++++++++++++++++++++------------------------ 1 file changed, 48 insertions(+), 49 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index e36f676812ceb..cc1fba7aadd23 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -2321,28 +2321,28 @@ PHP_FUNCTION(pg_lo_create) if (oid) { switch (Z_TYPE_P(oid)) { - case IS_STRING: - { - /* TODO: Use zend_is_numeric_string/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 */ - zend_value_error("Invalid OID value passed"); - RETURN_THROWS(); + 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 */ + zend_value_error("Invalid OID value passed"); + RETURN_THROWS(); + } } - } - break; - case IS_LONG: - if (Z_LVAL_P(oid) < (zend_long)InvalidOid) { - zend_value_error("Invalid OID value passed"); + break; + case IS_LONG: + if (Z_LVAL_P(oid) < (zend_long)InvalidOid) { + zend_value_error("Invalid OID value passed"); + RETURN_THROWS(); + } + wanted_oid = (Oid)Z_LVAL_P(oid); + break; + default: + zend_type_error("OID value must be of type string|int, %s given", zend_zval_type_name(oid)); RETURN_THROWS(); - } - wanted_oid = (Oid)Z_LVAL_P(oid); - break; - default: - 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"); @@ -2376,7 +2376,7 @@ 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 zend_is_numeric_string/subroutine? */ + /* TODO: Use subroutine? */ oid = (Oid)strtoul(oid_string, &end_ptr, 10); if ((oid_string+oid_strlen) != end_ptr) { /* wrong integer format */ @@ -2396,7 +2396,7 @@ PHP_FUNCTION(pg_lo_unlink) } else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc, "s", &oid_string, &oid_strlen) == SUCCESS) { - /* TODO: Use zend_is_numeric_string/subroutine? */ + /* TODO: subroutine? */ oid = (Oid)strtoul(oid_string, &end_ptr, 10); if ((oid_string+oid_strlen) != end_ptr) { /* wrong integer format */ @@ -2451,7 +2451,7 @@ 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 zend_is_numeric_string/subroutine? */ + /* TODO: Use subroutine? */ oid = (Oid)strtoul(oid_string, &end_ptr, 10); if ((oid_string+oid_strlen) != end_ptr) { /* wrong integer format */ @@ -2471,7 +2471,7 @@ PHP_FUNCTION(pg_lo_open) } else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc, "ss", &oid_string, &oid_strlen, &mode_string, &mode_strlen) == SUCCESS) { - /* TODO: Use zend_is_numeric_string/subroutine? */ + /* TODO: Use subroutine? */ oid = (Oid)strtoul(oid_string, &end_ptr, 10); if ((oid_string+oid_strlen) != end_ptr) { /* wrong integer format */ @@ -2531,7 +2531,6 @@ PHP_FUNCTION(pg_lo_open) if ((pgsql_lofd = lo_open(pgsql, oid, pgsql_mode)) == -1) { if (lo_unlink(pgsql, oid) == -1) { efree(pgsql_lofp); - /* TODO: Throw error? */ php_error_docref(NULL, E_WARNING, "Something is really messed up! Your database is badly corrupted in a way NOT related to PHP"); RETURN_FALSE; } @@ -2722,28 +2721,28 @@ PHP_FUNCTION(pg_lo_import) if (oid) { Oid wanted_oid; switch (Z_TYPE_P(oid)) { - case IS_STRING: - { - /* TODO: Use zend_is_numeric_string/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 */ - zend_value_error("Invalid OID value passed"); - RETURN_THROWS(); + 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 */ + zend_value_error("Invalid OID value passed"); + RETURN_THROWS(); + } } - } - break; - case IS_LONG: - if (Z_LVAL_P(oid) < (zend_long)InvalidOid) { - zend_value_error("Invalid OID value passed"); + break; + case IS_LONG: + if (Z_LVAL_P(oid) < (zend_long)InvalidOid) { + zend_value_error("Invalid OID value passed"); + RETURN_THROWS(); + } + wanted_oid = (Oid)Z_LVAL_P(oid); + break; + default: + zend_type_error("OID value must be of type string|int, %s given", zend_zval_type_name(oid)); RETURN_THROWS(); - } - wanted_oid = (Oid)Z_LVAL_P(oid); - break; - default: - 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); @@ -2789,7 +2788,7 @@ PHP_FUNCTION(pg_lo_export) } 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 zend_is_numeric_string/subroutine? */ + /* TODO: Use subroutine? */ oid = (Oid)strtoul(oid_string, &end_ptr, 10); if ((oid_string+oid_strlen) != end_ptr) { /* wrong integer format */ @@ -2810,7 +2809,7 @@ PHP_FUNCTION(pg_lo_export) } else if (zend_parse_parameters_ex(ZEND_PARSE_PARAMS_QUIET, argc, "sp", &oid_string, &oid_strlen, &file_out, &name_len) == SUCCESS) { - /* TODO: Use zend_is_numeric_string/subroutine? */ + /* TODO: Use subroutine? */ oid = (Oid)strtoul(oid_string, &end_ptr, 10); if ((oid_string+oid_strlen) != end_ptr) { /* wrong integer format */ @@ -5373,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) { - zend_value_error("Array of values must be an associative array of string keys"); + zend_value_error("Array of values must be an associative array with string keys"); goto cleanup; } if (opt & PGSQL_DML_ESCAPE) { @@ -5546,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) { - zend_value_error("Array of values must be an associated array of string keys"); + zend_value_error("Array of values must be an associative array with string keys"); return -1; } if (opt & PGSQL_DML_ESCAPE) { From 5ccf9c00ded486e980a8434a936d92d09c503de9 Mon Sep 17 00:00:00 2001 From: George Peter Banyard Date: Tue, 29 Sep 2020 15:07:09 +0100 Subject: [PATCH 4/4] Indentation... --- ext/pgsql/pgsql.c | 68 +++++++++++++++++++++++------------------------ 1 file changed, 34 insertions(+), 34 deletions(-) diff --git a/ext/pgsql/pgsql.c b/ext/pgsql/pgsql.c index cc1fba7aadd23..63e59b63d695d 100644 --- a/ext/pgsql/pgsql.c +++ b/ext/pgsql/pgsql.c @@ -2321,28 +2321,28 @@ PHP_FUNCTION(pg_lo_create) if (oid) { 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) { + 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 */ zend_value_error("Invalid OID value passed"); RETURN_THROWS(); - } - } - break; - case IS_LONG: - if (Z_LVAL_P(oid) < (zend_long)InvalidOid) { - zend_value_error("Invalid OID value passed"); - RETURN_THROWS(); } - wanted_oid = (Oid)Z_LVAL_P(oid); - break; - default: - zend_type_error("OID value must be of type string|int, %s given", zend_zval_type_name(oid)); + } + break; + case IS_LONG: + if (Z_LVAL_P(oid) < (zend_long)InvalidOid) { + zend_value_error("Invalid OID value passed"); RETURN_THROWS(); + } + wanted_oid = (Oid)Z_LVAL_P(oid); + break; + default: + 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"); @@ -2721,28 +2721,28 @@ PHP_FUNCTION(pg_lo_import) if (oid) { Oid wanted_oid; 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) { + 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 */ zend_value_error("Invalid OID value passed"); RETURN_THROWS(); - } - } - break; - case IS_LONG: - if (Z_LVAL_P(oid) < (zend_long)InvalidOid) { - zend_value_error("Invalid OID value passed"); - RETURN_THROWS(); } - wanted_oid = (Oid)Z_LVAL_P(oid); - break; - default: - zend_type_error("OID value must be of type string|int, %s given", zend_zval_type_name(oid)); + } + break; + case IS_LONG: + if (Z_LVAL_P(oid) < (zend_long)InvalidOid) { + zend_value_error("Invalid OID value passed"); RETURN_THROWS(); + } + wanted_oid = (Oid)Z_LVAL_P(oid); + break; + default: + 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);