Skip to content

Fix large int corruption to/from pdo_sqlite UDF - #25058

Closed
mvorisek wants to merge 3 commits into
php:PHP-8.4from
mvorisek:fix_sqlite_corrupt_int_udf
Closed

Fix large int corruption to/from pdo_sqlite UDF#25058
mvorisek wants to merge 3 commits into
php:PHP-8.4from
mvorisek:fix_sqlite_corrupt_int_udf

Conversation

@mvorisek

@mvorisek mvorisek commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

fix #25021

@mvorisek
mvorisek changed the base branch from master to PHP-8.2 October 30, 2026 17:10
@mvorisek
mvorisek force-pushed the fix_sqlite_corrupt_int_udf branch from ac32567 to 802f51b Compare October 30, 2026 17:22
@mvorisek
mvorisek marked this pull request as ready for review October 30, 2026 17:55
@devnexen

devnexen commented Oct 30, 2026

Copy link
Copy Markdown
Member

I think the fix is correct, but do not think it qualifies for 8.2 ?

ndossche
ndossche reviewed Oct 30, 2026
View reviewed changes

@ndossche ndossche left a comment

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.

Fix is right, see nits.
Indeed cannot qualify for a security branch.

Comment thread ext/pdo_sqlite/sqlite_driver.c Outdated
/* get the value */
switch (sqlite3_value_type(argv[i])) {
case SQLITE_INTEGER:
#if ZEND_LONG_MAX > 2147483647

@ndossche ndossche Oct 30, 2026

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.

Should use #ifdef ZEND_ENABLE_ZVAL_LONG64 instead (below too)

Comment thread ext/pdo_sqlite/tests/gh25021-udf.phpt Outdated
@@ -0,0 +1,28 @@
--TEST--
GH-25021 UDF isn't using sqlite3_*_int64 API

@ndossche ndossche Oct 30, 2026

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.

Suggested change
GH-25021 UDF isn't using sqlite3_*_int64 API
GH-25021 (UDF isn't using sqlite3_*_int64 API)

@mvorisek mvorisek Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Some names are formatted this way, some are not, why?

@ndossche ndossche Oct 30, 2026

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.

because people are inconsistent

Comment thread ext/sqlite3/tests/gh25021-udf.phpt Outdated
@@ -0,0 +1,27 @@
--TEST--
GH-25021 UDF isn't using sqlite3_*_int64 API

@ndossche ndossche Oct 30, 2026

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.

Suggested change
GH-25021 UDF isn't using sqlite3_*_int64 API
GH-25021 (UDF isn't using sqlite3_*_int64 API)

@mvorisek

mvorisek commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Indeed cannot qualify for a security branch.

Even if it corrupts data and no user workaround is possible?

@ndossche

ndossche commented Oct 30, 2026

Copy link
Copy Markdown
Member

Indeed cannot qualify for a security branch.

Even if it corrupts data and no user workaround is possible?

Yes. Same with many other bugfixes

@mvorisek
mvorisek changed the base branch from PHP-8.2 to PHP-8.4 October 30, 2026 19:35
ndossche
ndossche approved these changes Oct 30, 2026
View reviewed changes

@ndossche ndossche left a comment

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.

thanks

@ndossche
ndossche requested a review from devnexen October 31, 2026 12:28
@devnexen devnexen closed this in 9c69d7c Oct 31, 2026
@mvorisek
mvorisek deleted the fix_sqlite_corrupt_int_udf branch October 31, 2026 13:03
@mvorisek mvorisek mentioned this pull request Oct 31, 2026
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@devnexen devnexen devnexen approved these changes
@ndossche ndossche ndossche approved these changes
@SakiTakamachi SakiTakamachi Awaiting requested review from SakiTakamachi SakiTakamachi is a code owner

Assignees

No one assigned

Labels

Extension: pdo_sqlite Extension: sqlite3

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

PDO\Sqlite::createFunction() corrupts large integers

3 participants

@mvorisek @devnexen @ndossche