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

SQLCipher compile flags consistency #171

Open
davidmartos96 opened this issue Jul 27, 2023 · 5 comments
Open

SQLCipher compile flags consistency #171

davidmartos96 opened this issue Jul 27, 2023 · 5 comments

Comments

@davidmartos96
Copy link
Contributor

Hello!
I was looking into using the linux library for an existing Android/iOS app that uses sqlcipher_flutter_libs.
I've found that the flags used on Linux and Windows have some differences between iOS and Android that can break an existing app implementation.

Here are the flags used on the dependencies maintained by the SQLCipher team (Android, iOS and macOS)
https://github.com/sqlcipher/sqlcipher/blob/master/SQLCipher.podspec.json

"compiler_flags": [
        "-DNDEBUG",
        "-DSQLITE_HAS_CODEC",
        "-DSQLITE_TEMP_STORE=2",
        "-DSQLITE_SOUNDEX",
        "-DSQLITE_THREADSAFE",
        "-DSQLITE_ENABLE_RTREE",
        "-DSQLITE_ENABLE_STAT3",
        "-DSQLITE_ENABLE_STAT4",
        "-DSQLITE_ENABLE_COLUMN_METADATA",
        "-DSQLITE_ENABLE_MEMORY_MANAGEMENT",
        "-DSQLITE_ENABLE_LOAD_EXTENSION",
        "-DSQLITE_ENABLE_FTS4",
        "-DSQLITE_ENABLE_FTS4_UNICODE61",
        "-DSQLITE_ENABLE_FTS3_PARENTHESIS",
        "-DSQLITE_ENABLE_UNLOCK_NOTIFY",
        "-DSQLITE_ENABLE_JSON1",
        "-DSQLITE_ENABLE_FTS5",
        "-DSQLCIPHER_CRYPTO_CC",
        "-DHAVE_USLEEP=1",
        "-DSQLITE_MAX_VARIABLE_NUMBER=99999"
],

Differences found

  1. THREADSAFE is 2 instead of 1 like on sqlite3_flutter_libs and like on the team maintained flags. Related SqliteException(5): database is locked drift#2032 (comment)
  2. sqlite3_flutter_libs and sqlcipher_flutter_libs (linux and windows) use the flag DQS=0 which disables the usage of double quotes as string literal. The SQLCipher maintained libs don't set that flag, so existing apps that have used the Android and iOS dependency may use double quotes in many different queries. Using the Linux or Windows dependency now breaks those queries.

1 is an easy fix, like it was done for `sqlite3_flutter_libs' v0.5.10

For 2 I can see two solutions.

  1. Remove the DQS flag on Windows and Linux to match the Android, iOS and macOS flags from the SQLCipher team.

  2. Compile SQLCipher for Android, iOS and macOS with the same set of flags from sqlite3_flutter_libs so that drift with all platforms and engines have the same behavior.
    Android SQLCipher appears to be agnostic to the flags used. https://github.com/sqlcipher/android-database-sqlcipher#building
    I'm not sure about distribution on gradle though.
    As for iOS and macOS they use the following podfile https://github.com/sqlcipher/sqlcipher/blob/master/SQLCipher.podspec.json
    But I'm not sure if it's possible to override the complete set of flags to use the ones we want.
    This solution would be breaking, but at least we could have same behavior across the board.

@simolus3
Copy link
Owner

1 is an easy fix, like it was done for sqlite3_flutter_libs v0.5.10

I agree we should use the default of 1 instead of 2 for SQLCipher builds as well, done.

As you've mentioned, issue 2 is a bit more troublesome. We can't add compile-time options to a podspec from another podspec (it can be done from the root Podfile, but we have no control over that). I also prefer to use these existing libraries where available to reduce the maintenance work (especially since we have to be really careful not to link both sqlite3 and SQLCipher on iOS).

The compatibility argument between Linux/Windows vs. Android/macOS/iOS is compelling, but I think there should also be compatibility between sqlite3_flutter_libs and sqlcipher_flutter_libs.


With the native assets feature currently being developed for the Dart and Flutter SDK, we will likely have a cross-platform solution in the future that works for both Dart and Flutter and allows us to write compile scripts independent of the native build system used in the end. Since that puts the responsibility of linking the right library on iOS and macOS away from us, I think that would be a good opportunity to migrate away from all native build scripts once it is stable. The current situation is not ideal, but I wonder if we should just document it and leave it as is until we have full control over the build.

@davidmartos96
Copy link
Contributor Author

@simolus3 Thanks for the information!
2 is not really a problem for me right now, but it definitely caught me off guard when I saw many queries randomly failing with a Syntax error I wasn't aware about, so I think it was worth mentioning. I understand how DQS=0 can be useful when starting from scratch and avoid using legacy syntax. I always thought SQLite treated double quotes and single quotes the same way, so my project mostly uses double quotes as is what I'm used to in my keyboard.

Is this the feature you are referring to? dart-lang/sdk#50565

@simolus3
Copy link
Owner

Is this the feature you are referring to? dart-lang/sdk#50565

Exactly.

It seems this like this will also support user-defines, so we will likely be able to let application developers decide whether they need DQS or not.

@davidmartos96
Copy link
Contributor Author

@simolus3 A simple reminder just in case 😄
It looks like version 0.5.7 of sqlcipher_flutter_libs with the THREADSAFE fix hasn't been uploaded to pub yet.

@simolus3
Copy link
Owner

Thanks for the reminder, I've just published 0.5.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants