From db6a9834d2b109e81cc73ef3c64d0c420237e9dc Mon Sep 17 00:00:00 2001 From: Volker Hilsheimer Date: Fri, 3 Jan 2025 16:18:09 +0100 Subject: [PATCH] JNI: don't access null jobject When Java calls a native function that expects a QString with a null String object, then we get a nullptr jstring. We must not try to convert that to a QString. Add the necessary checks at the call-site of the toQString helper, and an assert to the helper itself. Add test case. Pick-to: 6.9 Change-Id: If5c55c702bdb3f89ac45fdf3912af2ac295f5e5c Reviewed-by: Volker Krause --- src/corelib/kernel/qjniarray.h | 10 ++++++---- src/corelib/kernel/qjnitypes.h | 4 ++-- src/corelib/kernel/qjnitypes_impl.h | 1 + .../testdata/QtJniObjectTestClass.java | 5 +++++ .../kernel/qjniobject/tst_qjniobject.cpp | 19 +++++++++++++++++++ 5 files changed, 33 insertions(+), 6 deletions(-) diff --git a/src/corelib/kernel/qjniarray.h b/src/corelib/kernel/qjniarray.h index 68268790632..8a4af8775e8 100644 --- a/src/corelib/kernel/qjniarray.h +++ b/src/corelib/kernel/qjniarray.h @@ -819,12 +819,14 @@ public: container.reserve(sz); if constexpr (std::is_same_v) { for (auto element : *this) { - if constexpr (std::is_same_v) + if constexpr (std::is_same_v) { container.emplace_back(element); - else if constexpr (std::is_same_v) - container.emplace_back(QtJniTypes::Detail::toQString(element, env)); - else + } else if constexpr (std::is_same_v) { + container.emplace_back(element ? QtJniTypes::Detail::toQString(element, env) + : QString{}); + } else { container.emplace_back(QJniObject(element).toString()); + } } } else if constexpr (std::is_base_of_v, std::remove_pointer_t>) { for (auto element : *this) diff --git a/src/corelib/kernel/qjnitypes.h b/src/corelib/kernel/qjnitypes.h index 37b0bde2bd2..d9ad6b673fe 100644 --- a/src/corelib/kernel/qjnitypes.h +++ b/src/corelib/kernel/qjnitypes.h @@ -163,7 +163,7 @@ struct JNITypeForArgImpl static QString fromVarArg(Type t) { - return QtJniTypes::Detail::toQString(t, QJniEnvironment::getJniEnv()); + return t ? QtJniTypes::Detail::toQString(t, QJniEnvironment::getJniEnv()) : QString(); } }; @@ -190,7 +190,7 @@ public: static QList fromVarArg(Type t) { - return QJniArray(t).toContainer(); + return t ? QJniArray(t).toContainer() : QList{}; } }; diff --git a/src/corelib/kernel/qjnitypes_impl.h b/src/corelib/kernel/qjnitypes_impl.h index 6324ace17de..2af91a01a21 100644 --- a/src/corelib/kernel/qjnitypes_impl.h +++ b/src/corelib/kernel/qjnitypes_impl.h @@ -24,6 +24,7 @@ static inline jstring fromQString(const QString &string, JNIEnv *env) static inline QString toQString(jstring string, JNIEnv *env) { + Q_ASSERT(string); const jsize length = env->GetStringLength(string); QString res(length, Qt::Uninitialized); env->GetStringRegion(string, 0, length, reinterpret_cast(res.data())); diff --git a/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java b/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java index 2d2b98c8358..9df7df95412 100644 --- a/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java +++ b/tests/auto/corelib/kernel/qjniobject/testdata/src/org/qtproject/qt/android/testdata/QtJniObjectTestClass.java @@ -307,6 +307,7 @@ public class QtJniObjectTestClass native public int callbackWithRawArray(Object[] value); native public int callbackWithQList(double[] value); native public int callbackWithStringList(String[] value); + native public int callbackWithNull(String str); public int callMeBackWithObject(QtJniObjectTestClass that) { @@ -358,6 +359,10 @@ public class QtJniObjectTestClass { return callbackWithStringList(value); } + public int callMeBackWithNull() + { + return callbackWithNull(null); + } public Object callMethodThrowsException() throws Exception { throw new Exception(); diff --git a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp index 10b48dfa975..a0bd9004342 100644 --- a/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp +++ b/tests/auto/corelib/kernel/qjniobject/tst_qjniobject.cpp @@ -1959,6 +1959,7 @@ enum class CallbackParameterType RawArray, QList, QStringList, + Null, }; static std::optional calledWithObject; @@ -2047,6 +2048,14 @@ static int callbackWithStringList(JNIEnv *, jobject, const QStringList &value) } Q_DECLARE_JNI_NATIVE_METHOD(callbackWithStringList) +static std::optional calledWithNull; +static int callbackWithNull(JNIEnv *, jobject, const QString &str) +{ + calledWithNull.emplace(str); + return int(CallbackParameterType::Null); +} +Q_DECLARE_JNI_NATIVE_METHOD(callbackWithNull) + void tst_QJniObject::callback_data() { QTest::addColumn("parameterType"); @@ -2062,6 +2071,7 @@ void tst_QJniObject::callback_data() QTest::addRow("RawArray") << CallbackParameterType::RawArray; QTest::addRow("QList") << CallbackParameterType::QList; QTest::addRow("QStringList") << CallbackParameterType::QStringList; + QTest::addRow("Null") << CallbackParameterType::Null; } void tst_QJniObject::callback() @@ -2171,6 +2181,15 @@ void tst_QJniObject::callback() QCOMPARE(calledWithStringList.value(), strings); break; } + case CallbackParameterType::Null: { + QVERIFY(TestClass::registerNativeMethods({ + Q_JNI_NATIVE_METHOD(callbackWithNull) + })); + result = testObject.callMethod("callMeBackWithNull"); + QVERIFY(calledWithNull); + QCOMPARE(calledWithNull.value(), QString()); + break; + } } QCOMPARE(result, int(parameterType)); }