Skip to content
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

Jump to bottom

[sqlite3] Do not truncate results of user functions and aggregates on the first NUL #88985

Closed
erlend-aasland opened this issue Aug 4, 2021 · 8 comments
Closed

[sqlite3] Do not truncate results of user functions and aggregates on the first NUL #88985

erlend-aasland opened this issue Aug 4, 2021 · 8 comments
Assignees
@erlend-aasland
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes extension-modules C modules in the Modules dir topic-sqlite3 type-bug An unexpected behavior, bug, or error

Comments

@erlend-aasland
Copy link
Contributor

BPO 44822
Nosy @serhiy-storchaka, @miss-islington, @erlend-aasland
PRs
  • bpo-44822: Don't truncate strs with embedded NULL chars returned by sqlite3 UDF callbacks #27588
  • [3.10] bpo-44822: Don't truncate strs with embedded NULL chars returned by sqlite3 UDF callbacks (GH-27588) #27611
  • [3.9] bpo-44822: Don't truncate strs with embedded NULL chars returned by sqlite3 UDF callbacks (GH-27588) #27639
    		•

    Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/erlend-aasland'
closed_at = <Date 2021-08-06.21:08:39.691>
created_at = <Date 2021-08-04.08:29:38.206>
labels = ['extension-modules', 'type-bug', '3.9', '3.10', '3.11']
title = '[sqlite3] Do not truncate results of user functions and aggregates on the first NUL'
updated_at = <Date 2021-08-06.21:08:39.690>
user = 'https://github.com/erlend-aasland'

    bugs.python.org fields:

    activity = <Date 2021-08-06.21:08:39.690>
actor = 'erlendaasland'
assignee = 'erlendaasland'
closed = True
closed_date = <Date 2021-08-06.21:08:39.691>
closer = 'erlendaasland'
components = ['Extension Modules']
creation = <Date 2021-08-04.08:29:38.206>
creator = 'erlendaasland'
dependencies = []
files = []
hgrepos = []
issue_num = 44822
keywords = ['patch']
message_count = 8.0
messages = ['398865', '398869', '398873', '398881', '398971', '399115', '399146', '399149']
nosy_count = 3.0
nosy_names = ['serhiy.storchaka', 'miss-islington', 'erlendaasland']
pr_nums = ['27588', '27611', '27639']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue44822'
versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']
    @erlend-aasland
    Copy link
    Contributor Author

    The third argument to sqlite3_result_text() is the length of the string passed as the second argument. Currently, we pass -1, so SQLite has to invoke strlen() to compute the length of the passed string. Suggesting to use PyUnicode_AsUTF8AndSize() iso. PyUnicode_AsUTF8() and pass the string size to avoid the superfluous strlen().

    See also:

    @erlend-aasland erlend-aasland added type-feature A feature request or enhancement 3.11 only security fixes labels Aug 4, 2021
    @erlend-aasland erlend-aasland self-assigned this Aug 4, 2021
    @erlend-aasland erlend-aasland added type-feature A feature request or enhancement extension-modules C modules in the Modules dir 3.11 only security fixes labels Aug 4, 2021
    @erlend-aasland erlend-aasland self-assigned this Aug 4, 2021
    @erlend-aasland erlend-aasland added the extension-modules C modules in the Modules dir label Aug 4, 2021
    @serhiy-storchaka
    Copy link
    Member

    The difference between specifying negative and non-negative third argument of sqlite3_result_text() is that in the latter case the result can contain embedded NUL characters.

    Could you please add a test for string containing embedded NUL?

    Letting SQLite compute string length would work incorrect in case of embedded NULs, so I think that we should raise explicit error if sz > INT_MAX.

    @erlend-aasland
    Copy link
    Contributor Author

    Could you please add a test for string containing embedded NUL?

    Of course, thanks for the heads up.

    I think that we should raise explicit error if sz > INT_MAX.

    Yes, I thought about this. I do agree that raising OverflowError would be better. Thanks.

    @erlend-aasland
    Copy link
    Contributor Author

    Serhiy, I've updated the PR, if you want to take a look. Thanks for your feedback.

    @serhiy-storchaka serhiy-storchaka added 3.9 only security fixes 3.10 only security fixes labels Aug 4, 2021
    @serhiy-storchaka serhiy-storchaka changed the title [sqlite3] Micro-optimisation: pass string size to sqlite3_result_text() [sqlite3] Do not truncate results of user functions and aggregates on the first NUL Aug 4, 2021
    @serhiy-storchaka serhiy-storchaka added 3.9 only security fixes 3.10 only security fixes type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Aug 4, 2021
    @serhiy-storchaka serhiy-storchaka changed the title [sqlite3] Micro-optimisation: pass string size to sqlite3_result_text() [sqlite3] Do not truncate results of user functions and aggregates on the first NUL Aug 4, 2021
    @serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error and removed type-feature A feature request or enhancement labels Aug 4, 2021
    @serhiy-storchaka
    Copy link
    Member

    New changeset 8f010dc by Erlend Egeberg Aasland in branch 'main':
    bpo-44822: Don't truncate strs with embedded NULL chars returned by sqlite3 UDF callbacks (GH-27588)
    8f010dc

    @miss-islington
    Copy link
    Contributor

    New changeset 2b1e713 by Miss Islington (bot) in branch '3.10':
    bpo-44822: Don't truncate strs with embedded NULL chars returned by sqlite3 UDF callbacks (GH-27588)
    2b1e713

    @serhiy-storchaka
    Copy link
    Member

    New changeset c352412 by Erlend Egeberg Aasland in branch '3.9':
    [3.9] bpo-44822: Don't truncate strs with embedded NULL chars returned by sqlite3 UDF callbacks (GH-27588). (GH-27639)
    c352412

    @erlend-aasland
    Copy link
    Contributor Author

    Thanks, Serhiy!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @erlend-aasland erlend-aasland

    Labels
    3.9 only security fixes 3.10 only security fixes 3.11 only security fixes extension-modules C modules in the Modules dir topic-sqlite3 type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    3 participants
    @serhiy-storchaka @erlend-aasland @miss-islington