simolus3 / sqlite3.dart

sqlite3 bindings for Dart
MIT License
224 stars 75 forks source link

SQLCipher compile flags consistency #171

Open davidmartos96 opened 1 year ago

davidmartos96 commented 1 year ago

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 https://github.com/simolus3/drift/issues/2032#issuecomment-1247186128
  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 commented 1 year ago

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 commented 1 year ago

@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? https://github.com/dart-lang/sdk/issues/50565

simolus3 commented 1 year ago

Is this the feature you are referring to? https://github.com/dart-lang/sdk/issues/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 commented 1 year ago

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

simolus3 commented 1 year ago

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