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.

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

[sqlite3] add support for changing connection limits #89406

Closed
erlend-aasland opened this issue Sep 19, 2021 · 5 comments
Closed

[sqlite3] add support for changing connection limits #89406

erlend-aasland opened this issue Sep 19, 2021 · 5 comments
Assignees
Labels
expert-sqlite3 extension-modules C modules in the Modules dir type-feature A feature request or enhancement

Comments

@erlend-aasland
Copy link
Contributor

erlend-aasland commented Sep 19, 2021

BPO 45243
Nosy @tiran, @berkerpeksag, @serhiy-storchaka, @zooba, @pablogsal, @erlend-aasland
PRs
  • bpo-45243: Add support for setting/getting sqlite3 connection limits #28463
  • bpo-45243: Expose SQLite connection limits as sqlite3.Connection attributes #28790
  • bpo-45243: Use connection limits to simplify sqlite3 tests #29356
  • 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-11-13.21:17:03.972>
    created_at = <Date 2021-09-19.22:35:54.394>
    labels = ['extension-modules', 'type-feature']
    title = '[sqlite3] add support for changing connection limits'
    updated_at = <Date 2021-11-13.21:17:03.972>
    user = 'https://github.com/erlend-aasland'

    bugs.python.org fields:

    activity = <Date 2021-11-13.21:17:03.972>
    actor = 'erlendaasland'
    assignee = 'erlendaasland'
    closed = True
    closed_date = <Date 2021-11-13.21:17:03.972>
    closer = 'erlendaasland'
    components = ['Extension Modules']
    creation = <Date 2021-09-19.22:35:54.394>
    creator = 'erlendaasland'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45243
    keywords = ['patch']
    message_count = 5.0
    messages = ['402176', '402186', '405474', '405566', '405811']
    nosy_count = 6.0
    nosy_names = ['christian.heimes', 'berker.peksag', 'serhiy.storchaka', 'steve.dower', 'pablogsal', 'erlendaasland']
    pr_nums = ['28463', '28790', '29356']
    priority = 'low'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue45243'
    versions = []

    @erlend-aasland
    Copy link
    Contributor Author

    erlend-aasland commented Sep 19, 2021

    I propose to add wrappers for the SQLite sqlite3_limit() C API. Using this API, it is possible to query and set limits on a connection basis. This will make it easier (and faster) to test various corner cases in the test suite without relying on test.support.bigmemtest.

    Quoting from the SQLite sqlite3_limit() docs:

    Run-time limits are intended for use in applications that manage both their
    own internal database and also databases that are controlled by untrusted
    external sources. An example application might be a web browser that has its
    own databases for storing history and separate databases controlled by
    JavaScript applications downloaded off the Internet. The internal databases
    can be given the large, default limits. Databases managed by external
    sources can be given much smaller limits designed to prevent a denial of
    service attack.

    See also:

    Limit categories (C&P from SQLite docs)
    ---------------------------------------

    SQLITE_LIMIT_LENGTH
    The maximum size of any string or BLOB or table row, in bytes.

    SQLITE_LIMIT_SQL_LENGTH
    The maximum length of an SQL statement, in bytes.

    SQLITE_LIMIT_COLUMN
    The maximum number of columns in a table definition or in the result set of a SELECT or the maximum number of columns in an index or in an ORDER BY or GROUP BY clause.

    SQLITE_LIMIT_EXPR_DEPTH
    The maximum depth of the parse tree on any expression.

    SQLITE_LIMIT_COMPOUND_SELECT
    The maximum number of terms in a compound SELECT statement.

    SQLITE_LIMIT_VDBE_OP
    The maximum number of instructions in a virtual machine program used to implement an SQL statement. If sqlite3_prepare_v2() or the equivalent tries to allocate space for more than this many opcodes in a single prepared statement, an SQLITE_NOMEM error is returned.

    SQLITE_LIMIT_FUNCTION_ARG
    The maximum number of arguments on a function.

    SQLITE_LIMIT_ATTACHED
    The maximum number of attached databases.

    SQLITE_LIMIT_LIKE_PATTERN_LENGTH
    The maximum length of the pattern argument to the LIKE or GLOB operators.

    SQLITE_LIMIT_VARIABLE_NUMBER
    The maximum index number of any parameter in an SQL statement.

    SQLITE_LIMIT_TRIGGER_DEPTH
    The maximum depth of recursion for triggers.

    SQLITE_LIMIT_WORKER_THREADS
    The maximum number of auxiliary worker threads that a single prepared statement may start.

    @erlend-aasland erlend-aasland self-assigned this Sep 19, 2021
    @erlend-aasland erlend-aasland added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Sep 19, 2021
    @erlend-aasland erlend-aasland self-assigned this Sep 19, 2021
    @erlend-aasland erlend-aasland added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Sep 19, 2021
    @erlend-aasland
    Copy link
    Contributor Author

    erlend-aasland commented Sep 19, 2021

    Christian, how about adding an audit event for something like sqlite3.Connection.setlimit()? My initial thought is: yes.

    @pablogsal
    Copy link
    Member

    pablogsal commented Nov 1, 2021

    New changeset b6b38a8 by Erlend Egeberg Aasland in branch 'main':
    bpo-45243: Add support for setting/getting sqlite3 connection limits (GH-28463)
    b6b38a8

    @erlend-aasland
    Copy link
    Contributor Author

    erlend-aasland commented Nov 2, 2021

    Steve, do you think it is worth it adding an audit hook for setting connection limits?

    Most of the limits are harmless, but limits that control recursion are more interesting.

    SQLITE_LIMIT_EXPR_DEPTH:

    Maximum Depth Of An Expression Tree
    
    SQLite parses expressions into a tree for processing. During code
    generation, SQLite walks this tree recursively. The depth of expression
    trees is therefore limited in order to avoid using too much stack space.
    [...] If the value is 0, then no limit is enforced.
    

    SQLITE_LIMIT_TRIGGER_DEPTH:

    Maximum Depth Of Trigger Recursion
    
    SQLite limits the depth of recursion of triggers in order to prevent a
    statement involving recursive triggers from using an unbounded amount of
    memory.
    

    Note also, how the SQLite docs talk about SQLITE_LIMIT_LENGTH:

    Maximum length of a string or BLOB
    
    [...] In security-sensitive applications it is best not to try to increase
    the maximum string and blob length. In fact, you might do well to lower
    the maximum string and blob length to something more in the range of a few
    million if that is possible.
    

    @serhiy-storchaka
    Copy link
    Member

    serhiy-storchaka commented Nov 5, 2021

    New changeset 3d42cd9 by Erlend Egeberg Aasland in branch 'main':
    bpo-45243: Use connection limits to simplify sqlite3 tests (GH-29356)
    3d42cd9

    @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
    Labels
    expert-sqlite3 extension-modules C modules in the Modules dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants