<AWT Dev> RFR: 8211826 getGTKIcon with Unicode chars got StringIndexOutOfBoundsException

[Ichiroh Takiguchi]

Hello.
Could you review the fix ?

It's fixed invalid usage for JNI's data size calculation.
Small testcase is in JDK-8211826.

Thanks,
Ichiroh Takiguchi
IBM Japan, Ltd.

On 2018-10-08 11:46, Ichiroh Takiguchi wrote:
> Hello.
> 
> Could you review the fix ?
> 
> Bug:    https://bugs.openjdk.java.net/browse/JDK-8211826
> Change: https://cr.openjdk.java.net/~itakiguchi/8211826/webrev.00/
> 
> Thanks,
> Ichiroh Takiguchi
> IBM Japan, Ltd.
> 
> On 2018-06-19 21:33, Ichiroh Takiguchi wrote:
>> Hello,
>> IBM would like to contribute a patch to OpenJDK project.
>> 
>> If a parameter of getGTKIcon contains multi-byte UTF-8 characters,
>> StringIndexOutOfBoundsException may occur.
>> The calculation of string length is confused.
>> We should tell UTF-8’s length and java.lang.String’s length.
>> 
>> Candidate fix is below. Can we have a sponsor of the fix?
>> 
>> ------
>> --- old/src/java.desktop/unix/native/libawt_xawt/awt/awt_UNIXToolkit.c
>>        2018-06-19 21:07:13.873993935 +0900
>> +++ new/src/java.desktop/unix/native/libawt_xawt/awt/awt_UNIXToolkit.c
>>        2018-06-19 21:07:13.252007031 +0900
>> @@ -113,6 +113,7 @@
>>  {
>>  #ifndef HEADLESS
>>      int len;
>> +    jsize jlen;
>>      char *filename_str = NULL;
>>      GError **error = NULL;
>> 
>> @@ -122,6 +123,7 @@
>>      }
>> 
>>      len = (*env)->GetStringUTFLength(env, filename);
>> +    jlen = (*env)->GetStringLength(env, filename);
>>      filename_str = (char *)SAFE_SIZE_ARRAY_ALLOC(malloc,
>>              sizeof(char), len + 1);
>>      if (filename_str == NULL) {
>> @@ -132,7 +134,7 @@
>>          free(filename_str);
>>          return JNI_FALSE;
>>      }
>> -    (*env)->GetStringUTFRegion(env, filename, 0, len, filename_str);
>> +    (*env)->GetStringUTFRegion(env, filename, 0, jlen, filename_str);
>>      jboolean result = gtk->get_file_icon_data(env, filename_str, 
>> error,
>>                                              icon_upcall_method, 
>> this);
>> 
>> @@ -159,6 +161,7 @@
>>  {
>>  #ifndef HEADLESS
>>      int len;
>> +    jsize jlen;
>>      char *stock_id_str = NULL;
>>      char *detail_str = NULL;
>> 
>> @@ -168,25 +171,27 @@
>>      }
>> 
>>      len = (*env)->GetStringUTFLength(env, stock_id);
>> +    jlen = (*env)->GetStringLength(env, stock_id);
>>      stock_id_str = (char *)SAFE_SIZE_ARRAY_ALLOC(malloc,
>>              sizeof(char), len + 1);
>>      if (stock_id_str == NULL) {
>>          JNU_ThrowOutOfMemoryError(env, "OutOfMemoryError");
>>          return JNI_FALSE;
>>      }
>> -    (*env)->GetStringUTFRegion(env, stock_id, 0, len, stock_id_str);
>> +    (*env)->GetStringUTFRegion(env, stock_id, 0, jlen, stock_id_str);
>> 
>>      /* Detail isn't required so check for NULL. */
>>      if (detail != NULL)
>>      {
>>          len = (*env)->GetStringUTFLength(env, detail);
>> +        jlen = (*env)->GetStringLength(env, detail);
>>          detail_str = (char *)SAFE_SIZE_ARRAY_ALLOC(malloc,
>>                  sizeof(char), len + 1);
>>          if (detail_str == NULL) {
>>              JNU_ThrowOutOfMemoryError(env, "OutOfMemoryError");
>>              return JNI_FALSE;
>>          }
>> -        (*env)->GetStringUTFRegion(env, detail, 0, len, detail_str);
>> +        (*env)->GetStringUTFRegion(env, detail, 0, jlen, detail_str);
>>      }
>> 
>>      if (!init_method(env, this) ) {
>> 
>> ------
>> 
>> Note that,
>> src/java.desktop/unix/native/libawt_xawt/awt/swing_GTKEngine.c has
>> same kind of code.
>> But buffer handling is not same, so I could not determine, I need to
>> change swing_GTKEngine.c or not.
>> 
>> Thanks,
>> Ichiroh Takiguchi
>> IBM Japan, Ltd.