Skip to content

gh-91922: Fix sqlite connection on nonstardard locales and paths#92926

Merged
serhiy-storchaka merged 9 commits into
python:mainfrom
serhiy-storchaka:sqlite-connect-non-utf8
May 20, 2022
Merged

gh-91922: Fix sqlite connection on nonstardard locales and paths#92926
serhiy-storchaka merged 9 commits into
python:mainfrom
serhiy-storchaka:sqlite-connect-non-utf8

Conversation

@serhiy-storchaka

@serhiy-storchaka serhiy-storchaka commented May 18, 2022

Copy link
Copy Markdown
Member
  • Fix function sqlite.connect() and the sqlite.Connection constructor
    on non-UTF-8 locales.
  • Fix support of bytes paths non-decodable with the current FS encoding.

Closes #91922

@serhiy-storchaka
pythongh-91922: Fix sqlite connection on nonstardard locales and paths
dbfbf42 
* Fix function sqlite.connect() and the sqlite.Connection constructor
  on non-UTF-8 locales.
* Fix support of bytes paths non-decodable with the current FS encoding.
@serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.10 only security fixes needs backport to 3.11 only security fixes topic-sqlite3 labels May 18, 2022
@serhiy-storchaka serhiy-storchaka mentioned this pull request May 18, 2022
Closed
@erlend-aasland

erlend-aasland commented May 18, 2022

Copy link
Copy Markdown
Contributor

Thanks, I'll review it tomorrow! From a quick glance, it seems like the main change is the PyOS_FSPath conversion, right?

@serhiy-storchaka

serhiy-storchaka commented May 19, 2022

Copy link
Copy Markdown
Member Author

No, the main change is calling PySys_Audit() before encoding the database path to char *.

PyOS_FSPath() is used instead of PyUnicode_FSConverter() in the module-level function to avoid unneeded encoding at that stage. In is not necessary change now (I thought it was necessary at the beginning of work on this issue).

erlend-aasland
erlend-aasland approved these changes May 19, 2022
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good; thanks a lot!

I left some minor comments.

Comment thread Lib/test/test_sqlite3/test_dbapi.py
Comment thread Lib/test/test_sqlite3/test_dbapi.py Outdated
Comment thread Modules/_sqlite/module.c Outdated
Comment thread Modules/_sqlite/connection.c
@serhiy-storchaka serhiy-storchaka added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 20, 2022
@bedevere-bot

bedevere-bot commented May 20, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @serhiy-storchaka for commit 8f58dcf 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 20, 2022
@erlend-aasland

erlend-aasland commented May 20, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

The tree buildbot failures so far all seem unrelated to this PR.

@serhiy-storchaka

serhiy-storchaka commented May 20, 2022

Copy link
Copy Markdown
Member Author

I ran tests on buildbots because the issue is locale-specific.

@erlend-aasland

erlend-aasland commented May 20, 2022

Copy link
Copy Markdown
Contributor

BTW, if you want me to land this, add me as assigned.

@serhiy-storchaka
serhiy-storchaka merged commit d853758 into python:main May 20, 2022
@miss-islington

miss-islington commented May 20, 2022

Copy link
Copy Markdown
Contributor

Thanks @serhiy-storchaka for the PR 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@serhiy-storchaka
serhiy-storchaka deleted the sqlite-connect-non-utf8 branch May 20, 2022 08:53
@miss-islington

miss-islington commented May 20, 2022

Copy link
Copy Markdown
Contributor

Sorry, @serhiy-storchaka, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker d8537580921b2e02f477ff1a8dedcf82c24ef0c2 3.10

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label May 20, 2022
@bedevere-bot

bedevere-bot commented May 20, 2022

Copy link
Copy Markdown

GH-93006 is a backport of this pull request to the 3.11 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 20, 2022
@serhiy-storchaka @miss-islington
pythongh-91922: Fix sqlite connection on nonstardard locales and paths (
dee0c69 
pythonGH-92926)

(cherry picked from commit d853758)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
miss-islington added a commit that referenced this pull request May 20, 2022
@miss-islington @serhiy-storchaka
gh-91922: Fix sqlite connection on nonstardard locales and paths (GH-…
6ec050f 
…92926)

(cherry picked from commit d853758)

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
@serhiy-storchaka serhiy-storchaka removed the needs backport to 3.10 only security fixes label May 21, 2022
@serhiy-storchaka serhiy-storchaka removed their assignment May 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@erlend-aasland erlend-aasland erlend-aasland approved these changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag

Assignees

No one assigned

Labels

topic-sqlite3 type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test_sqlite3 fails on non-UTF-8 locale

4 participants

@serhiy-storchaka @erlend-aasland @bedevere-bot @miss-islington