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

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

@erlend-aasland erlend-aasland Jul 28, 2021

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

erlend-aasland commented Jul 29, 2021

Backport to 3.9 as well?

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

miss-islington commented Jul 29, 2021

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

miss-islington commented Jul 29, 2021

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

miss-islington commented Jul 29, 2021

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

miss-islington commented Jul 29, 2021

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

pablogsal commented Jul 29, 2021

Oh, seems that the backport party crashed :(

@erlend-aasland can you do them manually?

@erlend-aasland erlend-aasland deleted the sqlite-uninitialised-connection branch Jul 29, 2021
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Jul 29, 2021

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

pablogsal commented Jul 29, 2021

@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 <erlend.aasland@innova.no>
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Jul 29, 2021

Yes, please revert it.

@bedevere-bot
Copy link

bedevere-bot commented Jul 29, 2021

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

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 <erlend.aasland@innova.no>
@pablogsal
Copy link
Member

pablogsal commented Jul 29, 2021

We are missing the 3.10 backport still

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Jul 30, 2021

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 <erlend.aasland@innova.no>
@bedevere-bot
Copy link

bedevere-bot commented Jul 30, 2021

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

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 <erlend.aasland@innova.no>
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.

None yet

5 participants