fix for issue #199 "JNI crash when reading russian / emoji character using simpleQueryForString"

[tnt-medallia]

this fix prevents utf crash by converting text to utf16

this change is done by terry turner on behalf of my employer, medallia, inc.

[sjlombardo]

Hello @tnt-medallia Thanks for your recent work on SQLCipher for Android. Before reviewing and potentially merging we need to have a contributor agreement on file. If you and your employer could review the "Entities" CLA, we'd appreciate it. Then, just send us an email to [email protected] and let us know the names and email addresses of your employer, etc, who will sign the document, and we'll send it over for electronic signature. Once we have a CLA in place we can perform further review and testing. Thanks!

[tnt-medallia]

stephen,

please contact

thanks,
terry

On Fri, Dec 4, 2015 at 7:41 AM, Stephen Lombardo [email protected]
wrote:

Hello @tnt-medallia https://github.com/tnt-medallia Thanks for your
recent work on SQLCipher for Android. Before reviewing and potentially
merging we need to have a contributor agreement on file. If you and your
employer could review the "Entities" CLA
https://www.zetetic.net/contributions/, we'd appreciate it. Then, just
send us an email to [email protected] and let us know the names and
email addresses of your employer, etc, who will sign the document, and
we'll send it over for electronic signature. Once we have a CLA in place we
can perform further review and testing. Thanks!

—
Reply to this email directly or view it on GitHub
#206 (comment)
.

[sjlombardo]

@tnt-medallia thanks for the information, I've forwarded a copy over for review.

[tnt-medallia]

when might we expect an update on this?

[sjlombardo]

Hello @tnt-medallia we received the contributor agreement yesterday, so we'll start looking at this soon. Thanks for your patience!

[developernotes]

Hi @tnt-medallia

Thank you for submitting a pull request. Just a few quick comments after taking a look at the pull request. We generally prefer to include separate unit tests that execute changes such as this within the SQLCipher for Android test suite, would you mind creating a few tests cases that exercise these changes? Should a client developer on the Java side check for a return value of 0xFFFD?

[tnt-medallia]

nick,

you wrote:

We generally prefer to include separate unit tests that execute
changes such as this within the
SQLCipher for Android test suite
https://github.com/sqlcipher/sqlcipher-android-tests, would you mind
creating a few tests cases that exercise these
changes?

before i started the fix someone created a unit test for this bug in the
android test project
(https://github.com/sqlcipher/sqlcipher-android-tests). as i recall he
made it test only for versions of android that will pass without this
fix. will enabling that test for all versions be satisfactory?

you wrote:

Should a client developer on the Java side check for a return value
of |0xFFFD|?

from my research it seems that this value is used in the unicode world
to indicate invalid (unrenderable) utf data. there is no need to test
for this value. if bad utf gets into the database it'll show up as
nonsense when displayed.

terry

On 12/14/2015 01:12 PM, Nick Parker wrote:

Hi @tnt-medallia https://github.com/tnt-medallia

Thank you for submitting a pull request. Just a few quick comments
after taking a look at the pull request. We generally prefer to
include separate unit tests that execute changes such as this within
the SQLCipher for Android test suite
https://github.com/sqlcipher/sqlcipher-android-tests, would you mind
creating a few tests cases that exercise these changes? Should a
client developer on the Java side check for a return value of |0xFFFD|?

—
Reply to this email directly or view it on GitHub
#206 (comment).

[brody4hire]

Guys, I had already added a test for the issue in bug #199 in UnicodeTest.java#L22-L28 (see sqlcipher/sqlcipher-android-tests/pull/10:

        if (android.os.Build.VERSION.SDK_INT >= 23) { // Android M
            // This will crash on Android releases 1.X-5.X due the following Android bug:
            // https://code.google.com/p/android/issues/detail?id=81341
            SQLiteStatement st = database.compileStatement("SELECT '\uD83D\uDE03'"); // SMILING FACE (MOUTH OPEN)
            String res = st.simpleQueryForString();
            if (!res.equals("\uD83D\uDE03")) return false;
        }

This test case would crash on Android 5 (or earlier) if the if (android.os.Build.VERSION.SDK_INT >= 23) clause is removed. I suggest we remove this if clause (and test on Android 5 or earlier) to verify that the issue in bug #199 is actually fixed.

[developernotes]

@tnt-medallia @brodybits,

Ok, excellent - thanks for the quick response!

[tnt-medallia]

what is the status of this?

[developernotes]

Hello @tnt-medallia,

We plan to merge this into the master branch, it will then be released publicly following our next mainline SQLCipher release. Thanks!