[9] RFR(S): Crash with assert: symbol conversion failure in java_lang_String::create_from_symbol()

[Tobias Hartmann]

Coleen, David, thanks for the reviews and sorry for the delay!

I agree that we should validate the Strings we get through JNI, but the code in create_from_str() and create_from_symbol() was originally added to check the result of String compression with the Compact Strings feature and not to check for valid UTF-8.

I think UTF-8 validity should be checked earlier (before or during Symbol creation) and this affects other JNI methods that take C-Strings as well (jni_NewString, jni_NewStringUTF, jni_DefineClass, ...). This is a general problem with JNI not validating input Strings.

Are you fine with pushing the proposed fix (webrev.00) and open a follow up bug to fix JNI (and potentially re-enable the Compact Strings asserts because their existence is still justified)?

Thanks,
Tobias

On 12.09.2016 02:47, David Holmes wrote:
> On 10/09/2016 6:55 AM, Coleen Phillimore wrote:
>> This change is fine because it matches the commented out assert in
>> create_from_str().   We should probably figure out what it would take to
>> check the characters coming in from JNI and decide whether we should do
>> this.  If not, it doesn't make sense to have commented out asserts.
>> But this is okay for jdk9.
> 
> Grumble, grumble ... both are bad. If the VM doesn't validate this bad UTF-8 then where does it go? And how does the generator of the bad UTF-8 get informed? An assert may be too drastic but can we throw an exception (InternalError?) ?
> 
> David
> 
>> Thanks,
>> Coleen
>>
>>
>> On 9/9/16 8:42 AM, Tobias Hartmann wrote:
>>> Hi,
>>>
>>> please review the following patch:
>>> https://bugs.openjdk.java.net/browse/JDK-8164561
>>> http://cr.openjdk.java.net/~thartmann/8164561/webrev.00/
>>>
>>> The verification code in java_lang_String::create_from_symbol() that
>>> was added by Compact Strings fails because the input symbol does not
>>> contain valid UTF8. The problem is that a JCK JNI test passes an
>>> invalid UTF8 string as class name to the JNI method "FindClass". In
>>> fact, the string contains garbage from reading past array boundaries
>>> because of a bug in the test [1]. The JNI spec [2] states that 'name'
>>> should be "a fully-qualified class name (that is, a package name,
>>> delimited by “/”, followed by the class name). If the name begins with
>>> “[“ (the array signature character), it returns an array class. The
>>> string is encoded in modified UTF-8".
>>>
>>> I nevertheless think that we should not crash in the case of an
>>> invalid UTF8 string and therefore disabled the verification code with
>>> a comment. We did the same for java_lang_String::create_from_str() [3].
>>>
>>> Tested with failing JCK test and JPRT (running).
>>>
>>> Thanks,
>>> Tobias
>>>
>>> [1] https://bugs.openjdk.java.net/browse/JCK-7307244
>>> [2]
>>> https://docs.oracle.com/javase/8/docs/technotes/guides/jni/spec/functions.html#FindClass
>>>
>>> [3]
>>> http://hg.openjdk.java.net/jdk9/hs/hotspot/file/d060826d0911/src/share/vm/classfile/javaClasses.cpp#l274
>>>
>>