Possibility to define own onCorruption-method?

[TheNephilim88]

Regarding this: https://code.google.com/p/android/issues/detail?id=10127 it would may be useful to be able to define an own onCorruption-method (via DatabaseErrorHandler). Did I overlook the possibility here or is it missing?

Background of the question is, that very few of my users are running into the problem, that when they want to access their encrypted data, their password will be accepted, but then suddenly no data are there. Thats why I assume that the openDatabase-method in https://github.com/sqlcipher/android-database-sqlcipher/blob/master/src/net/sqlcipher/database/SQLiteDatabase.java triggers a SQLiteDatabaseCorruptException, which leads to deletion of the database file and a new one will be created. As I am not able to reproduce this problem it would be nice to be able to override the behaviour like its possible in the issue I linked above.

[developernotes]

Hello @TheNephilim88

There is an existing pull request (i.e., #84) that addresses this, however there was an issue and no tests submitted, so it was not merged into the mainstream. The reason the API didn't originally contain support for a DatabaseErrorHandler is the Java API is based on Android 7 snapshot, DatabaseErrorHandler was added in API 11.

[brodybits]

If I am not mistaken, https://code.google.com/p/android/issues/detail?id=10127 has a test database attached to a comment that can reproduce this condition. If I can reproduce the corrupt database condition I will raise a new, updated pull request to address this condition.

[brodybits]

I have a solution available in https://github.com/brodybits/android-database-sqlcipher/tree/corrupt-db-handler-update but there are a couple things that I would like to fix before issuing a PR.

I noticed #142, where a couple versions of SQLiteDatabase.openDatabase() would ignore the flags parameter. Looking carefully, I don't expect it to have significant impact on these changes but want to be absolutely sure.

In this project, SQLiteDatabase.java only constructs a valid SQLiteDatabase object if it has successfully opened or created the database. But in order to follow the normal Android SQLiteDatabase API as closely as possible, we do need to have a valid SQLiteDatabase object constructed, with the database file name set when we call onCorruption() on the DatabaseErrorHandler object. In my solution, I made and labeled a HORRIBLE HACK that constructs a special SQLiteDatabase object upon detecting a corrupt database, in order to follow the normal SQLiteDatabase API. This SQLiteDatabase object has only a limited number of fields set/initialized.

In this case, I think we need to all check all public functions on SQLiteDatabase to make sure we will not have undefined behavior, meaning any form of unexpected or unforeseen behavior, just in case someone tries calling them on the special SQLiteDatabase object.

I already went through the public SQLiteDatabase functions and marked the most of the ones where we should make sure the database is open. But there are some public SQLiteDatabase functions that completely depend upon what the native code does. In these cases we may want to also have the native functions verify that the database is open.

I think it is a good idea for most of the public SQLiteDatabase functions to check the database state anyway and have started working on a test to cover these.

I would like to look at the issue in #142; test and propose the changes to the public SQLiteDatabase functions to check the database state; and then raise the fix for this issue in a PR. Maybe next week.

Any feedback?

[developernotes]

I noticed #142, where a couple versions of SQLiteDatabase.openDatabase() would ignore the flags parameter. Looking carefully, I don't expect it to have significant impact on these changes but want to be absolutely sure.

Hmm, that is probably something we should address.

In my solution, I made and labeled a HORRIBLE HACK that constructs a special SQLiteDatabase object upon detecting a corrupt database, in order to follow the normal SQLiteDatabase API.

Was this due to the ignored flags above, or for another reason?

I think it is a good idea for most of the public SQLiteDatabase functions to check the database state anyway and have started working on a test to cover these.

Are you just referring to the constructors? Are you looking to get the return value of dbopen(…)?

[brodybits]

I noticed #142, where a couple versions of SQLiteDatabase.openDatabase() would ignore the flags parameter. Looking carefully, I don't expect it to have significant impact on these changes but want to be absolutely sure.

Hmm, that is probably something we should address.

My fix for this issue, which was reported in #142, was already merged. Perhaps #142 should be closed.

[developernotes]

My fix for this issue, which was reported in #142, was already merged. Perhaps #142 should be closed.

Done, thanks!

[brodybits]

In my solution, I made and labeled a HORRIBLE HACK that constructs a special SQLiteDatabase object upon detecting a corrupt database, in order to follow the normal SQLiteDatabase API.

Was this due to the ignored flags above

Nope! I will try to explain my reasoning tomorrow.

I think it is a good idea for most of the public SQLiteDatabase functions to check the database state anyway and have started working on a test to cover these.

Are you just referring to the constructors?

No, I was not referring to the constructors here. Again, I will try to explain it tomorrow.

[brodybits]

I still have not pushed my solution since I discovered that in the Android project, it will also call the onCorruption() callback if it detects a database is corrupted during at least certain sql operations(s). Will take another look within the next 1-2 weeks.

[brodybits]

I still have not pushed my solution since I discovered that in the Android project, it will also call the onCorruption() callback if it detects a database is corrupted during at least certain sql operations(s).

I am now looking to port the newer Android database classes to use SQLCipher as described in https://github.com/sqlcipher/sqlcipher/issues/125#issuecomment-110079660 and do not think it is worth fixing here.

[developernotes]

@brodybits - ok, that sounds good.

[brodybits]

I just pushed some changes to support the custom database corruption error handler in #192 (with tests in sqlcipher/sqlcipher-android-tests#9).

[brodybits]

Changes have been merged, thanks @developernotes!

[developernotes]

Thank you @brodybits!

[TheNephilim88]

Thanks!!