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

Unintentional changes in sqlite3.connect() #93044

Closed
serhiy-storchaka opened this issue May 21, 2022 · 3 comments · Fixed by #93046
Closed

Unintentional changes in sqlite3.connect() #93044

serhiy-storchaka opened this issue May 21, 2022 · 3 comments · Fixed by #93046
Labels
3.11 only security fixes topic-sqlite3 type-bug An unexpected behavior, bug, or error

Comments

@serhiy-storchaka
Copy link
Member

There are some unintentional consequences of converting sqlite3.connect() to Argument Clinic in #40956.

  1. The database argument always converted to bytes.
  2. Keyword arguments are passed as positional arguments to factory().
  3. If an argument is not passed to sqlite3.connect(), its default value is passed to factory().

It all works with the default factory=Connection, but factory can be an arbitrary callable, not completely compatible with Connection(). There may be a user code which uses a factory which only works with string database and does not support the uri argument. It worked fine in older versions when it is called with correct arguments, bet will become failing in 3.11.

This is a hypothetical scenario, but we should at least add a note about potential incompatibility. And I think that it is better to get rid of PyUnicode_FSConverter here.

@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error 3.11 only security fixes topic-sqlite3 labels May 21, 2022
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue May 21, 2022
…nnect() to bytes

Just pass it to the factory as is.
@erlend-aasland
Copy link
Contributor

FTR, sqlite3 was converted to Argument Clinic in gh-85128 (bpo-40956).

@erlend-aasland
Copy link
Contributor

Sounds good to me.

serhiy-storchaka added a commit that referenced this issue May 21, 2022
serhiy-storchaka added a commit to serhiy-storchaka/cpython that referenced this issue May 21, 2022
…ite3.connect() to bytes (pythonGH-93046)

Just pass it to the factory as is..
(cherry picked from commit 14c0d33)

Co-authored-by: Serhiy Storchaka <[email protected]>
serhiy-storchaka added a commit that referenced this issue May 21, 2022
…onnect() to bytes (GH-93046) (GH-93048)

Just pass it to the factory as is.
(cherry picked from commit 14c0d33)

Co-authored-by: Serhiy Storchaka <[email protected]>
erlend-aasland pushed a commit that referenced this issue Jul 23, 2022
…ctory (#95146)

This PR partially reverts gh-24421 (PR) and fixes the remaining concerns
given in gh-93044 (issue):

- keyword arguments are passed as positional arguments to factory()
- if an argument is not passed to sqlite3.connect(), its default value
  is passed to factory()

Co-authored-by: Serhiy Storchaka <[email protected]>
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this issue Jul 23, 2022
…connect to factory (pythonGH-95146)

This PR partially reverts pythongh-24421 (PR) and fixes the remaining concerns
given in pythongh-93044 (issue):

- keyword arguments are passed as positional arguments to factory()
- if an argument is not passed to sqlite3.connect(), its default value
  is passed to factory()

Co-authored-by: Serhiy Storchaka <[email protected]>.
(cherry picked from commit a3d4d15)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
erlend-aasland pushed a commit that referenced this issue Jul 23, 2022
…t to factory (GH-95146) (#95158)

This PR partially reverts gh-24421 (PR) and fixes the remaining concerns
given in gh-93044 (issue):

- keyword arguments are passed as positional arguments to factory()
- if an argument is not passed to sqlite3.connect(), its default value
  is passed to factory()

Co-authored-by: Serhiy Storchaka <[email protected]>.
(cherry picked from commit a3d4d15)

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
Labels
3.11 only security fixes topic-sqlite3 type-bug An unexpected behavior, bug, or error
Projects
None yet
2 participants