Added separate functions UnicodeToWtf8() and Wtf8ToUnicode() to fix an old bug in Android

virxkane · virxkane · commit cbc439070fae · 2019-12-22T20:35:36.000+04:00
that older than M (API23), which can't process 4-byte UTF-8 sequences. How to reproduce this bug: just open a text file with such UTF-8 sequences and you will get a crash with the following error: "input is not valid Modified UTF-8: illegal start byte 0xf0". Reason: unhandled exception: 'input is not valid Modified UTF-8: illegal start byte 0xf0'. stack trace (for commit a1bf2be): _JNIEnv::NewStringUTF(char const*) // jni.h:841 jstring CRJNIEnv::toJavaString( const lString16 & str ) // cr3java.cpp:18 void CRStringField::set( const lString16& str) // cr3java.h:165 JNIEXPORT jobject JNICALL Java_org_coolreader_crengine_DocView_getPositionPropsInternal (JNIEnv * _env, jobject _this, jstring _path) // docview.cpp:1662 at org.coolreader.crengine.DocView.getPositionPropsInternal(Native Method) at org.coolreader.crengine.DocView.getPositionProps(DocView.java:270) at org.coolreader.crengine.ReaderView.preparePageImage(ReaderView.java:3248) at org.coolreader.crengine.ReaderView.access$1700(ReaderView.java:42) at org.coolreader.crengine.ReaderView$LoadDocumentTask.work(ReaderView.java:4829) at org.coolreader.crengine.Engine$TaskHandler.run(Engine.java:180) at android.os.Handler.handleCallback(Handler.java:725) at android.os.Handler.dispatchMessage(Handler.java:92) at android.os.Looper.loop(Looper.java:137) at org.coolreader.crengine.BackgroundThread.run(BackgroundThread.java:122) And solution: just don't pass such strings to env->NewStringUTF(), but pass WTF-8 strings. But this don't fix the crash when CoolReader finds files whose names contain such characters!
diff --git a/android/jni/cr3engine.cpp b/android/jni/cr3engine.cpp
@@ -552,7 +552,9 @@ void cr3androidFatalErrorHandler(int errorCode, const char * errorText )
 /// set fatal error handler
 void crSetFatalErrorHandler( lv_FatalErrorHandler_t * handler );
 
