Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 9 commits into from
May 20, 2022
Merged

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

merged 9 commits into from
May 20, 2022

Conversation

serhiy-storchaka
Copy link
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
Copy link
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
Copy link
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
Copy link
Contributor

@erlend-aasland erlend-aasland left a comment

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.

@serhiy-storchaka serhiy-storchaka added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label May 20, 2022
@bedevere-bot
Copy link

🤖 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
Copy link
Contributor

erlend-aasland commented May 20, 2022 •
edited
Loading

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

@serhiy-storchaka
Copy link
Member Author

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

@erlend-aasland
Copy link
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
Copy link
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
Copy link
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
Copy link

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 <[email protected]>
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 <[email protected]>
@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 berkerpeksag is a code owner

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