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

Throw exception when null value in WhereCondition #438

Closed
wants to merge 1 commit into from

Conversation

jeffdgr8
Copy link

@jeffdgr8 jeffdgr8 commented Sep 9, 2016

It's possible to get an IllegalArgumentException all the way down at the database level if a query was built using null values in any of the WhereConditions. The stack trace looks like this:

Fatal Exception: java.lang.IllegalArgumentException
the bind value at index 1 is null
    net.sqlcipher.database.SQLiteProgram.bindString (SQLiteProgram.java:237)
    net.sqlcipher.database.SQLiteQuery.bindString (SQLiteQuery.java:185)
    net.sqlcipher.database.SQLiteDirectCursorDriver.query (SQLiteDirectCursorDriver.java:48)
    net.sqlcipher.database.SQLiteDatabase.rawQueryWithFactory (SQLiteDatabase.java:1772)
    net.sqlcipher.database.SQLiteDatabase.rawQuery (SQLiteDatabase.java:1737)
    de.greenrobot.dao.database.EncryptedDatabase.rawQuery (EncryptedDatabase.java:31)
    de.greenrobot.dao.query.Query.list (Query.java:76)
    ...

It is difficult to debug the source of the poorly constructed queries, that didn't check for null before creating the WhereConditions, especially when the query was built using an AsyncSession. In that case the stack trace looks like this:

Fatal Exception: de.greenrobot.dao.async.AsyncDaoException
    de.greenrobot.dao.async.AsyncOperation.getResult (AsyncOperation.java:105)
    com.my.code.MyClass.onAsyncOperationCompleted (MyClass.java:42)
    de.greenrobot.dao.async.AsyncOperationExecutor.handleOperationCompleted (AsyncOperationExecutor.java:241)
    de.greenrobot.dao.async.AsyncOperationExecutor.executeOperationAndPostCompleted (AsyncOperationExecutor.java:260)
    de.greenrobot.dao.async.AsyncOperationExecutor.run (AsyncOperationExecutor.java:167)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1080)
    java.lang.Thread.run (Thread.java:841)
Caused by java.lang.IllegalArgumentException
    net.sqlcipher.database.SQLiteProgram.bindString (SQLiteProgram.java:237)
    net.sqlcipher.database.SQLiteQuery.bindString (SQLiteQuery.java:185)
    net.sqlcipher.database.SQLiteDirectCursorDriver.query (SQLiteDirectCursorDriver.java:48)
    net.sqlcipher.database.SQLiteDatabase.rawQueryWithFactory (SQLiteDatabase.java:1758)
    net.sqlcipher.database.SQLiteDatabase.rawQuery (SQLiteDatabase.java:1723)
    de.greenrobot.dao.database.EncryptedDatabase.rawQuery (EncryptedDatabase.java:31)
    de.greenrobot.dao.query.Query.list (Query.java:76)
    de.greenrobot.dao.async.AsyncOperationExecutor.executeOperation (AsyncOperationExecutor.java:311)
    de.greenrobot.dao.async.AsyncOperationExecutor.executeOperationAndPostCompleted (AsyncOperationExecutor.java:259)
    de.greenrobot.dao.async.AsyncOperationExecutor.run (AsyncOperationExecutor.java:167)
    ...

In this case, the IllegalArgumentException does not even log the actual message, at least indicating the index where the null value was found. But even knowing the index, it's much more difficult to figure out which was the specific WhereCondition that bound the value to that index.

It's much more helpful if the WhereCondition itself throws an exception at the time of creation in order to detect the invalid value sooner. The PropertyCondition.checkValueForType() method previously would throw a DaoException in the case of a Date or boolean property type being null. But every other type did not have any explicit checking. This change will throw a DaoException if any null value is found.

It's possible to get an `IllegalArgumentException` all the way down at the database level if a query was built using null values in any of the `WhereConditions`. The stack trace looks like this:

```
Fatal Exception: java.lang.IllegalArgumentException
the bind value at index 1 is null
    net.sqlcipher.database.SQLiteProgram.bindString (SQLiteProgram.java:237)
    net.sqlcipher.database.SQLiteQuery.bindString (SQLiteQuery.java:185)
    net.sqlcipher.database.SQLiteDirectCursorDriver.query (SQLiteDirectCursorDriver.java:48)
    net.sqlcipher.database.SQLiteDatabase.rawQueryWithFactory (SQLiteDatabase.java:1772)
    net.sqlcipher.database.SQLiteDatabase.rawQuery (SQLiteDatabase.java:1737)
    de.greenrobot.dao.database.EncryptedDatabase.rawQuery (EncryptedDatabase.java:31)
    de.greenrobot.dao.query.Query.list (Query.java:76)
    ...
```

It is difficult to debug the source of the poorly constructed queries, that didn't check for null before creating the `WhereCondition`s, especially when the query was built using an `AsyncSession`. In that case the stack trace looks like this:

```
Fatal Exception: de.greenrobot.dao.async.AsyncDaoException
    de.greenrobot.dao.async.AsyncOperation.getResult (AsyncOperation.java:105)
    com.my.code.MyClass.onAsyncOperationCompleted (MyClass.java:42)
    de.greenrobot.dao.async.AsyncOperationExecutor.handleOperationCompleted (AsyncOperationExecutor.java:241)
    de.greenrobot.dao.async.AsyncOperationExecutor.executeOperationAndPostCompleted (AsyncOperationExecutor.java:260)
    de.greenrobot.dao.async.AsyncOperationExecutor.run (AsyncOperationExecutor.java:167)
    java.util.concurrent.ThreadPoolExecutor.runWorker (ThreadPoolExecutor.java:1080)
    java.lang.Thread.run (Thread.java:841)
Caused by java.lang.IllegalArgumentException
    net.sqlcipher.database.SQLiteProgram.bindString (SQLiteProgram.java:237)
    net.sqlcipher.database.SQLiteQuery.bindString (SQLiteQuery.java:185)
    net.sqlcipher.database.SQLiteDirectCursorDriver.query (SQLiteDirectCursorDriver.java:48)
    net.sqlcipher.database.SQLiteDatabase.rawQueryWithFactory (SQLiteDatabase.java:1758)
    net.sqlcipher.database.SQLiteDatabase.rawQuery (SQLiteDatabase.java:1723)
    de.greenrobot.dao.database.EncryptedDatabase.rawQuery (EncryptedDatabase.java:31)
    de.greenrobot.dao.query.Query.list (Query.java:76)
    de.greenrobot.dao.async.AsyncOperationExecutor.executeOperation (AsyncOperationExecutor.java:311)
    de.greenrobot.dao.async.AsyncOperationExecutor.executeOperationAndPostCompleted (AsyncOperationExecutor.java:259)
    de.greenrobot.dao.async.AsyncOperationExecutor.run (AsyncOperationExecutor.java:167)
    ...
```

In this case, the `IllegalArgumentException` does not even log the actual message, at least indicating the index where the null value was found. But even knowing the index, it's much more difficult to figure out which was the specific `WhereCondition` that bound the value to that index.

It's much more helpful if the `WhereCondition` itself throws an exception at the time of creation in order to detect the invalid value sooner. The `PropertyCondition.checkValueForType()` method previously would throw a `DaoException` in the case of a `Date` or `boolean` property type being null. But every other type did not have any explicit checking. This change will throw a `DaoException` if any null value is found.
@jeffdgr8
Copy link
Author

Recreated on clean branch: #1031

@jeffdgr8 jeffdgr8 closed this Apr 28, 2020
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

Successfully merging this pull request may close these issues.

2 participants