8304290: Some JNI calls made without checking exceptions in media

Reviewed-by: arapte, kcr

modules/javafx.media/src/main/java/com/sun/media/jfxmedia/MediaError.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -150,6 +150,7 @@ public enum MediaError {
     ERROR_JNI_SEND_STOP_REACHED_EVENT(ERROR_BASE_JNI.code()+0x000C),
     ERROR_JNI_SEND_DURATION_UPDATE_EVENT(ERROR_BASE_JNI.code()+0x000D),
     ERROR_JNI_SEND_AUDIO_SPECTRUM_EVENT(ERROR_BASE_JNI.code()+0x000E),
+    ERROR_JNI_UNEXPECTED(ERROR_BASE_JNI.code()+0x000F),
 
     ERROR_OSX_INIT(ERROR_BASE_OSX.code() + 0x0001),
 

modules/javafx.media/src/main/native/jfxmedia/Locator/Locator.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -53,23 +53,23 @@ CLocator::LocatorType CLocator::GetType()
 
 jstring CLocator::LocatorGetStringLocation(JNIEnv *env, jobject locator)
 {
-    static jmethodID mid_toString = 0;
+    static jmethodID mid_GetStringLocation = NULL;
     jstring result = NULL;
     CJavaEnvironment javaEnv(env);
 
-    if (mid_toString == 0)
+    if (mid_GetStringLocation == NULL)
     {
         jclass klass = env->GetObjectClass(locator);
 
-        mid_toString = env->GetMethodID(klass, "getStringLocation", "()Ljava/lang/String;");
+        mid_GetStringLocation = env->GetMethodID(klass, "getStringLocation", "()Ljava/lang/String;");
         env->DeleteLocalRef(klass);
-        if (javaEnv.clearException())
+        if (javaEnv.clearException() || mid_GetStringLocation == NULL)
         {
             return NULL;
         }
     }
 
-    result = (jstring)env->CallObjectMethod(locator, mid_toString);
+    result = (jstring)env->CallObjectMethod(locator, mid_GetStringLocation);
     if (javaEnv.clearException())
     {
         return NULL;

modules/javafx.media/src/main/native/jfxmedia/Utils/JavaUtils.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -31,7 +31,6 @@ extern "C" {
 #endif
 
 NSString *NSStringFromJavaString(JNIEnv *env, jstring js);
-jstring JavaStringFromNSString(JNIEnv *env, NSString *ns);
 
 /*
     Returns a valid JNIEnv, if we had to attach the current thread then attached is set

modules/javafx.media/src/main/native/jfxmedia/Utils/JavaUtils.m

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -41,38 +41,6 @@
     return outString;
 }
 
-jstring JavaStringFromNSString(JNIEnv *env, NSString *ns)
-{
-    jstring outString = 0;
-
-    if (NULL != env && nil != ns) {
-        jsize length;
-        if (ns.length > INT32_MAX) {
-            // overflow protection: NSUInteger is 64 bit ulong, jsize is 32 bit int
-            length = INT32_MAX-1;
-        } else {
-            length = (jsize)ns.length;
-        }
-        unichar *strBuf = malloc(length * sizeof(unichar));
-        if (!strBuf) {
-            return 0;
-        }
-
-        @try {
-            [ns getCharacters:strBuf range:NSMakeRange(0, length)];
-        }
-        @catch (NSException *exception) {
-            free(strBuf);
-            return 0;
-        }
-
-        outString = (*env)->NewString(env, strBuf, length);
-        free(strBuf);
-    }
-
-    return outString;
-}
-
 JNIEnv *GetJavaEnvironment(JavaVM *jvm, BOOL *attached)
 {
     JNIEnv *env = NULL;

modules/javafx.media/src/main/native/jfxmedia/jni/JavaBandsHolder.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -82,7 +82,10 @@ void CJavaBandsHolder::UpdateBands(int size, const float* magnitudes, const floa
 
         if (localMagnitudes && localPhases) {
             pEnv->SetFloatArrayRegion(localMagnitudes, 0, size, magnitudes);
-            pEnv->SetFloatArrayRegion(localPhases, 0, size, phases);
+            if (!jenv.clearException()) {
+                pEnv->SetFloatArrayRegion(localPhases, 0, size, phases);
+                jenv.clearException();
+            }
         }
 
         pEnv->DeleteLocalRef(localMagnitudes);

modules/javafx.media/src/main/native/jfxmedia/jni/JavaInputStreamCallbacks.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -59,17 +59,21 @@ bool CJavaInputStreamCallbacks::Init(JNIEnv *env, jobject jLocator)
 
     CJavaEnvironment javaEnv(m_jvm);
 
-    static jmethodID createConnectionHolder = 0;
-    if (0 == createConnectionHolder)
+    static jmethodID createConnectionHolder = NULL;
+    if (NULL == createConnectionHolder)
     {
         jclass klass = env->GetObjectClass(jLocator);
         createConnectionHolder = env->GetMethodID(klass, "createConnectionHolder", "()Lcom/sun/media/jfxmedia/locator/ConnectionHolder;");
         env->DeleteLocalRef(klass);
-        if (javaEnv.reportException())
+        if (javaEnv.reportException() || (NULL == createConnectionHolder))
             return false;
     }
 
-    m_ConnectionHolder = env->NewGlobalRef(env->CallObjectMethod(jLocator, createConnectionHolder));
+    jobject connectionHolder = env->CallObjectMethod(jLocator, createConnectionHolder);
+    if (javaEnv.reportException() || (NULL == connectionHolder))
+        return false;
+
+    m_ConnectionHolder = env->NewGlobalRef(connectionHolder);
     if (NULL == m_ConnectionHolder)
     {
         javaEnv.reportException();
@@ -83,62 +87,61 @@ bool CJavaInputStreamCallbacks::Init(JNIEnv *env, jobject jLocator)
         // Get the parent abstract class. It's wrong to get method ids from the concrete implementation
         // because it crashes jvm when it tries to call virtual methods.
         // See https://javafx-jira.kenai.com/browse/RT-37115
-        jclass klass = NULL;
-        klass = env->FindClass("com/sun/media/jfxmedia/locator/ConnectionHolder");
-        hasException = javaEnv.reportException();
+        jclass klass = env->FindClass("com/sun/media/jfxmedia/locator/ConnectionHolder");
+        hasException = (javaEnv.reportException() || (NULL == klass));
 
         if (!hasException)
         {
             m_BufferFID = env->GetFieldID(klass, "buffer", "Ljava/nio/ByteBuffer;");
-            hasException = javaEnv.reportException();
+            hasException = (javaEnv.reportException() || (NULL == m_BufferFID));
         }
 
         if (!hasException)
         {
             m_NeedBufferMID = env->GetMethodID(klass, "needBuffer", "()Z");
-            hasException = javaEnv.reportException();
+            hasException = (javaEnv.reportException() || (NULL == m_NeedBufferMID));
         }
 
         if (!hasException)
         {
             m_ReadNextBlockMID = env->GetMethodID(klass, "readNextBlock", "()I");
-            hasException = javaEnv.reportException();
+            hasException = (javaEnv.reportException() || (NULL == m_ReadNextBlockMID));
         }
 
         if (!hasException)
         {
             m_ReadBlockMID = env->GetMethodID(klass, "readBlock", "(JI)I");
-            hasException = javaEnv.reportException();
+            hasException = (javaEnv.reportException() || (NULL == m_ReadBlockMID));
         }
 
         if (!hasException)
         {
             m_IsSeekableMID = env->GetMethodID(klass, "isSeekable", "()Z");
-            hasException = javaEnv.reportException();
+            hasException = (javaEnv.reportException() || (NULL == m_IsSeekableMID));
         }
 
         if (!hasException)
         {
             m_IsRandomAccessMID = env->GetMethodID(klass, "isRandomAccess", "()Z");
-            hasException = javaEnv.reportException();
+            hasException = (javaEnv.reportException() || (NULL == m_IsRandomAccessMID));
         }
 
         if (!hasException)
         {
             m_SeekMID = env->GetMethodID(klass, "seek", "(J)J");
-            hasException = javaEnv.reportException();
+            hasException = (javaEnv.reportException() || (NULL == m_SeekMID));
         }
 
         if (!hasException)
         {
             m_CloseConnectionMID = env->GetMethodID(klass, "closeConnection", "()V");
-            hasException = javaEnv.reportException();
+            hasException = (javaEnv.reportException() || (NULL == m_CloseConnectionMID));
         }
 
         if (!hasException)
         {
             m_PropertyMID = env->GetMethodID(klass, "property", "(II)I");
-            hasException = javaEnv.reportException();
+            hasException = (javaEnv.reportException() || (NULL == m_PropertyMID));
         }
 
         if (NULL != klass)
@@ -163,7 +166,7 @@ bool CJavaInputStreamCallbacks::NeedBuffer()
             pEnv->DeleteLocalRef(connection);
         }
 
-        javaEnv.reportException();
+        javaEnv.clearException();
     }
 
     return result;
@@ -179,12 +182,11 @@ int CJavaInputStreamCallbacks::ReadNextBlock()
         jobject connection = pEnv->NewLocalRef(m_ConnectionHolder);
         if (connection) {
             result = pEnv->CallIntMethod(connection, m_ReadNextBlockMID);
+            if (javaEnv.clearException()) {
+                result = -2;
+            }
             pEnv->DeleteLocalRef(connection);
         }
-
-        if (javaEnv.clearException()) {
-            result = -2;
-        }
     }
 
     return result;
@@ -200,12 +202,11 @@ int CJavaInputStreamCallbacks::ReadBlock(int64_t position, int size)
         jobject connection = pEnv->NewLocalRef(m_ConnectionHolder);
         if (connection) {
             result = pEnv->CallIntMethod(connection, m_ReadBlockMID, position, size);
+            if (javaEnv.clearException()) {
+                result = -2;
+            }
             pEnv->DeleteLocalRef(connection);
         }
-
-        if (javaEnv.clearException()) {
-            result = -2;
-        }
     }
 
     return result;
@@ -238,10 +239,9 @@ bool CJavaInputStreamCallbacks::IsSeekable()
         jobject connection = pEnv->NewLocalRef(m_ConnectionHolder);
         if (connection) {
             result = (pEnv->CallBooleanMethod(connection, m_IsSeekableMID) == JNI_TRUE);
+            javaEnv.clearException();
             pEnv->DeleteLocalRef(connection);
         }
-
-        javaEnv.reportException();
     }
 
     return result;
@@ -257,10 +257,9 @@ bool CJavaInputStreamCallbacks::IsRandomAccess()
         jobject connection = pEnv->NewLocalRef(m_ConnectionHolder);
         if (connection) {
             result = (pEnv->CallBooleanMethod(connection, m_IsRandomAccessMID) == JNI_TRUE);
+            javaEnv.clearException();
             pEnv->DeleteLocalRef(connection);
         }
-
-        javaEnv.reportException();
     }
 
     return result;
@@ -276,10 +275,9 @@ int64_t CJavaInputStreamCallbacks::Seek(int64_t position)
         jobject connection = pEnv->NewLocalRef(m_ConnectionHolder);
         if (connection) {
             result = pEnv->CallLongMethod(connection, m_SeekMID, (jlong)position);
+            javaEnv.clearException();
             pEnv->DeleteLocalRef(connection);
         }
-
-        javaEnv.reportException();
     }
 
     return (int64_t)result;
@@ -294,9 +292,8 @@ void CJavaInputStreamCallbacks::CloseConnection()
         jobject connection = pEnv->NewLocalRef(m_ConnectionHolder);
         if (connection) {
             pEnv->CallVoidMethod(connection, m_CloseConnectionMID);
+            javaEnv.clearException();
             pEnv->DeleteLocalRef(connection);
-
-            javaEnv.reportException();
         }
 
         pEnv->DeleteGlobalRef(m_ConnectionHolder);
@@ -314,10 +311,9 @@ int CJavaInputStreamCallbacks::Property(int prop, int value)
         jobject connection = pEnv->NewLocalRef(m_ConnectionHolder);
         if (connection) {
             result = pEnv->CallIntMethod(connection, m_PropertyMID, (jint)prop, (jint)value);
+            javaEnv.clearException();
             pEnv->DeleteLocalRef(connection);
         }
-
-        javaEnv.reportException();
     }
 
     return result;

modules/javafx.media/src/main/native/jfxmedia/jni/JavaMediaWarningListener.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -45,15 +45,15 @@ void CJavaMediaWarningListener::Warning(int warningCode, const char* warningMess
     JNIEnv *pEnv = javaEnv.getEnvironment();
     if (pEnv) {
         jclass mediaUtilsClass = pEnv->FindClass("com/sun/media/jfxmediaimpl/MediaUtils");
-        if (!javaEnv.clearException()) {
-            jmethodID errorMethodID = pEnv->GetStaticMethodID(mediaUtilsClass,
-                                                              "nativeWarning",
-                                                              "(ILjava/lang/String;)V");
-            char* message = NULL == warningMessage ? (char*)"" : (char*)warningMessage;
+        if (!javaEnv.clearException() && mediaUtilsClass != NULL) {
+            jmethodID nativeWarningMethodID = pEnv->GetStaticMethodID(mediaUtilsClass,
+                                                                      "nativeWarning",
+                                                                      "(ILjava/lang/String;)V");
             if (!javaEnv.clearException()) {
+                char* message = NULL == warningMessage ? (char*)"" : (char*)warningMessage;
                 jstring jmessage = pEnv->NewStringUTF(message);
-                if (!javaEnv.clearException()) {
-                    pEnv->CallStaticVoidMethod(mediaUtilsClass, errorMethodID,
+                if (!javaEnv.clearException() && jmessage != NULL) {
+                    pEnv->CallStaticVoidMethod(mediaUtilsClass, nativeWarningMethodID,
                                                (jint)warningCode, jmessage);
                     javaEnv.clearException();
                     pEnv->DeleteLocalRef(jmessage);