Skip to content

Check database file encoding to set appropriate opening mode #3792

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

Closed
lucydodo wants to merge 2 commits into from
Closed

Check database file encoding to set appropriate opening mode #3792

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
25bac8d
Check database file encoding to set appropriate opening mode
lucydodo Nov 2, 2024
afb2c43
Use the preprocessor macro instead of the magic number for the encoding
lucydodo Nov 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion src/sqlitedb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,8 +196,43 @@ bool DBBrowserDB::open(const QString& db, bool readOnly)
if(tryEncryptionSettings(db, &isEncrypted, &cipherSettings) == false)
return false;

// Open database file header to determine encoding
QFile file(db);
if(!file.open(QIODevice::ReadOnly))
{
lastErrorMessage = tr("Cannot open database file: %1").arg(file.errorString());
return false;
}

QByteArray header = file.read(100);
file.close();

const char* encodingBytes = header.constData() + 56;
// The variable 'encoding' will have one of the following values:
// SQLITE_UTF8, SQLITE_UTF16LE, SQLITE_UTF16BE (https://www.sqlite.org/fileformat.html#text_encoding)
quint32 encoding = ((quint8)encodingBytes[0] << 24) |
Copy link
Copy Markdown
Member

@mgrojo mgrojo Nov 2, 2024

Choose a reason for hiding this comment

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

From https://www.sqlite.org/fileformat.html#text_encoding

. The sqlite3.h header file defines C-preprocessor macros SQLITE_UTF8 as 1, SQLITE_UTF16LE as 2, and SQLITE_UTF16BE as 3, to use in place of the numeric codes for the text encoding.

Could you use that instead of these magic numbers? You could also reference that page.

On the other hand, there's no reference to any preprocessor definition for 56, nor to the database header size. Is this the recommended way to determine the encoding? What is the source documentation you have found for this?

Copy link
Copy Markdown
Member Author

@lucydodo lucydodo Nov 2, 2024

Choose a reason for hiding this comment

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

Good idea, changed via afb2c43.

https://www.sqlite.org/fileformat.html#database_header

Regarding the 56 offset, you can refer to the file header specifications provided by SQLite.

((quint8)encodingBytes[1] << 16) |
((quint8)encodingBytes[2] << 8) |
((quint8)encodingBytes[3]);

// Open database file
if(sqlite3_open_v2(db.toUtf8(), &_db, readOnly ? SQLITE_OPEN_READONLY : SQLITE_OPEN_READWRITE, nullptr) != SQLITE_OK)
int status=0;
if(encoding == SQLITE_UTF8)
{
status = sqlite3_open_v2(
db.toUtf8(),
&_db,
readOnly ? SQLITE_OPEN_READONLY : SQLITE_OPEN_READWRITE,
nullptr);
}
else
{
status = sqlite3_open16(db.constData(), &_db);
if (status == SQLITE_OK && readOnly)
sqlite3_exec(_db, "PRAGMA query_only = TRUE;", nullptr, nullptr, nullptr);
Copy link
Copy Markdown
Member

@mgrojo mgrojo Nov 2, 2024

Choose a reason for hiding this comment

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

According to https://www.sqlite.org/pragma.html#pragma_query_only

However, the database is not truly read-only. You can still run a checkpoint or a COMMIT and the return value of the sqlite3_db_readonly() routine is not affected.

Is strange that this is the solution for those two issues (#3714 and #3636). Are we working around some bug in SQLite3? Shouldn't it detect the database encoding and act accordingly?

The documentation for these functions say that they determine the default encoding for databases created using these functions, not when opening already created databases. Everything seems to indicate that SQLite3 should do that automatically.

There's nothing written, but my impression is that sqlite3_open16 is deprecated in favour of sqlite3_open_v2, and you only have to convert to UTF-8 the filename if running on an OS with UTF-16 filenames (I think Windows is like that, but not Linux; I don't know about macOS). Then, if creating the DB, you use "pragma ENCODING" to set the database encoding.

Copy link
Copy Markdown
Member Author

@lucydodo lucydodo Nov 2, 2024

Choose a reason for hiding this comment

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

The sqlite3_open16 function doesn’t provide an option to open the file in read-only mode, unlike sqlite3_open_v2, so that’s why it was done this way.

It might be possible to use a URI filename as shown like this: file:data.db?mode=ro
but to do so, we would need to compile and provide the SQLite binaries ourselves for Linux, macOS, and Windows.

}

if (status != SQLITE_OK)
{
lastErrorMessage = QString::fromUtf8(sqlite3_errmsg(_db));
return false;
Expand Down