629604 - nsJNIString constructor mistakes the empty string for a null/void string

Status: RESOLVED INVALID

(GeckoView :: General, defect)

Description

[jag (Peter Annema)]

GetStringLength(...) doesn't include the zero terminator (and to my knowledge, can't be negative), so the test in the constructor should drop the length check.

If you want to handle the OOM case (!jCharPtr) a little better, you could set up a helper function, like:

nsresult
MakeJNIString(jstring jstr, JNIEnv *jenv, nsString& aStr)
{
    if (!jstr) {
        aStr.SetIsVoid(PR_TRUE);
        return NS_OK;
    }

    JNIEnv *jni = jenv;
    if (!jni)
        jni = JNI();

    const jchar* jCharPtr = jni->GetStringChars(jstr, NULL);
    if (!jCharPtr) {
        // OOM, exception was thrown for us
        aStr.SetIsVoid(PR_TRUE);
        return NS_ERROR_OUT_OF_MEMORY;
    }

    aStr.Assign(jCharPtr, jni->GetStringLength(jstr));

    jni->ReleaseStringChars(jstr, jCharPtr);

    return NS_OK;
}

And then you can drop the nsJNIString type completely.

Comment 1

[jag (Peter Annema)]

Bug 637915 addressed handling empty strings improperly. That leaves handling OOM through a helper function and getting rid of nsJNIString, or just mark this bug WFM.

Comment 2

[David Lawrence [:dkl]]

Moving all open Core::Widget: Android bugs to GeckoView::General (then the triage owner of GeckoView will decide which ones are valuable and which ones should be closed).

Comment 3

[Emily Toop (:fluffyemily)]

Not applicable to GeckoView