-jboolean initInternal(JNIEnv * penv, jclass obj, jobjectArray fontArray) {
+jboolean initInternal(JNIEnv * penv, jclass obj, jobjectArray fontArray, jint sdk_int) {
+
+	CRJNIEnv::sdk_int = sdk_int;
 
 	CRJNIEnv env(penv);
 
@@ -591,13 +593,13 @@ jboolean initInternal(JNIEnv * penv, jclass obj, jobjectArray fontArray) {
 /*
  * Class:     org_coolreader_crengine_Engine
  * Method:    initInternal
- * Signature: ([Ljava/lang/String;)Z
+ * Signature: ([Ljava/lang/String;I)Z
  */
 JNIEXPORT jboolean JNICALL Java_org_coolreader_crengine_Engine_initInternal
-  (JNIEnv * penv, jclass obj, jobjectArray fontArray)
+  (JNIEnv * penv, jclass obj, jobjectArray fontArray, jint sdk_int)
 {
 	jboolean res = JNI_FALSE;
-	COFFEE_TRY_JNI(penv, res = initInternal(penv, obj, fontArray));
+	COFFEE_TRY_JNI(penv, res = initInternal(penv, obj, fontArray, sdk_int));
 	return res;
 }
 
@@ -775,7 +777,7 @@ JNIEXPORT jboolean JNICALL Java_org_coolreader_crengine_Engine_setKeyBacklightIn
 
 static JNINativeMethod sEngineMethods[] = {
   /* name, signature, funcPtr */
-  {"initInternal", "([Ljava/lang/String;)Z", (void*)Java_org_coolreader_crengine_Engine_initInternal},
+  {"initInternal", "([Ljava/lang/String;I)Z", (void*)Java_org_coolreader_crengine_Engine_initInternal},
   {"uninitInternal", "()V", (void*)Java_org_coolreader_crengine_Engine_uninitInternal},
   {"getFontFaceListInternal", "()[Ljava/lang/String;", (void*)Java_org_coolreader_crengine_Engine_getFontFaceListInternal},
   {"setCacheDirectoryInternal", "(Ljava/lang/String;I)Z", (void*)Java_org_coolreader_crengine_Engine_setCacheDirectoryInternal},
diff --git a/android/jni/cr3java.cpp b/android/jni/cr3java.cpp
@@ -2,20 +2,35 @@
 
 #include <dlfcn.h>
 
+// M is for Marshmallow!
+#define ANDROID_SDK_M	23
+
+uint8_t CRJNIEnv::sdk_int = 0;
+
 lString16 CRJNIEnv::fromJavaString( jstring str )
 {
 	if (!str)
         return lString16::empty_str;
 	jboolean iscopy;
 	const char * s = env->GetStringUTFChars(str, &iscopy);
-	lString16 res(s);
+    lString16 res;
+    if (CRJNIEnv::sdk_int >= ANDROID_SDK_M)
+        res = Utf8ToUnicode(s);
+    else
+        res = Wtf8ToUnicode(s);
 	env->ReleaseStringUTFChars(str, s);
 	return res;
 }
 
 jstring CRJNIEnv::toJavaString( const lString16 & str )
 {
-	return env->NewStringUTF(UnicodeToUtf8(str).c_str());
+    if (CRJNIEnv::sdk_int >= ANDROID_SDK_M)
+        return env->NewStringUTF(UnicodeToUtf8(str).c_str());
+    // To support 4-byte UTF-8 sequence on Android older that 6.0 (API 23),
+    // we encode characters with codes >= 0x10000 to WTF-8.
+    // Otherwise, we have crash with following message:
+    //   "input is not valid Modified UTF-8: illegal start byte 0xf0"
+    return env->NewStringUTF(UnicodeToWtf8(str).c_str());
 }
 
 void CRJNIEnv::fromJavaStringArray( jobjectArray array, lString16Collection & dst )
diff --git a/android/jni/cr3java.h b/android/jni/cr3java.h
@@ -58,6 +58,7 @@ class BitmapAccessorInterface {
 class CRJNIEnv {
 public:
 	JNIEnv * env;
+	static uint8_t sdk_int;
     CRJNIEnv(JNIEnv * pEnv) : env(pEnv) { }
     JNIEnv * operator -> () { return env; }
 	lString16 fromJavaString( jstring str );
diff --git a/android/jni/org_coolreader_crengine_Engine.h b/android/jni/org_coolreader_crengine_Engine.h
diff --git a/android/src/org/coolreader/crengine/Engine.java b/android/src/org/coolreader/crengine/Engine.java
@@ -590,7 +590,7 @@ public void initAgain() {
 		}
 		mFonts = findFonts();
 		findExternalHyphDictionaries();
-		if (!initInternal(mFonts)) {
+		if (!initInternal(mFonts, DeviceInfo.getSDKLevel())) {
 			log.i("Engine.initInternal failed!");
 			throw new RuntimeException("Cannot initialize CREngine JNI");
 		}
@@ -599,7 +599,7 @@ public void initAgain() {
 	}
 
 	// Native functions
-	private native static boolean initInternal(String[] fontList);
+	private native static boolean initInternal(String[] fontList, int sdk_int);
 
 	private native static void uninitInternal();
 
@@ -2022,7 +2022,7 @@ public MountPathCorrector getPathCorrector() {
 		}
 		mFonts = findFonts();
 		findExternalHyphDictionaries();
-		if (!initInternal(mFonts)) {
+		if (!initInternal(mFonts, DeviceInfo.getSDKLevel())) {
 			log.i("Engine.initInternal failed!");
 			throw new RuntimeException("Cannot initialize CREngine JNI");
 		}
diff --git a/crengine/Tools/CMakeLists.txt b/crengine/Tools/CMakeLists.txt
@@ -8,3 +8,4 @@ add_subdirectory(HyphConv)
 add_subdirectory(langstat)
 add_subdirectory(langstat2)
 add_subdirectory(glyphcache_bench)
+add_subdirectory(wtf8-test)
diff --git a/crengine/Tools/wtf8-test/CMakeLists.txt b/crengine/Tools/wtf8-test/CMakeLists.txt
@@ -0,0 +1,26 @@
+
+set(SRC_LIST
+    main.cpp
+)
+
+set(crengine_part_SRC_LIST
+    ../../src/cp_stats.cpp
+    ../../src/crtxtenc.cpp
+    ../../src/lvmemman.cpp
+    ../../src/lvstream.cpp
+    ../../src/lvstring.cpp
+)
+
+#add_definitions(-DBUILD_LITE=1)
+
+if(UNIX)
+    add_definitions(-DLINUX -D_LINUX)
+endif(UNIX)
+
+if(WIN32)
+    add_definitions(-DWIN32 -D_CONSOLE)
+    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -mconsole")
+endif(WIN32)
+
+add_executable(wtf8-test ${SRC_LIST} ${crengine_part_SRC_LIST})
+target_link_libraries(wtf8-test ${STD_LIBS})
diff --git a/crengine/Tools/wtf8-test/main.cpp b/crengine/Tools/wtf8-test/main.cpp
@@ -0,0 +1,42 @@
+#include "lvstring.h"
+
+#include <stdio.h>
+
+lUInt32 uni_chars[] = {
+    0x10000,        // LINEAR B SYLLABLE B008 A
+    0x10123,        // AEGEAN NUMBER TWO THOUSAND
+    0x10081,        // LINEAR B IDEOGRAM B102 WOMAN
+    0x1F600,        // GRINNING FACE
+    0x1F601,        // GRINNING FACE WITH SMILING EYES
+    0x1F602,        // FACE WITH TEARS OF JOY
+    0x1F603,        // SMILING FACE WITH OPEN MOUTH
+    0x1F604,        // SMILING FACE WITH OPEN MOUTH AND SMILING EYES
+    0x1F605,        // SMILING FACE WITH OPEN MOUTH AND COLD SWEAT
+    0x1F606,        // SMILING FACE WITH OPEN MOUTH AND TIGHTLY-CLOSED EYES
+    0x1F607,        // SMILING FACE WITH HALO
+    0x1F608,        // SMILING FACE WITH HORNS
+    0x1F609,        // WINKING FACE
+    0x1F60A,        // SMILING FACE WITH SMILING EYES
+    0x1F60B         // FACE SAVOURING DELICIOUS FOOD
+};
+
+int main(int argc, char* argv[])
+{
+    lString16 src;
+    for (size_t i = 0; i < sizeof(uni_chars)/sizeof(lUInt32); i++)
+    {
+        src.append(1, uni_chars[i]);
+    }
+    lString8 dst = UnicodeToUtf8(src);
+    printf("UTF8: %s\n", dst.c_str());
+    lString8 dstw = UnicodeToWtf8(src);
+    printf("WTF8: %s\n", dstw.c_str());
+    // Back to unicode
+    lString16 str2 = Wtf8ToUnicode(dstw);
+    //   and compare...
+    if (str2.compare(src) == 0)
+        printf("OK, strings is equal.\n");
+    else
+        printf("Sorry, strings is NOT equal!\n");
+    return 0;
+}
diff --git a/crengine/include/lvstring.h b/crengine/include/lvstring.h
@@ -1030,6 +1030,10 @@ lString8  UnicodeToLocal( const lString16 & str );
 lString8  UnicodeToUtf8( const lString16 & str );
 /// converts wide unicode string to utf-8 string
 lString8 UnicodeToUtf8(const lChar16 * s, int count);
+/// converts wide unicode string to wtf-8 string
+lString8  UnicodeToWtf8( const lString16 & str );
+/// converts wide unicode string to wtf-8 string
+lString8 UnicodeToWtf8(const lChar16 * s, int count);
 /// converts unicode string to 8-bit string using specified conversion table
 lString8  UnicodeTo8Bit( const lString16 & str, const lChar8 * * table );
 /// converts 8-bit string to unicode string using specified conversion table for upper 128 characters
@@ -1044,6 +1048,12 @@ lString16 Utf8ToUnicode( const char * s );
 lString16 Utf8ToUnicode( const char * s, int sz );
 /// converts utf-8 string fragment to wide unicode string
 void Utf8ToUnicode(const lUInt8 * src,  int &srclen, lChar16 * dst, int &dstlen);
+/// converts wtf-8 string to wide unicode string
+lString16 Wtf8ToUnicode( const lString8 & str );
+/// converts utf-8 c-string to wide unicode string
+lString16 Wtf8ToUnicode( const char * s );
+/// converts utf-8 string fragment to wide unicode string
+lString16 Wtf8ToUnicode( const char * s, int sz );
 /// decodes path like "file%20name" to "file name"
 lString16 DecodeHTMLUrlString( lString16 s );
 /// truncates string by specified size, appends ... if truncated, prefers to wrap whole words
diff --git a/crengine/src/lvstring.cpp b/crengine/src/lvstring.cpp

Original file line number	Diff line number	Diff line change
`@@ -590,7 +590,7 @@ public void initAgain() {`
`590`	`590`	`}`
`591`	`591`	`mFonts = findFonts();`
`592`	`592`	`findExternalHyphDictionaries();`
`593`		`- if (!initInternal(mFonts)) {`
	`593`	`+ if (!initInternal(mFonts, DeviceInfo.getSDKLevel())) {`
`594`	`594`	`log.i("Engine.initInternal failed!");`
`595`	`595`	`throw new RuntimeException("Cannot initialize CREngine JNI");`
`596`	`596`	`}`
`@@ -599,7 +599,7 @@ public void initAgain() {`
`599`	`599`	`}`
`600`	`600`
`601`	`601`	`// Native functions`
`602`		`- private native static boolean initInternal(String[] fontList);`
	`602`	`+ private native static boolean initInternal(String[] fontList, int sdk_int);`
`603`	`603`
`604`	`604`	`private native static void uninitInternal();`
`605`	`605`
`@@ -2022,7 +2022,7 @@ public MountPathCorrector getPathCorrector() {`
`2022`	`2022`	`}`
`2023`	`2023`	`mFonts = findFonts();`
`2024`	`2024`	`findExternalHyphDictionaries();`
`2025`		`- if (!initInternal(mFonts)) {`
	`2025`	`+ if (!initInternal(mFonts, DeviceInfo.getSDKLevel())) {`
`2026`	`2026`	`log.i("Engine.initInternal failed!");`
`2027`	`2027`	`throw new RuntimeException("Cannot initialize CREngine JNI");`
`2028`	`2028`	`}`