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

Avoid using double-quoted string literals in SQL queries #4709

Open
TheRealDev0 opened this issue Mar 14, 2023 · 8 comments · May be fixed by #5236
Open

Avoid using double-quoted string literals in SQL queries #4709

TheRealDev0 opened this issue Mar 14, 2023 · 8 comments · May be fixed by #5236
Labels
bug bugs that are confirmed and actionable

Comments

@TheRealDev0
Copy link

Problem

With a default beets installation (via pkg or ports) on FreeBSD (13.1), any action requiring interaction with the SQLite database will cause beets to crash.

This is due to Double-quoted String Literals being disabled by default in the 3.41.0,1 version of SQLite - enabling DQS in the make options for the SQLite port corrects this issue and allows beets to complete database functions normally.

The maintainers of the SQLite port on FreeBSD have since re-enabled DQS by default but are planning to disable DQS by default no later than 20240101.

Running this command in verbose (-vv) mode:

$ beet -vv ls

Led to this problem:

Traceback (most recent call last):
  File "/usr/local/bin/beet", line 33, in <module>
    sys.exit(load_entry_point('beets==1.6.0', 'console_scripts', 'beet')())
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/ui/__init__.py", line 1285, in main
    _raw_main(args)
  File "/usr/local/lib/python3.11/site-packages/beets/ui/__init__.py", line 1272, in _raw_main
    subcommand.func(lib, suboptions, subargs)
  File "/usr/local/lib/python3.11/site-packages/beets/ui/commands.py", line 1089, in list_func
    list_items(lib, decargs(args), opts.album)
  File "/usr/local/lib/python3.11/site-packages/beets/ui/commands.py", line 1084, in list_items
    for item in lib.items(query):
                ^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/library.py", line 1529, in items
    return self._fetch(Item, query, sort or self.get_default_item_sort())
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/library.py", line 1503, in _fetch
    return super()._fetch(
           ^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/dbcore/db.py", line 1093, in _fetch
    rows = tx.query(sql, subvals)
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/beets/dbcore/db.py", line 858, in query
    cursor = self.db._connection().execute(statement, subvals)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
sqlite3.OperationalError: no such column:

Setup

  • OS: FreeBSD 13.1
  • Python version: 3.9 and 3.11
  • beets version: 1.6.0
  • Turning off plugins made problem go away (yes/no): no

My configuration (output of beet config) is:

library: library.db
directory: /mnt/Music/
import:
   copy: yes
   move: no
@sampsyo sampsyo added the bug bugs that are confirmed and actionable label Mar 24, 2023
@sampsyo sampsyo changed the title Crash when DQS Disabled in SQLite on FreeBSD Avoid using double-quoted string literals in SQL queries Mar 24, 2023
@sampsyo
Copy link
Member

sampsyo commented Mar 24, 2023

Wow; thank you for pointing this out! This looks very serious. For a little more background on the issue, the SQLite docs discuss the syntax compatibility problem:
https://www.sqlite.org/quirks.html#double_quoted_string_literals_are_accepted

Unfortunately, it is going to be tricky in general to track down where we are using this incorrect syntax. Based on the clue from this error message, suggesting that the sting literal in question is empty, I found one such instance:

'WHEN "" THEN {0} '

That should be '' instead of "". But it's hard to tell if that's the only issue. We should start here, though, and hopefully run the whole test suite with DQS disabled to see if there are more similar crashes.

@wisp3rwind
Copy link
Member

[...] and hopefully run the whole test suite with DQS disabled to see if there are more similar crashes.

Unfortunately, sqlite3_db_config does not appear to be exposed in any way in Python, so we can't disable DQS at runtime :/

@reyjrar
Copy link

reyjrar commented May 4, 2024

[...] and hopefully run the whole test suite with DQS disabled to see if there are more similar crashes.

Unfortunately, sqlite3_db_config does not appear to be exposed in any way in Python, so we can't disable DQS at runtime :/

I'm not a Python programmer and I find the namespace and pypi more confusing than dealing with any other languages repositories, but I came across this:

https://docs.python.org/3/library/sqlite3.html#sqlite3.Connection.setconfig

Which seems to expose the thing that's needed for this to work on FreeBSD.

con.setconfig(sqlite3.SQLITE_DBCONFIG_DQS_DDL, 1)
con.setconfig(sqlite3.SQLITE_DBCONFIG_DQS_DML, 1)

That should work on all version of sqlite3 if I'm reading the docs correctly.

UPDATE: Of course, import sqlite3 isn't the same as the docs for this import sqlite3.. This is one of the bigger issues I have with python. There's no way for me to look at a script and figure out which of the things import sqlite3 means.

Nevermind, whatever is satisfying import sqlite3 for the beets project doesn't expose Connection.setconfig(). Yay!

@sungo
Copy link

sungo commented May 7, 2024

FreeBSD appears to have disabled DQS again as foretold. Is there any change I can locally apply to get my setup running again? Looks like the setconfig bits aren't it, yeah?

@wisp3rwind
Copy link
Member

wisp3rwind commented May 7, 2024

That should work on all version of sqlite3 if I'm reading the docs correctly.

UPDATE: Of course, import sqlite3 isn't the same as the docs for this import sqlite3.. This is one of the bigger issues I have with python. There's no way for me to look at a script and figure out which of the things import sqlite3 means.

Nevermind, whatever is satisfying import sqlite3 for the beets project doesn't expose Connection.setconfig(). Yay!

The problem is likely that setconfig was only added in Python 3.12. But we should try to use it conditionally there, which would get us a check by CI which in turn should be sufficient to track down any remaining DQS usage.

EDIT: I'll open a PR for this later.

@arogl
Copy link
Contributor

arogl commented May 7, 2024

A quick look throughthe code I found the reference @sampsyo found, as well as the following 2 references in beets/dbcore/query.py.

I am having trouble testing on windows running python 3.12.3, so looking to help out. I'll start a discussion about windows testing

arogl added a commit to arogl/beets that referenced this issue May 8, 2024
@arogl arogl linked a pull request May 8, 2024 that will close this issue
1 task
@FST777
Copy link

FST777 commented May 8, 2024

If you want to get things working locally on FreeBSD while this issue is still unresolved, you can build sqlite3 from ports with DQS enabled via make config .

@sungo
Copy link

sungo commented May 8, 2024

If you want to get things working locally on FreeBSD while this issue is still unresolved, you can build sqlite3 from ports with DQS enabled via make config .

@FST777 my system is using binary packages and I'm not a fan of mixing ports and packages. but that said, there is a less risky approach, turns out. I built a pyenv of python 3.12 last night, installed beets from git, and manually applied #5235 , setting the values to 1 instead. on freebsd 13.1, that magically got beets up and running again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs that are confirmed and actionable
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants