Skip to content

nit: returning NULL with no explanation is not cool #18543

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+25 −3
Open

nit: returning NULL with no explanation is not cool #18543

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
eb3370c
returning NULL with no explanation is not cool
divinity76 May 12, 2025
183911c
update test
divinity76 May 12, 2025
d742ce2
update test
divinity76 May 12, 2025
bff6e29
update error message
divinity76 May 12, 2025
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
12 changes: 11 additions & 1 deletion ext/pdo_sqlite/sqlite_driver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,17 @@ static zend_string *pdo_sqlite_last_insert_id(pdo_dbh_t *dbh, const zend_string
static zend_string* sqlite_handle_quoter(pdo_dbh_t *dbh, const zend_string *unquoted, enum pdo_param_type paramtype)
{
char *quoted;
if (ZSTR_LEN(unquoted) > (INT_MAX - 3) / 2) {

if (UNEXPECTED(ZSTR_LEN(unquoted) > (INT_MAX - 3) / 2)) {
if (dbh->error_mode == PDO_ERRMODE_EXCEPTION) {
zend_throw_exception_ex(
php_pdo_get_exception(), 0,
"SQLite PDO::quote: string is too long to quote");
} else if (dbh->error_mode == PDO_ERRMODE_WARNING) {
php_error_docref(
NULL, E_WARNING,
"string is too long to quote");
}
Comment on lines +232 to +240
Copy link
Member

Choose a reason for hiding this comment

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

How about using pdo_raise_impl_error() for SQLSTATE 22001?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm, should it use 22001 regardless of error mode, or should it use 22001 for exception and 01004 otherwise?

Copy link
Member

@SakiTakamachi SakiTakamachi May 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Since 01004 is a warning that occurs when read data is discarded for instance, when the buffer is too small, I believe that in every case, 22001 is the appropriate choice.

edit:
Since this distinction is made in systems like DB2 and ODBC, 01004 is generally considered to be related to fetch operations.

return NULL;
}

Expand Down
16 changes: 14 additions & 2 deletions ext/pdo_sqlite/tests/bug81740.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,19 @@ memory_limit=-1
<?php
$pdo = new PDO("sqlite::memory:");
$string = str_repeat("a", 0x80000000);
var_dump($pdo->quote($string));
try{
var_dump($pdo->quote($string));
} finally {
$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_WARNING);
var_dump($pdo->quote($string));
}
?>
--EXPECT--
--EXPECTF--
Warning: PDO::quote(): string is too long to quote in %s on line %d
bool(false)

Fatal error: Uncaught PDOException: SQLite PDO::quote: string is too long to quote in %s:%d
Stack trace:
#0 %s(%d): PDO->quote('aaaaaaaaaaaaaaa...')
#1 {main}
thrown in %s on line %d