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

bpo-31746: Prevent segfaults with uninitialised sqlite3.Connection objects #27431

Merged

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Jul 28, 2021

  • Only set sqlite3.Connection.initialized when the connection object
    is initialised
  • Check initialised flag in close()
  • Check initialised flag in isolation_level
  • Use SQLite API when enabling autocommit mode [1]
  • Add tests

[1] This may happen during connection init, so we cannot use
pysqlite_connection_commit, since it requires an initialised
connection object. Previously, this was not a problem, since the
initialised flag was incorrectly set before the connection object was
initialised.

https://bugs.python.org/issue31746

Comment on lines +373 to +378
if (!self->initialized) {
pysqlite_state *state = pysqlite_get_state(NULL);
PyErr_SetString(state->ProgrammingError,
"Base Connection.__init__ not called.");
return NULL;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We cannot use pysqlite_check_connection here, since it also checks self->db. Calling close() twice should still be a no-op.

@erlend-aasland
Copy link
Contributor Author

Backport to 3.9 as well?

@pablogsal pablogsal merged commit 7e311e4 into python:main Jul 29, 2021
@miss-islington
Copy link
Contributor

Thanks @erlend-aasland for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry, @erlend-aasland and @pablogsal, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 7e311e496b0e26b3d3c62fe9b0ed2a4677c37ee9 3.10

@miss-islington
Copy link
Contributor

Thanks @erlend-aasland for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @erlend-aasland and @pablogsal, I had trouble checking out the 3.9 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 7e311e496b0e26b3d3c62fe9b0ed2a4677c37ee9 3.9

@pablogsal
Copy link
Member

Oh, seems that the backport party crashed :(

@erlend-aasland can you do them manually?

@erlend-aasland erlend-aasland deleted the sqlite-uninitialised-connection branch July 29, 2021 19:46
@erlend-aasland
Copy link
Contributor Author

Oh, seems that the backport party crashed :(

@erlend-aasland can you do them manually?

Yes, I was expecting it to fail. I'll fix it right away. Should we backport to 3.9 as well?

@pablogsal
Copy link
Member

@erlend-aasland apparently we have a buildbot failure:

https://buildbot.python.org/all/#/builders/596/builds/703/

We may need to revert this meanwhile we understand what failed

erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Jul 29, 2021
…alised (pythonGH-27431).

(cherry picked from commit 7e311e4)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@erlend-aasland
Copy link
Contributor Author

Yes, please revert it.

@bedevere-bot
Copy link

GH-27465 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label Jul 29, 2021
pablogsal pushed a commit that referenced this pull request Jul 29, 2021
pablogsal pushed a commit that referenced this pull request Jul 29, 2021
…alised (GH-27431) (GH-27465)

(cherry picked from commit 7e311e4)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@pablogsal
Copy link
Member

We are missing the 3.10 backport still

@erlend-aasland
Copy link
Contributor Author

We are missing the 3.10 backport still

Doh, I'll get to it after morning coffee :)

erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Jul 30, 2021
…ialised (pythonGH-27431).

(cherry picked from commit 7e311e4)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@bedevere-bot
Copy link

GH-27472 is a backport of this pull request to the 3.10 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 30, 2021
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Jul 30, 2021
ambv pushed a commit that referenced this pull request Jul 30, 2021
…ialised (GH-27431). (GH-27472)

(cherry picked from commit 7e311e4)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants