-
Notifications
You must be signed in to change notification settings - Fork 134
fix(android): migrate from android-database-sqlcipher
to sqlcipher-android
#607
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
base: master
Are you sure you want to change the base?
Conversation
@robingenz Can we get this merged? |
Unfortunately, I don't have the capacity to test it at the moment. So this PR will have to wait or someone else from the Capacitor community takes over. |
Thank you! |
I tried these change on this version of the plugin And the changes seem to be working fine when deployed on a 16kb page size device. Without the changes in this PR, the app does crash on launch on a 16kb page size device. If merged, we also need to patch this on the 6.x version of the plugin for capacitor 6 users. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Migrate from net.zetetic:android-database-sqlcipher
to net.zetetic:sqlcipher-android
to support Android 15's 16 KB page size.
- Swapped imports, casts, and method calls to use
net.zetetic.database.sqlcipher
types and overloads. - Replaced
SQLiteDatabase.loadLibs(context)
withSystem.loadLibrary("sqlcipher")
and updatedopenDatabase
andopenOrCreateDatabase
signatures. - Changed conversion of passwords to bytes and updated exception handling in
changePassword
.
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
UtilsSQLite.java | Replaced net.sqlcipher.Cursor with SQLiteCursor and adjusted casts. |
UtilsSQLCipher.java | Swapped native library loading, updated database open calls with new overloads, and tweaked exception. |
UtilsDrop.java | Updated imports and casts to use SQLiteCursor for table/view/index/trigger queries. |
Database.java | Swapped library init, adjusted password byte conversion, open/creation calls, and cursor usage. |
Files not reviewed (1)
- android/build.gradle: Language not supported
Comments suppressed due to low confidence (3)
android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/UtilsSQLCipher.java:207
- [nitpick] The parameter name
nwpassword
is unclear; rename it tonewPassword
for readability and consistency.
public void changePassword(Context ctxt, File file, String password, String nwpassword) throws Exception {
android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/UtilsSQLCipher.java:214
- Throwing a generic
Exception
loses error specificity; consider usingSQLiteException
or a custom exception type to preserve context.
throw new Exception("database " + file.getAbsolutePath() + " open failed");
android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/Database.java:117
- Calling
System.loadLibrary
directly may miss other native dependencies; verify that all required native libraries are loaded or use the library’s recommended initialization method.
System.loadLibrary("sqlcipher");
android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/Database.java
Outdated
Show resolved
Hide resolved
android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/Database.java
Show resolved
Hide resolved
@@ -59,7 +58,14 @@ public State getDatabaseState(Context ctxt, File dbPath, SharedPreferences share | |||
} catch (Exception e1) { | |||
try { | |||
if (globVar.secret.length() > 0) { | |||
db = SQLiteDatabase.openDatabase(dbPath.getAbsolutePath(), globVar.secret, null, SQLiteDatabase.OPEN_READONLY); | |||
db = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The linting error might be coming from this section of UtilsSQLCipher.java
Once the linting error is fixed, I can approve and merge this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi!
This PR is for @capacitor-community/sqlite@6.0.2
and linter works.
npm run prettier -- --check
> @capacitor-community/sqlite@6.0.2 prettier
> prettier "**/*.{css,html,ts,js,java}" --check
Checking formatting...
[warn] jsxBracketSameLine is deprecated.
All matched files use Prettier code style!
Error on CI tries to use version 7.x (I don't know why)
Run npm run lint
> @capacitor-community/sqlite@[7](https://github.com/capacitor-community/sqlite/actions/runs/15118907452/job/42548687676?pr=607#step:5:8).0.0 lint
> npm run eslint && npm run prettier -- --check && npm run swiftlint -- lint
> @capacitor-community/sqlite@7.0.0 eslint
> eslint . --ext ts
> @capacitor-community/sqlite@7.0.0 prettier
> prettier "**/*.{css,html,ts,js,java}" --plugin=prettier-plugin-java --check
Checking formatting...
[warn] android/src/main/java/com/getcapacitor/community/database/sqlite/SQLite/UtilsSQLCipher.java
[warn] Code style issues found in the above file. Run Prettier with --write to fix.
Error: Process completed with exit code 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There must be a 6.x branch in the repo, you'll need to make the 6.x PR to that branch.
@shiv19 Thank you for helping out!
Please do that and let me know how it goes. Then we can merge it. This change is pretty central. So I want to be sure.
I always publish updates only for the latest Capacitor version supported to keep the maintenance effort as low as possible. Otherwise we would also have to test both versions intensively. |
Should I update the PR to version 7 of the plugin? |
@dragermrb Isn't it already Capacitor 7? Your PR goes against the master branch. What do you need to change? |
I created the PR for version 6.0.2 (master branch at the time). Therefore, the fork used to develop the PR is based on version 6.0.2 and does not include the changes introduced in version 7.x. Linter, for example, differs between versions 6 and 7. |
@dragermrb So yes, please update to Capacitor 7. |
Fixed |
Here is a dev release for testing:
If no bugs are reported in the next few days, I'll merge it. |
android-database-sqlcipher
to sqlcipher-android
@robingenz, we've shipped this fix to prod (minus the last commit with the UTF-8 encoding param). Our QA didn't find any issues, and it also passed the google app review. We're monitoring crash analytics closely. If you don't hear back from me here, it means it's all good ✅️. |
Pull request to migrate from
net.zetetic:android-database-sqlcipher
tonet.zetetic:sqlcipher-android
to allow using an Android 15 (API Level 35) with 16 KB page size supports.
Related issue: #594