8313040: Enhanced Font handling

Reviewed-by: rhalade, prr, kcr

Author: Lukasz Kostyra

modules/javafx.graphics/src/main/java/com/sun/javafx/font/PrismFontFactory.java

@@ -954,15 +954,14 @@ private String findFile(String name) {
     private static String sysFontDir = null;
     private static String userFontDir = null;
 
-    private static native byte[] getFontPath();
+    private static native String getFontPath();
 
     private static void getPlatformFontDirs() {
 
         if (userFontDir != null || sysFontDir != null) {
             return;
         }
-        byte [] pathBytes = getFontPath();
-        String path = new String(pathBytes);
+        String path = getFontPath();
 
         int scIdx = path.indexOf(';');
         if (scIdx < 0) {

modules/javafx.graphics/src/main/native-font/coretext.c

@@ -672,7 +672,13 @@ JNIEXPORT jlong JNICALL OS_NATIVE(CFStringCreateWithCharacters__J_3CJJ)
 {
     jchar *lparg1=NULL;
     jlong rc = 0;
-    if (arg1) if ((lparg1 = (*env)->GetPrimitiveArrayCritical(env, arg1, NULL)) == NULL) goto fail;
+    if (!arg1) goto fail;
+    if ((lparg1 = (*env)->GetPrimitiveArrayCritical(env, arg1, NULL)) == NULL) goto fail;
+    if (arg2 < 0) goto fail;
+    if (arg3 < 0) goto fail;
+    if (arg2 > LONG_MAX - arg3) goto fail;
+    if (arg2 + arg3 > (*env)->GetArrayLength(env, arg1)) goto fail;
+
     UniChar* str = lparg1 + arg2;
     rc = (jlong)CFStringCreateWithCharacters((CFAllocatorRef)arg0, str, (CFIndex)arg3);
 fail:

modules/javafx.graphics/src/main/native-font/directwrite.cpp

@@ -1517,6 +1517,8 @@ JNIEXPORT jint JNICALL OS_NATIVE(JFXTextRendererGetGlyphIndices)
     if (!arg1) return 0;
     jint* data = env->GetIntArrayElements(arg1, NULL);
     if (!data) return 0;
+    if (start < 0) return 0;
+    if (slot < 0) return 0;
 
     JFXTextRenderer* renderer = (JFXTextRenderer*)arg0;
     // Type cast unsigned int to int. It is safe to assume that GetGlyphCount will never exceed max of jint
@@ -1538,6 +1540,7 @@ JNIEXPORT jint JNICALL OS_NATIVE(JFXTextRendererGetGlyphAdvances)
     if (!arg1) return 0;
     jfloat* data = env->GetFloatArrayElements(arg1, NULL);
     if (!data) return 0;
+    if (start < 0) return 0;
 
     JFXTextRenderer* renderer = (JFXTextRenderer*)arg0;
     // Type cast unsigned int to int. It is safe to assume that GetGlyphCount will never exceed max of jint
@@ -1559,12 +1562,14 @@ JNIEXPORT jint JNICALL OS_NATIVE(JFXTextRendererGetGlyphOffsets)
     if (!arg1) return 0;
     jfloat* data = env->GetFloatArrayElements(arg1, NULL);
     if (!data) return 0;
+    if (start < 0) return 0;
 
     JFXTextRenderer* renderer = (JFXTextRenderer*)arg0;
     // Type cast unsigned int to int. It is safe to assume the result will never exceed max of jint
     jint offsetCount = (jint) renderer->GetGlyphCount() * 2;
     jint length = env->GetArrayLength(arg1);
     jint copiedCount = length - start > offsetCount ? offsetCount : length - start;
+    if (copiedCount % 2 != 0) return 0;
 
     const DWRITE_GLYPH_OFFSET* offsets = renderer->GetGlyphOffsets();
     UINT32 i = 0, j = 0;
@@ -1582,6 +1587,8 @@ JNIEXPORT jint JNICALL OS_NATIVE(JFXTextRendererGetClusterMap)
     if (!arg1) return 0;
     jshort* data = env->GetShortArrayElements(arg1, NULL);
     if (!data) return 0;
+    if (start < 0) return 0;
+    if (glyphStart < 0) return 0;
 
     JFXTextRenderer* renderer = (JFXTextRenderer*)arg0;
     // Type cast unsigned int to int. It is safe to assume that GetLength will never exceed max of jint
@@ -1890,6 +1897,8 @@ JNIEXPORT jlong JNICALL OS_NATIVE(CreateTextLayout)
     IDWriteTextLayout* result = NULL;
     jchar *lparg1 = NULL;
     if (arg1) if ((lparg1 = env->GetCharArrayElements(arg1, NULL)) == NULL) goto fail;
+    if (start < 0 || count < 0) goto fail;
+    if (count > INT_MAX - start) goto fail;
     if (start + count > env->GetArrayLength(arg1)) goto fail;
 
     const WCHAR * text = (const WCHAR *)(lparg1 + start);

modules/javafx.graphics/src/main/native-font/fontpath.c

@@ -33,8 +33,6 @@
 #include <jni.h>
 #include <com_sun_javafx_font_PrismFontFactory.h>
 
-#define BSIZE (max(512, MAX_PATH+1))
-
 /* Typically all local references held by a JNI function are automatically
  * released by JVM when the function returns. However, there is a limit to the
  * number of local references that can remain active. If the local references
@@ -64,56 +62,141 @@ JNIEXPORT jint JNICALL JNI_OnLoad_javafx_font(JavaVM *vm, void *reserved) {
 }
 #endif // STATIC_BUILD
 
-JNIEXPORT jbyteArray JNICALL
+JNIEXPORT jstring JNICALL
 Java_com_sun_javafx_font_PrismFontFactory_getFontPath(JNIEnv *env, jobject thiz)
 {
-    char windir[BSIZE];
-    char sysdir[BSIZE];
-    char fontpath[BSIZE*2];
-    char *end;
-    jbyteArray byteArrObj;
-    int pathLen;
-    unsigned char *data;
+    const wchar_t* const FONTS_DIR = L"\\Fonts";
+    const UINT FONTS_DIR_STRLEN = (UINT)wcslen(FONTS_DIR);
+    errno_t err = 0;
+    wchar_t* windir = NULL;
+    UINT windirBufSize = 0; // total buffer size in wchar_t elements (including NULL terminator)
+    UINT windirFilledLen = 0; // amount of wchar_t elements filled (excluding NULL terminator)
+    wchar_t* sysdir = NULL;
+    UINT sysdirBufSize = 0;
+    UINT sysdirFilledLen = 0;
+    wchar_t* fontpath = NULL;
+    UINT fontpathBufSize = 0;
+    UINT fontpathFilledLen = 0;
+    wchar_t *end = NULL;
+    jsize pathLen = 0;
+    jstring stringObj = NULL;
+
+    /* Preallocate dummy 1-char buffers in case WinAPI wants to write something despite count being 0.
+     *
+     * Windows API doesn't explicitly say that calling GetSystem/GetWindowsDirectory(NULL, 0)
+     * will NOT attempt writing to NULL, so we want to provide something to prevent access
+     * violation if they do
+     */
+    sysdir = calloc(1, sizeof(wchar_t));
+    windir = calloc(1, sizeof(wchar_t));
+    if (!sysdir || !windir) goto finish;
 
     /* Locate fonts directories relative to the Windows System directory.
      * If Windows System location is different than the user's window
      * directory location, as in a shared Windows installation,
      * return both locations as potential font directories
      */
-    GetSystemDirectory(sysdir, BSIZE);
-    end = strrchr(sysdir,'\\');
-    if (end && (stricmp(end,"\\System") || stricmp(end,"\\System32"))) {
-        *end = 0;
-         strcat(sysdir, "\\Fonts");
+    UINT ret = GetSystemDirectoryW(sysdir, 0);
+    if (ret == 0) {
+        goto finish;
+    } else {
+        /* if buffer is too small, GetSystemDirectory will return the length in wchar_t-s
+         * that's needed to allocate the buffer, including terminating null character.
+         * `*Len` variables keep amount of elements without the NULL-terminator, so appropriate
+         * corrections have to be applied here.
+         */
+        free(sysdir);
+        sysdirBufSize = ret + FONTS_DIR_STRLEN;
+        sysdir = calloc(sysdirBufSize, sizeof(wchar_t));
+        if (!sysdir) goto finish;
+        ret = GetSystemDirectoryW(sysdir, sysdirBufSize - FONTS_DIR_STRLEN);
+        if (ret == 0 || ret >= (sysdirBufSize - FONTS_DIR_STRLEN)) {
+            goto finish;
+        }
+        // write was successful so now ret contains only characters written (without NULL-terminator)
+        sysdirFilledLen = ret;
+    }
+
+    /* Figure out fonts location based on acquired System directory - commonly it is one level up
+     * from sysdir and then in "Fonts" dir
+     */
+    end = wcsrchr(sysdir, '\\');
+    if (end) {
+        const wchar_t* const SYSTEM_DIR = L"\\System";
+        const wchar_t* const SYSTEM32_DIR = L"\\System32";
+
+        UINT sysdirToEnd = (UINT)(end - sysdir);
+        UINT endLen = sysdirFilledLen - sysdirToEnd;
+
+        // If the last directory in sysdir is "System" or "System32", strip that and replace it with "\Fonts"
+        if ((_wcsnicmp(end, SYSTEM_DIR, endLen) == 0) || (_wcsnicmp(end, SYSTEM32_DIR, endLen) == 0)) {
+            *end = 0;
+            // no length re-check here, both \System and \System32 are longer than \Fonts string
+            sysdirFilledLen -= endLen;
+            err = wcsncat_s(sysdir, sysdirBufSize, FONTS_DIR, FONTS_DIR_STRLEN);
+            if (err != 0) goto finish;
+            sysdirFilledLen += FONTS_DIR_STRLEN;
+        }
     }
 
-    GetWindowsDirectory(windir, BSIZE);
-    if (strlen(windir) > BSIZE-7) {
-        *windir = 0;
+    // Acquire Windows' directory - follows same assumptions as sysdir above
+    ret = GetWindowsDirectoryW(windir, 0);
+    if (ret == 0) {
+        goto finish;
     } else {
-        strcat(windir, "\\Fonts");
+        free(windir);
+        windirBufSize = ret + FONTS_DIR_STRLEN;
+        windir = calloc(windirBufSize, sizeof(wchar_t));
+        if (!windir) goto finish;
+        ret = GetWindowsDirectoryW(windir, windirBufSize - FONTS_DIR_STRLEN);
+        if (ret == 0 || ret >= (windirBufSize - FONTS_DIR_STRLEN)) {
+            goto finish;
+        }
+        windirFilledLen = ret;
     }
 
-    strcpy(fontpath,sysdir);
-    if (stricmp(sysdir,windir)) {
-        strcat(fontpath,";");
-        strcat(fontpath,windir);
+    // "Fonts" directory should be placed right inside Windows directory, so just append it to the path
+    err = wcsncat_s(windir, windirBufSize, FONTS_DIR, FONTS_DIR_STRLEN);
+    if (err != 0) goto finish;
+    windirFilledLen += FONTS_DIR_STRLEN;
+
+    // Copy sysdir to final string that we'll return to JVM
+    fontpathBufSize = windirFilledLen + sysdirFilledLen + 2; // add semicolon and zero-terminator
+    fontpath = calloc(fontpathBufSize, sizeof(wchar_t));
+    if (!fontpath) goto finish;
+    err = wcsncpy_s(fontpath, fontpathBufSize, sysdir, sysdirFilledLen);
+    if (err != 0) goto finish;
+    fontpathFilledLen = sysdirFilledLen;
+
+    /* JFX expects either one path, or two separated by a semicolon.
+     * If sysdir and windir are different, form a complete path "list" by
+     * joining them in "<sysdir>;<windir>" pattern. JVM side will unpack it.
+     */
+    if (_wcsnicmp(sysdir, windir, min(sysdirFilledLen, windirFilledLen))) {
+        err = wcsncat_s(fontpath, fontpathBufSize, L";", 1);
+        if (err != 0) goto finish;
+        err = wcsncat_s(fontpath, fontpathBufSize, windir, windirFilledLen);
+        if (err != 0) goto finish;
+        fontpathFilledLen += windirFilledLen + 1;
     }
 
-    pathLen = strlen(fontpath);
+    pathLen = (jsize)wcsnlen_s(fontpath, fontpathBufSize);
+
+    // Lastly, return what we just created to JVM as a String
+    stringObj = (*env)->NewString(env, fontpath, fontpathFilledLen);
 
-    byteArrObj = (*env)->NewByteArray(env, pathLen);
-    if (byteArrObj == NULL) {
-        return (jbyteArray)NULL;
+finish:
+    if (sysdir != NULL) {
+        free(sysdir);
+    }
+    if (windir != NULL) {
+        free(windir);
     }
-    data = (*env)->GetByteArrayElements(env, byteArrObj, NULL);
-    if (data == NULL) {
-        return byteArrObj;
+    if (fontpath != NULL) {
+        free(fontpath);
     }
-    memcpy(data, fontpath, pathLen);
-    (*env)->ReleaseByteArrayElements(env, byteArrObj, (jbyte*) data, (jint)0);
 
-    return byteArrObj;
+    return stringObj;
 }
 
 /* The code below is used to obtain information from the windows font APIS

modules/javafx.graphics/src/main/native-font/freetype.c

@@ -31,12 +31,19 @@
 #include <dlfcn.h>
 #include <ft2build.h>
 #include <stdint.h>
+#include <errno.h>
 #include FT_FREETYPE_H
 #include FT_OUTLINE_H
 #include FT_LCD_FILTER_H
 
 #define OS_NATIVE(func) Java_com_sun_javafx_font_freetype_OSFreetype_##func
 
+#define SAFE_FREE(PTR)  \
+    if ((PTR) != NULL) {  \
+        free(PTR);     \
+        (PTR) = NULL;     \
+    }
+
 extern jboolean checkAndClearException(JNIEnv *env);
 #ifdef STATIC_BUILD
 JNIEXPORT jint JNICALL
@@ -485,35 +492,54 @@ JNIEXPORT jint JNICALL OS_NATIVE(FT_1Set_1Char_1Size)
 /***********************************************/
 
 #define F26DOT6TOFLOAT(n) (float)n/64.0;
-static const int DEFAULT_LEN_TYPES = 10;
-static const int DEFAULT_LEN_COORDS = 50;
+static const size_t DEFAULT_LEN_TYPES = 10;
+static const size_t DEFAULT_LEN_COORDS = 50;
 typedef struct _PathData {
     jbyte* pointTypes;
-    int numTypes;
-    int lenTypes;
+    size_t numTypes;
+    size_t lenTypes;
     jfloat* pointCoords;
-    int numCoords;
-    int lenCoords;
+    size_t numCoords;
+    size_t lenCoords;
 } PathData;
 
 static PathData* checkSize(void* user, int coordCount)
 {
     PathData* info = (PathData *)user;
+
     if (info->numTypes == info->lenTypes) {
+        if (info->lenTypes > SIZE_MAX - DEFAULT_LEN_TYPES) goto fail;
         info->lenTypes += DEFAULT_LEN_TYPES;
-        info->pointTypes = (jbyte*)realloc(info->pointTypes, info->lenTypes * sizeof(jbyte));
+
+        jbyte* newPointTypes = (jbyte*)realloc(info->pointTypes, info->lenTypes * sizeof(jbyte));
+        if (newPointTypes == NULL) goto fail;
+        info->pointTypes = newPointTypes;
     }
+
     if (info->numCoords + (coordCount * 2) > info->lenCoords) {
+        if (info->lenCoords > SIZE_MAX - DEFAULT_LEN_COORDS) goto fail;
         info->lenCoords += DEFAULT_LEN_COORDS;
-        info->pointCoords = (jfloat*)realloc(info->pointCoords, info->lenCoords * sizeof(jfloat));
+
+        jbyte* newPointCoords = (jfloat*)realloc(info->pointCoords, info->lenCoords * sizeof(jfloat));
+        if (newPointCoords == NULL) goto fail;
+        info->pointCoords = newPointCoords;
     }
+
     return info;
+
+fail:
+    SAFE_FREE(info->pointTypes);
+    SAFE_FREE(info->pointCoords);
+    return NULL;
 }
 
 static int JFX_Outline_MoveToFunc(const FT_Vector*   to,
                                   void*              user)
 {
     PathData *info = checkSize(user, 1);
+    if (info == NULL) {
+        return FT_Err_Array_Too_Large;
+    }
     info->pointTypes[info->numTypes++] = 0;
     info->pointCoords[info->numCoords++] = F26DOT6TOFLOAT(to->x);
     info->pointCoords[info->numCoords++] = -F26DOT6TOFLOAT(to->y);
@@ -524,6 +550,9 @@ static int JFX_Outline_LineToFunc(const FT_Vector*   to,
                                   void*              user)
 {
     PathData *info =  checkSize(user, 1);
+    if (info == NULL) {
+        return FT_Err_Array_Too_Large;
+    }
     info->pointTypes[info->numTypes++] = 1;
     info->pointCoords[info->numCoords++] = F26DOT6TOFLOAT(to->x);
     info->pointCoords[info->numCoords++] = -F26DOT6TOFLOAT(to->y);
@@ -535,6 +564,9 @@ static int JFX_Outline_ConicToFunc(const FT_Vector*  control,
                                    void*             user )
 {
     PathData *info = checkSize(user, 2);
+    if (info == NULL) {
+        return FT_Err_Array_Too_Large;
+    }
     info->pointTypes[info->numTypes++] = 2;
     info->pointCoords[info->numCoords++] = F26DOT6TOFLOAT(control->x);
     info->pointCoords[info->numCoords++] = -F26DOT6TOFLOAT(control->y);
@@ -549,6 +581,9 @@ static int JFX_Outline_CubicToFunc(const FT_Vector*  control1,
                                    void*             user)
 {
     PathData *info = checkSize(user, 3);
+    if (info == NULL) {
+        return FT_Err_Array_Too_Large;
+    }
     info->pointTypes[info->numTypes++] = 3;
     info->pointCoords[info->numCoords++] = F26DOT6TOFLOAT(control1->x);
     info->pointCoords[info->numCoords++] = -F26DOT6TOFLOAT(control1->y);
@@ -583,12 +618,16 @@ JNIEXPORT jobject JNICALL OS_NATIVE(FT_1Outline_1Decompose)
     data.pointTypes = (jbyte*)malloc(sizeof(jbyte) * DEFAULT_LEN_TYPES);
     data.numTypes = 0;
     data.lenTypes = DEFAULT_LEN_TYPES;
+    if (data.pointTypes == NULL) goto fail;
+
     data.pointCoords = (jfloat*)malloc(sizeof(jfloat) * DEFAULT_LEN_COORDS);
     data.numCoords = 0;
     data.lenCoords = DEFAULT_LEN_COORDS;
+    if (data.pointCoords == NULL) goto fail;
 
     /* Decompose outline */
-    FT_Outline_Decompose(outline, &JFX_Outline_Funcs, &data);
+    FT_Error ftError = FT_Outline_Decompose(outline, &JFX_Outline_Funcs, &data);
+    if (ftError != FT_Err_Ok) goto fail;
 
     static jclass path2DClass = NULL;
     static jmethodID path2DCtr = NULL;
@@ -628,9 +667,10 @@ JNIEXPORT jobject JNICALL OS_NATIVE(FT_1Outline_1Decompose)
             goto fail;
         }
     }
+
 fail:
-    free(data.pointTypes);
-    free(data.pointCoords);
+    SAFE_FREE(data.pointTypes);
+    SAFE_FREE(data.pointCoords);
     return path2D;
 }
 

modules/javafx.graphics/src/main/native-glass/win/Utils.h

@@ -414,6 +414,10 @@ class JBufferArray {
             if (!arr) {
                 data = (T*)env->GetDirectBufferAddress(buf);
             } else {
+                if (offs < 0 || offs > env->GetArrayLength(arr)) {
+                    fprintf(stderr, "Failed to attach bytes array\n");
+                    return;
+                }
                 array.Attach(env, arr);
                 offset = offs;
             }