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

Test crash scenario due to Unicode emoji characters #10

Merged
merged 1 commit into from
Oct 12, 2015

Conversation

brodycj
Copy link

@brodycj brodycj commented Oct 8, 2015

Add test to UnicodeTest.java with the crash scenario due to Android bug 81341 on emoji characters, as reported in sqlcipher/android-database-sqlcipher#199. Following the link from Android bug 81341 to https://android-review.googlesource.com/#/c/130121/

Note that the crash scenario is guarded by if (android.os.Build.VERSION.SDK_INT >= 23) which should run the test on Android M/6.X or later, may crash on older M pre-releases. I did not try running this on Android M/6.X myself.

developernotes added a commit that referenced this pull request Oct 12, 2015
Test crash scenario due to Unicode emoji characters
@developernotes developernotes merged commit e7b8502 into sqlcipher:master Oct 12, 2015
@developernotes
Copy link
Member

Thanks @brodybits!

@tnt-medallia
Copy link

i'm working on a fix for this bug. i see the script run-testsuite.sh but i don't see how to build the apk that is required by the script. can someone explain how to build target/net.zetetic.sqlcipher.test.apk

@developernotes
Copy link
Member

Hi @tnt-medallia

The test suite includes the IntelliJ project file, can you try opening that to build the application?

@tnt-medallia
Copy link

thanks for the quick response.

  1. compilation fails because the included sqlcipher.jar is missing a required class: DatabaseErrorHandler.java. i couldn't find a binary to download so i had to pull the source; build it and replace the jar
  2. it would reduce the friction of testing bug fixes if the readme explained all the steps to run the tests

2b. a way to build and run from the command line would be even better.

@developernotes
Copy link
Member

Hello @tnt-medallia

compilation fails because the included sqlcipher.jar is missing a required class: DatabaseErrorHandler.java. i couldn't find a binary to download so i had to pull the source; build it and replace the jar

We will look to update the binaries we are including - thanks!

it would reduce the friction of testing bug fixes if the readme explained all the steps to run the tests

The run-testsuite.sh executes the tests against a compiled application. What do you think we could change to make this more seamless?

2b. a way to build and run from the command line would be even better.

This is the first request we've received asking for this, we can certainly look into this in the future.

@tnt-medallia
Copy link

most of the android world is moving to gradle. incorporating the run-testsuite.sh into a build.gradle that depends on a successful assemble and install seems the logical approach.

@developernotes
Copy link
Member

Hi @tnt-medallia

Sure, that sounds like a good next step, we just recently announced community support for AAR packaging of SQLCipher for Android. We also welcome pull requests!

@tnt-medallia
Copy link

i have a fix for this bug that now passes this test. as this will affect performance i think it would be best to apply it conditionally so that only the errant versions of android are impacted.

i'll add code to get the android api level but before i start i'm wondering if it's already available. if it's there i couldn't find it.

@developernotes
Copy link
Member

Hello @tnt-medallia,

We would be glad to review a pull request and discuss, thanks!

@sevar83
Copy link

sevar83 commented Jun 27, 2016

Hi, I've ported the AOSP JUnit tests for the JNI layer. Some of them are not relevant to SQLCipher but i think gives a better overal idea how it compares with the platform Sqlite API. If you're interested i could upload a ZIP, you only have to change my package name to the original "net.sqlcipher". And btw, in one of the tests there's a native crash in CursorWindow.copyStringToBuffer_native(). Tried on phys. Nexus5X / 6.0.1. Here is the tombstone if it can give you any hints: http://pastebin.com/raw/1qVKSwg6

@developernotes
Copy link
Member

Hi @sevar83

We would be interested in reviewing it, would you be able to publish it to Github with an accessible license? With regard to your crasher, have you tried utilizing the latest 3.5.1 of SQLCipher for Android? It appears you are using spatialite, is that correct?

@sevar83
Copy link

sevar83 commented Jun 27, 2016

Yesterday I manually (but very carefully) picked all the latest changes into my codebase. I use Spatialite on top of SQLite but without crypto. Practically now is 99% the same as SQLCipher. I'll try to publish a project with the unit tests. You could also bundle some of them directly in the android-database-sqlcipher library when you're ready with the gradle build migration. They are pure AOSP.

@sevar83
Copy link

sevar83 commented Jun 28, 2016

Hi again, I pushed the tests here: https://github.com/sevar83/android-sqlcipher-database-unittests/tree/master

Please, check them carefully! I think there are some suspicious failures related to CursorWindow and char buffers. Also 2 tests are marked with @ignore because they are causing native crashes and stop the tests entirely. So I just ignored them to make the rest running. Please take a look also at CursorWrapperTest.testDeactivate - something wrong happens there.

@developernotes
Copy link
Member

Hi @sevar83

Thanks for sharing this!

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.

4 participants