From 3e9a14e6e47aeb26cd55aacead8123d39f9bdee1 Mon Sep 17 00:00:00 2001 From: Alex Andres Date: Sun, 1 Nov 2026 15:34:55 +0100 Subject: [PATCH] fix: keep strings intact across JNI and bind both AudioResampler initializers Native strings went back through NewStringUTF, which takes Modified UTF-8, so emoji and embedded NULs came back changed, and empty strings became null. Convert UTF-8 to UTF-16 properly and keep "" as "", mapping to null only where WebRTC encodes an absent value as empty or unset. WebRTC's error text, which carries track ids and SDP lines, was used as the printf format of the thrown exception and also reached Java in Modified UTF-8. Pass it as an argument and convert it the same way. The JNI header lacked the long names of the overloaded AudioResampler.initialize, so initialize(int, int, int) had C++ linkage and Java bound it to the parameterless function. --- .../jni-voithos/include/JavaString.h | 9 +- .../jni-voithos/include/JavaThrowable.h | 22 +- .../jni-voithos/src/JavaString.cpp | 189 +++++++++++++- .../jni-voithos/src/JavaThrowable.cpp | 8 + .../jni-voithos/src/JavaUtils.cpp | 27 +- .../src/main/cpp/include/JNI_AudioResampler.h | 12 +- .../main/cpp/include/JNI_VideoDesktopSource.h | 8 - .../src/main/cpp/src/JNI_AudioResampler.cpp | 4 +- .../main/cpp/src/JNI_RTCPeerConnection.cpp | 10 +- .../src/main/cpp/src/JNI_RTCRtpSender.cpp | 4 +- .../main/cpp/src/JNI_RTCRtpTransceiver.cpp | 5 +- .../main/cpp/src/JNI_VideoDesktopSource.cpp | 9 - .../cpp/src/api/AddIceCandidateObserver.cpp | 2 +- .../src/api/RTCDataChannelSendObserver.cpp | 2 +- .../src/api/RTCEncodedFrameTransformer.cpp | 5 +- .../src/main/cpp/src/api/RTCIceCandidate.cpp | 5 +- .../src/main/cpp/src/api/RTCIceServer.cpp | 7 +- .../api/RTCPeerConnectionIceErrorEvent.cpp | 8 +- .../src/api/SetSessionDescriptionObserver.cpp | 2 +- .../dev/onvoid/webrtc/NativeStringTests.java | 241 ++++++++++++++++++ .../dev/onvoid/webrtc/RTCDtmfSenderTests.java | 20 +- 21 files changed, 531 insertions(+), 68 deletions(-) create mode 100644 webrtc/src/test/java/dev/onvoid/webrtc/NativeStringTests.java diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaString.h b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaString.h index cd867002b..dde8b1bd5 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaString.h +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaString.h @@ -13,6 +13,7 @@ #include #include +#include namespace jni { @@ -21,13 +22,19 @@ namespace jni public: explicit JavaString(JNIEnv * env); + // UTF-16 to UTF-8; null becomes "". static std::string toNative(JNIEnv * env, const JavaRef & jstr); + // UTF-8 to UTF-16: "" stays "", U+0000 and characters beyond the + // BMP survive. Throws no C++ exception; null with a pending Java + // exception if the string cannot be made. static JavaLocalRef toJava(JNIEnv * env, const std::string & str); + // For values that WebRTC leaves empty when they are absent and that + // Java reports as null. + static JavaLocalRef toJavaOrNull(JNIEnv * env, const std::string & str); static JavaLocalRef createArray(JNIEnv * env, const std::vector & vector); private: jclass cls; - jmethodID getBytes; }; } diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaThrowable.h b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaThrowable.h index e907b3eb1..02a80ee8c 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaThrowable.h +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/include/JavaThrowable.h @@ -43,12 +43,32 @@ namespace jni { const auto classDef = JavaClasses::get(env); - jobject throwable = env->NewObject(classDef->cls, classDef->ctor, env->NewStringUTF(message.c_str())); + jstring text = createMessage(); + + jobject throwable = text != nullptr + ? env->NewObject(classDef->cls, classDef->ctor, text) + : nullptr; + + if (text != nullptr) { + env->DeleteLocalRef(text); + } + + if (throwable == nullptr) { + // Hand on what made it fail, an OutOfMemoryError mostly, so + // that Throw never gets null. + jthrowable pending = env->ExceptionOccurred(); + env->ExceptionClear(); + + return pending; + } return static_cast(throwable); } private: + // The message as a local reference, or null with an exception pending. + jstring createMessage() const; + JNIEnv * env; std::string message; diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaString.cpp b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaString.cpp index 2fb0cc66f..0590312b9 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaString.cpp +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaString.cpp @@ -10,13 +10,155 @@ #include "JavaString.h" #include "JavaUtils.h" +#include +#include + namespace jni { + namespace + { + constexpr jchar kReplacementChar = 0xFFFD; + + bool IsContinuation(unsigned char byte, unsigned char low = 0x80, unsigned char high = 0xBF) + { + return byte >= low && byte <= high; + } + + void AppendCodePoint(std::vector & out, char32_t cp) + { + if (cp < 0x10000) { + out.push_back(static_cast(cp)); + } + else { + cp -= 0x10000; + out.push_back(static_cast(0xD800 + (cp >> 10))); + out.push_back(static_cast(0xDC00 + (cp & 0x3FF))); + } + } + + // Standard UTF-8 to UTF-16. JNI's NewStringUTF takes Modified UTF-8, + // which encodes U+0000 and supplementary characters differently and + // mangles what WebRTC hands out. Every ill-formed sequence becomes one + // U+FFFD per maximal subpart (Unicode 3.9, Table 3-7), as Java's own + // UTF-8 decoder does. + std::vector DecodeUtf8(const std::string & str) + { + const auto * bytes = reinterpret_cast(str.data()); + const size_t length = str.size(); + + std::vector out; + out.reserve(length); + + size_t i = 0; + + while (i < length) { + const unsigned char lead = bytes[i]; + + if (lead < 0x80) { + out.push_back(lead); + i++; + continue; + } + + size_t count; + char32_t cp; + unsigned char low = 0x80; + unsigned char high = 0xBF; + + if (lead >= 0xC2 && lead <= 0xDF) { + count = 1; + cp = lead & 0x1F; + } + else if (lead >= 0xE0 && lead <= 0xEF) { + count = 2; + cp = lead & 0x0F; + low = lead == 0xE0 ? 0xA0 : 0x80; + high = lead == 0xED ? 0x9F : 0xBF; // No surrogates. + } + else if (lead >= 0xF0 && lead <= 0xF4) { + count = 3; + cp = lead & 0x07; + low = lead == 0xF0 ? 0x90 : 0x80; + high = lead == 0xF4 ? 0x8F : 0xBF; // Nothing above U+10FFFF. + } + else { + out.push_back(kReplacementChar); + i++; + continue; + } + + size_t consumed = 1; + + while (consumed <= count && i + consumed < length) { + const unsigned char next = bytes[i + consumed]; + + // Only the first continuation byte has a narrowed range. + if (consumed == 1 ? !IsContinuation(next, low, high) : !IsContinuation(next)) { + break; + } + + cp = (cp << 6) | (next & 0x3F); + consumed++; + } + + if (consumed == count + 1) { + AppendCodePoint(out, cp); + } + else { + out.push_back(kReplacementChar); + } + + i += consumed; + } + + return out; + } + + // Standard UTF-16 to UTF-8. An unpaired surrogate becomes '?', as with + // String.getBytes(UTF_8). + std::string EncodeUtf8(const std::vector & chars) + { + std::string out; + out.reserve(chars.size()); + + for (size_t i = 0; i < chars.size(); i++) { + char32_t cp = chars[i]; + + if (cp >= 0xD800 && cp <= 0xDBFF && i + 1 < chars.size() && chars[i + 1] >= 0xDC00 && chars[i + 1] <= 0xDFFF) { + cp = 0x10000 + ((cp - 0xD800) << 10) + (chars[i + 1] - 0xDC00); + i++; + } + else if (cp >= 0xD800 && cp <= 0xDFFF) { + cp = '?'; + } + + if (cp < 0x80) { + out.push_back(static_cast(cp)); + } + else if (cp < 0x800) { + out.push_back(static_cast(0xC0 | (cp >> 6))); + out.push_back(static_cast(0x80 | (cp & 0x3F))); + } + else if (cp < 0x10000) { + out.push_back(static_cast(0xE0 | (cp >> 12))); + out.push_back(static_cast(0x80 | ((cp >> 6) & 0x3F))); + out.push_back(static_cast(0x80 | (cp & 0x3F))); + } + else { + out.push_back(static_cast(0xF0 | (cp >> 18))); + out.push_back(static_cast(0x80 | ((cp >> 12) & 0x3F))); + out.push_back(static_cast(0x80 | ((cp >> 6) & 0x3F))); + out.push_back(static_cast(0x80 | (cp & 0x3F))); + } + } + + return out; + } + } + JavaString::JavaString(JNIEnv * env) { cls = FindClass(env, "java/lang/String"); - - getBytes = GetMethod(env, cls, "getBytes", "(Ljava/lang/String;)[B"); } std::string JavaString::toNative(JNIEnv * env, const JavaRef & jstr) @@ -25,25 +167,50 @@ namespace jni return ""; } - const auto strClass = JavaClasses::get(env); - - jbyteArray stringBytes = static_cast(env->CallObjectMethod(jstr, strClass->getBytes, env->NewStringUTF("UTF-8"))); - jsize length = env->GetArrayLength(stringBytes); + const jsize length = env->GetStringLength(jstr); - std::string str(length, '\0'); + std::vector chars(static_cast(length)); - env->GetByteArrayRegion(stringBytes, 0, length, reinterpret_cast(&str[0])); + if (length > 0) { + env->GetStringRegion(jstr, 0, length, chars.data()); + } - return str; + return EncodeUtf8(chars); } JavaLocalRef JavaString::toJava(JNIEnv * env, const std::string & str) + { + static const jchar empty = 0; + + std::vector chars; + + try { + chars = DecodeUtf8(str); + } + catch (const std::bad_alloc &) { + // Fail the JNI way, so that callers in noexcept code need not catch. + JavaLocalRef error(env, env->FindClass("java/lang/OutOfMemoryError")); + + if (error.get() != nullptr) { + env->ThrowNew(error.get(), "Converting a native string failed"); + } + + return nullptr; + } + + const jchar * data = chars.empty() ? &empty : chars.data(); + + // A null result means an OutOfMemoryError is pending. + return JavaLocalRef(env, env->NewString(data, static_cast(chars.size()))); + } + + JavaLocalRef JavaString::toJavaOrNull(JNIEnv * env, const std::string & str) { if (str.empty()) { return nullptr; } - return JavaLocalRef(env, env->NewStringUTF(str.c_str())); + return toJava(env, str); } JavaLocalRef JavaString::createArray(JNIEnv * env, const std::vector & vector) @@ -52,4 +219,4 @@ namespace jni return JavaArray::createObjectArray(env, vector, javaClass->cls, &toJava); } -} \ No newline at end of file +} diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaThrowable.cpp b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaThrowable.cpp index e93dc9a2c..4687072d4 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaThrowable.cpp +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaThrowable.cpp @@ -6,6 +6,7 @@ */ #include "JavaThrowable.h" +#include "JavaString.h" #include "JavaUtils.h" #include @@ -27,6 +28,13 @@ namespace jni } } + jstring JavaThrowable::createMessage() const + { + // The message is standard UTF-8 and may come from WebRTC or the + // application, which NewStringUTF and its Modified UTF-8 would mangle. + return JavaString::toJava(env, message).release(); + } + JavaThrowable::operator jthrowable() const { return createThrowable(); diff --git a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaUtils.cpp b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaUtils.cpp index f0d6734f7..52b4f22e1 100644 --- a/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaUtils.cpp +++ b/webrtc-jni/src/main/cpp/dependencies/jni-voithos/src/JavaUtils.cpp @@ -45,8 +45,27 @@ void ThrowJavaException(JNIEnv * env, const char * className, const char * messa jni::JavaLocalRef cls(env, env->FindClass(className)); - if (cls.get() != nullptr) { - env->ThrowNew(cls.get(), message); + if (cls.get() == nullptr) { + return; + } + + // ThrowNew takes Modified UTF-8; messages may carry what WebRTC reports. + jmethodID ctor = env->GetMethodID(cls.get(), "", "(Ljava/lang/String;)V"); + + if (ctor == nullptr) { + return; + } + + jni::JavaLocalRef text = message != nullptr ? jni::JavaString::toJava(env, message) : nullptr; + + if (message != nullptr && text.get() == nullptr) { + return; + } + + jni::JavaLocalRef throwable(env, static_cast(env->NewObject(cls.get(), ctor, text.get()))); + + if (throwable.get() != nullptr) { + env->Throw(throwable.get()); } } @@ -65,10 +84,10 @@ void ThrowCxxJavaException(JNIEnv * env) env->Throw(e.exception().get()); } catch (const std::ios_base::failure & e) { - env->Throw(jni::JavaIOException(env, e.what())); + env->Throw(jni::JavaIOException(env, "%s", e.what())); } catch (const std::exception & e) { - env->Throw(jni::JavaError(env, e.what())); + env->Throw(jni::JavaError(env, "%s", e.what())); } catch (...) { env->Throw(jni::JavaError(env, "Unhandled Exception")); diff --git a/webrtc-jni/src/main/cpp/include/JNI_AudioResampler.h b/webrtc-jni/src/main/cpp/include/JNI_AudioResampler.h index d667248ea..ec70b397e 100644 --- a/webrtc-jni/src/main/cpp/include/JNI_AudioResampler.h +++ b/webrtc-jni/src/main/cpp/include/JNI_AudioResampler.h @@ -20,12 +20,20 @@ extern "C" { * Method: initialize * Signature: ()V */ - JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_initialize + JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_initialize__ (JNIEnv*, jobject); /* * Class: dev_onvoid_webrtc_media_audio_AudioResampler - * Method: resample + * Method: initialize + * Signature: (III)V + */ + JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_initialize__III + (JNIEnv*, jobject, jint, jint, jint); + + /* + * Class: dev_onvoid_webrtc_media_audio_AudioResampler + * Method: resampleInternal * Signature: ([BI[BII)I */ JNIEXPORT jint JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_resampleInternal diff --git a/webrtc-jni/src/main/cpp/include/JNI_VideoDesktopSource.h b/webrtc-jni/src/main/cpp/include/JNI_VideoDesktopSource.h index ed26a79aa..9195b4b21 100644 --- a/webrtc-jni/src/main/cpp/include/JNI_VideoDesktopSource.h +++ b/webrtc-jni/src/main/cpp/include/JNI_VideoDesktopSource.h @@ -31,14 +31,6 @@ extern "C" { JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoDesktopSource_setMaxFrameSize (JNIEnv*, jobject, jint, jint); - /* - * Class: dev_onvoid_webrtc_media_video_VideoDesktopSource - * Method: setFocusSelectedSource - * Signature: (Z)V - */ - JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoDesktopSource_setFocusSelectedSource - (JNIEnv*, jobject, jboolean); - /* * Class: dev_onvoid_webrtc_media_video_VideoDesktopSource * Method: start diff --git a/webrtc-jni/src/main/cpp/src/JNI_AudioResampler.cpp b/webrtc-jni/src/main/cpp/src/JNI_AudioResampler.cpp index 45c777819..3949e7d5e 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_AudioResampler.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_AudioResampler.cpp @@ -35,7 +35,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_dispose SetHandle(env, caller, nullptr); } -JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_initialize +JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_initialize__ (JNIEnv * env, jobject caller) { try { @@ -48,7 +48,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_initial } } -JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_initialize +JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_audio_AudioResampler_initialize__III (JNIEnv* env, jobject caller, jint srcSamplesPerChannel, jint dstSamplesPerChannel, jint channels) { try { diff --git a/webrtc-jni/src/main/cpp/src/JNI_RTCPeerConnection.cpp b/webrtc-jni/src/main/cpp/src/JNI_RTCPeerConnection.cpp index bfa8a5dd3..cc2ca4277 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_RTCPeerConnection.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_RTCPeerConnection.cpp @@ -136,7 +136,7 @@ JNIEXPORT jobject JNICALL Java_dev_onvoid_webrtc_RTCPeerConnection_addTrackInter return jni::JavaFactories::create(env, sender.release()).release(); } - env->Throw(jni::JavaRuntimeException(env, jni::RTCErrorToString(result.error()).c_str())); + env->Throw(jni::JavaRuntimeException(env, "%s", jni::RTCErrorToString(result.error()).c_str())); return nullptr; } @@ -200,7 +200,7 @@ JNIEXPORT jobject JNICALL Java_dev_onvoid_webrtc_RTCPeerConnection_addTransceive return jni::JavaFactories::create(env, transceiver.release()).release(); } - env->Throw(jni::JavaRuntimeException(env, jni::RTCErrorToString(result.error()).c_str())); + env->Throw(jni::JavaRuntimeException(env, "%s", jni::RTCErrorToString(result.error()).c_str())); return nullptr; } @@ -241,7 +241,7 @@ JNIEXPORT jobject JNICALL Java_dev_onvoid_webrtc_RTCPeerConnection_addTransceive return jni::JavaFactories::create(env, transceiver.release()).release(); } - env->Throw(jni::JavaRuntimeException(env, jni::RTCErrorToString(result.error()).c_str())); + env->Throw(jni::JavaRuntimeException(env, "%s", jni::RTCErrorToString(result.error()).c_str())); } catch (...) { ThrowCxxJavaException(env); @@ -622,7 +622,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_RTCPeerConnection_setBitrate webrtc::RTCError error = pc->SetBitrate(settings); if (!error.ok()) { - env->Throw(jni::JavaRuntimeException(env, jni::RTCErrorToString(error).c_str())); + env->Throw(jni::JavaRuntimeException(env, "%s", jni::RTCErrorToString(error).c_str())); } } catch (...) { @@ -740,7 +740,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_RTCPeerConnection_setConfiguration webrtc::RTCError error = pc->SetConfiguration(config); if (!error.ok()) { - env->Throw(jni::JavaRuntimeException(env, jni::RTCErrorToString(error).c_str())); + env->Throw(jni::JavaRuntimeException(env, "%s", jni::RTCErrorToString(error).c_str())); } } catch (...) { diff --git a/webrtc-jni/src/main/cpp/src/JNI_RTCRtpSender.cpp b/webrtc-jni/src/main/cpp/src/JNI_RTCRtpSender.cpp index f5a3e8d79..f38910bd4 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_RTCRtpSender.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_RTCRtpSender.cpp @@ -94,7 +94,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_RTCRtpSender_setParameters webrtc::RTCError result = sender->SetParameters(rtp_parameters); if (!result.ok()) { - env->Throw(jni::JavaRuntimeException(env, jni::RTCErrorToString(result).c_str())); + env->Throw(jni::JavaRuntimeException(env, "%s", jni::RTCErrorToString(result).c_str())); } } catch (...) { @@ -194,7 +194,7 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_RTCRtpSender_generateKeyFrame webrtc::RTCError result = sender->GenerateKeyFrame({}); if (!result.ok()) { - env->Throw(jni::JavaRuntimeException(env, jni::RTCErrorToString(result).c_str())); + env->Throw(jni::JavaRuntimeException(env, "%s", jni::RTCErrorToString(result).c_str())); } } diff --git a/webrtc-jni/src/main/cpp/src/JNI_RTCRtpTransceiver.cpp b/webrtc-jni/src/main/cpp/src/JNI_RTCRtpTransceiver.cpp index e304583fb..18ef0b0e6 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_RTCRtpTransceiver.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_RTCRtpTransceiver.cpp @@ -35,7 +35,10 @@ JNIEXPORT jstring JNICALL Java_dev_onvoid_webrtc_RTCRtpTransceiver_getMid webrtc::RtpTransceiverInterface * transceiver = GetHandle(env, caller); CHECK_HANDLEV(transceiver, nullptr); - return jni::JavaString::toJava(env, transceiver->mid().value_or("")).release(); + // Null until negotiated, and again after a rollback. + const auto mid = transceiver->mid(); + + return mid ? jni::JavaString::toJava(env, *mid).release() : nullptr; } catch (...) { ThrowCxxJavaException(env); diff --git a/webrtc-jni/src/main/cpp/src/JNI_VideoDesktopSource.cpp b/webrtc-jni/src/main/cpp/src/JNI_VideoDesktopSource.cpp index 1cb2e1fe4..3d3f23f8d 100644 --- a/webrtc-jni/src/main/cpp/src/JNI_VideoDesktopSource.cpp +++ b/webrtc-jni/src/main/cpp/src/JNI_VideoDesktopSource.cpp @@ -51,15 +51,6 @@ JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoDesktopSource_set videoSource->setMaxFrameSize(webrtc::DesktopSize(width, height)); } -JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoDesktopSource_setFocusSelectedSource -(JNIEnv * env, jobject caller, jboolean focus) -{ - jni::VideoTrackDesktopSource * videoSource = GetHandle(env, caller); - CHECK_HANDLE(videoSource); - - videoSource->setFocusSelectedSource(focus); -} - JNIEXPORT void JNICALL Java_dev_onvoid_webrtc_media_video_VideoDesktopSource_start (JNIEnv * env, jobject caller) { diff --git a/webrtc-jni/src/main/cpp/src/api/AddIceCandidateObserver.cpp b/webrtc-jni/src/main/cpp/src/api/AddIceCandidateObserver.cpp index 79a24fcbc..008615687 100644 --- a/webrtc-jni/src/main/cpp/src/api/AddIceCandidateObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/AddIceCandidateObserver.cpp @@ -71,7 +71,7 @@ namespace jni env->CallVoidMethod(callback.get(), javaClass->onSuccess); } else { - JavaLocalRef message(env, env->NewStringUTF(error)); + JavaLocalRef message = JavaString::toJava(env, error); if (!env->ExceptionCheck()) { env->CallVoidMethod(callback.get(), javaClass->onFailure, message.get()); diff --git a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelSendObserver.cpp b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelSendObserver.cpp index a517fa199..f2a07a8cc 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCDataChannelSendObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCDataChannelSendObserver.cpp @@ -61,7 +61,7 @@ namespace jni env->CallVoidMethod(callback.get(), javaClass->onSuccess); } else { - JavaLocalRef jMessage(env, env->NewStringUTF(message->c_str())); + JavaLocalRef jMessage = jni::JavaString::toJava(env, *message); if (!env->ExceptionCheck()) { env->CallVoidMethod(callback.get(), javaClass->onFailure, jMessage.get()); } diff --git a/webrtc-jni/src/main/cpp/src/api/RTCEncodedFrameTransformer.cpp b/webrtc-jni/src/main/cpp/src/api/RTCEncodedFrameTransformer.cpp index 1d3509d9c..148b3842f 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCEncodedFrameTransformer.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCEncodedFrameTransformer.cpp @@ -17,6 +17,7 @@ #include "api/RTCEncodedFrameTransformer.h" #include "api/WebRTCUtils.h" #include "JavaClasses.h" +#include "JavaString.h" #include "JavaUtils.h" #include "JNI_WebRTC.h" @@ -116,7 +117,7 @@ namespace jni return found->second->get(); } - jstring text = env->NewStringUTF(mimeType.c_str()); + jstring text = JavaString::toJava(env, mimeType).release(); if (text == nullptr) { return nullptr; @@ -142,7 +143,7 @@ namespace jni jstring jRid = nullptr; if (rid.has_value() && !rid->empty()) { - jRid = env->NewStringUTF(rid->c_str()); + jRid = JavaString::toJava(env, *rid).release(); } return env->NewObject(javaVideoFrameClass->cls, javaVideoFrameClass->ctor, diff --git a/webrtc-jni/src/main/cpp/src/api/RTCIceCandidate.cpp b/webrtc-jni/src/main/cpp/src/api/RTCIceCandidate.cpp index faa691ace..e3ab1e681 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCIceCandidate.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCIceCandidate.cpp @@ -36,10 +36,11 @@ namespace jni candidate->ToString(&sdpStr); jobject jCandidate = env->NewObject(javaClass->cls, javaClass->ctor, - JavaString::toJava(env, candidate->sdp_mid()).get(), + // WebRTC leaves both empty when a candidate has none. + JavaString::toJavaOrNull(env, candidate->sdp_mid()).get(), candidate->sdp_mline_index(), JavaString::toJava(env, sdpStr).get(), - JavaString::toJava(env, candidate->server_url()).get()); + JavaString::toJavaOrNull(env, candidate->server_url()).get()); return JavaLocalRef(env, jCandidate); } diff --git a/webrtc-jni/src/main/cpp/src/api/RTCIceServer.cpp b/webrtc-jni/src/main/cpp/src/api/RTCIceServer.cpp index 08866fb10..daf13498a 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCIceServer.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCIceServer.cpp @@ -51,9 +51,10 @@ namespace jni ecList.add(JavaString::toJava(env, ec)); } - auto username = JavaString::toJava(env, server.username); - auto password = JavaString::toJava(env, server.password); - auto hostname = JavaString::toJava(env, server.hostname); + // WebRTC keeps unset credentials as empty strings. + auto username = JavaString::toJavaOrNull(env, server.username); + auto password = JavaString::toJavaOrNull(env, server.password); + auto hostname = JavaString::toJavaOrNull(env, server.hostname); auto certPolicy = JavaEnums::toJava(env, server.tls_cert_policy); jobject iceServer = env->NewObject(javaClass->cls, javaClass->ctor); diff --git a/webrtc-jni/src/main/cpp/src/api/RTCPeerConnectionIceErrorEvent.cpp b/webrtc-jni/src/main/cpp/src/api/RTCPeerConnectionIceErrorEvent.cpp index 3f11bd362..1fab0cd32 100644 --- a/webrtc-jni/src/main/cpp/src/api/RTCPeerConnectionIceErrorEvent.cpp +++ b/webrtc-jni/src/main/cpp/src/api/RTCPeerConnectionIceErrorEvent.cpp @@ -30,9 +30,13 @@ namespace jni { const auto javaClass = JavaClasses::get(env); + // WebRTC clears the address it must not expose, and then both + // address and port are null. + const bool hasAddress = !address.empty(); + jobject jEvent = env->NewObject(javaClass->cls, javaClass->ctor, - JavaString::toJava(env, address).get(), - Integer::create(env, port).get(), + JavaString::toJavaOrNull(env, address).get(), + hasAddress ? Integer::create(env, port).get() : nullptr, JavaString::toJava(env, url).get(), error_code, JavaString::toJava(env, error_text).get()); diff --git a/webrtc-jni/src/main/cpp/src/api/SetSessionDescriptionObserver.cpp b/webrtc-jni/src/main/cpp/src/api/SetSessionDescriptionObserver.cpp index 1e3032ac5..483ac3866 100644 --- a/webrtc-jni/src/main/cpp/src/api/SetSessionDescriptionObserver.cpp +++ b/webrtc-jni/src/main/cpp/src/api/SetSessionDescriptionObserver.cpp @@ -70,7 +70,7 @@ namespace jni env->CallVoidMethod(callback.get(), javaClass->onSuccess); } else { - JavaLocalRef message(env, env->NewStringUTF(error)); + JavaLocalRef message = JavaString::toJava(env, error); if (!env->ExceptionCheck()) { env->CallVoidMethod(callback.get(), javaClass->onFailure, message.get()); diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/NativeStringTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/NativeStringTests.java new file mode 100644 index 000000000..0cea516b2 --- /dev/null +++ b/webrtc/src/test/java/dev/onvoid/webrtc/NativeStringTests.java @@ -0,0 +1,241 @@ +/* + * Copyright 2026 Alex Andres + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package dev.onvoid.webrtc; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import dev.onvoid.webrtc.media.MediaType; +import dev.onvoid.webrtc.media.audio.AudioOptions; +import dev.onvoid.webrtc.media.audio.AudioTrack; +import dev.onvoid.webrtc.media.audio.AudioTrackSource; + +import java.util.List; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +/** + * Strings that go to native code and come back. They used to come back + * through NewStringUTF, which takes Modified UTF-8 rather than the UTF-8 they + * were sent as, and empty ones came back as null. + */ +class NativeStringTests extends TestBase { + + /** Each must come back exactly as it was sent. */ + private static final List VALUES = List.of( + "", + "test-πŸ˜€", // "test-πŸ˜€", 7 chars. + "πŸ˜€", + "a\u0000b", + "\u0000", + "end\u0000", + "Àâü € δΈ­ζ–‡ π„ž πŸ‘πŸ½", + "\u007F\u0080߿ࠀ￿𐀀τΏΏ"); + + private RTCPeerConnection connection; + + + @BeforeEach + void init() { + connection = factory.createPeerConnection(new RTCConfiguration(), candidate -> { }); + } + + @AfterEach + void dispose() { + connection.close(); + } + + @Test + void dataChannelLabelAndProtocol() { + for (String value : VALUES) { + RTCDataChannelInit init = new RTCDataChannelInit(); + init.protocol = value; + + RTCDataChannel channel = connection.createDataChannel(value, init); + + try { + assertSame(value, channel.getLabel(), "label"); + assertSame(value, channel.getProtocol(), "protocol"); + } + finally { + channel.dispose(); + } + } + } + + @Test + void trackAndStreamIds() { + AudioTrackSource source = factory.createAudioSource(new AudioOptions()); + + try { + for (String value : VALUES) { + AudioTrack track = factory.createAudioTrack(value, source); + + try { + assertSame(value, track.getId(), "track id"); + + if (value.isEmpty()) { + // An empty stream id names no stream. + continue; + } + + RTCRtpSender sender = connection.addTrack(track, List.of(value)); + + assertEquals(List.of(value), sender.getStreams()); + + connection.removeTrack(sender); + } + finally { + track.dispose(); + } + } + } + finally { + source.dispose(); + } + } + + @Test + void unpairedSurrogateBecomesQuestionMark() { + // UTF-8 has no form for it, so it goes the way of String.getBytes(UTF_8). + RTCDataChannel channel = connection.createDataChannel("x\uD800y\uDC00z", new RTCDataChannelInit()); + + try { + assertEquals("x?y?z", channel.getLabel()); + } + finally { + channel.dispose(); + } + } + + @Test + void iceServerCredentials() { + RTCIceServer server = new RTCIceServer(); + server.urls.add("turn:127.0.0.1:3478"); + server.username = "nutzer-πŸ˜€"; + server.password = "a\u0000b"; + + RTCIceServer unset = new RTCIceServer(); + unset.urls.add("stun:127.0.0.1:3478"); + + RTCConfiguration config = new RTCConfiguration(); + config.iceServers.add(server); + config.iceServers.add(unset); + + RTCPeerConnection peer = factory.createPeerConnection(config, candidate -> { }); + + try { + List servers = peer.getConfiguration().iceServers; + + assertSame(server.username, servers.get(0).username, "username"); + assertSame(server.password, servers.get(0).password, "password"); + + // WebRTC keeps an unset credential as an empty string; it stays unset. + assertNull(servers.get(1).username); + assertNull(servers.get(1).password); + assertNull(servers.get(1).hostname); + } + finally { + peer.close(); + } + } + + @Test + void errorMessageCarriesTrackId() { + // WebRTC puts the id into its error text, which used to go to vsnprintf + // as the format, and on to NewStringUTF. + String id = "track %s%n%d Γ€πŸ˜€"; + + AudioTrackSource source = factory.createAudioSource(new AudioOptions()); + AudioTrack track = factory.createAudioTrack(id, source); + + try { + RTCRtpSender sender = connection.addTrack(track, List.of("stream")); + + try { + RuntimeException e = assertThrows(RuntimeException.class, + () -> connection.addTrack(track, List.of("stream"))); + + assertTrue(e.getMessage().contains(id), () -> "message: " + codeUnits(e.getMessage())); + } + finally { + connection.removeTrack(sender); + } + } + finally { + track.dispose(); + source.dispose(); + } + } + + @Test + void errorMessageCarriesSdpLine() { + // The parse error quotes the line, and the native exception reaches + // Java through the same path as above. + String line = "%s%n ΓΌπŸ˜€"; + RTCSessionDescription description = new RTCSessionDescription(RTCSdpType.OFFER, + "v=0\r\n" + line + "\r\n"); + + Throwable e = assertThrows(Throwable.class, + () -> connection.setRemoteDescription(description, new SetSessionDescriptionObserver() { + + @Override + public void onSuccess() { + } + + @Override + public void onFailure(String error) { + } + })); + + assertTrue(e.getMessage().contains(line), () -> "message: " + codeUnits(e.getMessage())); + } + + @Test + void midIsNullUntilNegotiated() { + RTCRtpTransceiver transceiver = connection.addTransceiver(MediaType.AUDIO, new RTCRtpTransceiverInit()); + + assertNull(transceiver.getMid()); + } + + /** + * Equal content, with the code units in the message: two strings that + * differ only in a NUL or a surrogate look alike when printed. + */ + private static void assertSame(String expected, String actual, String what) { + assertEquals(expected, actual, () -> what + ": expected " + codeUnits(expected) + + " but was " + codeUnits(actual)); + } + + private static String codeUnits(String value) { + if (value == null) { + return "null"; + } + + StringBuilder builder = new StringBuilder("["); + + for (int i = 0; i < value.length(); i++) { + builder.append(i == 0 ? "" : " ").append(String.format("%04X", (int) value.charAt(i))); + } + + return builder.append("] (").append(value.length()).append(" chars)").toString(); + } +} diff --git a/webrtc/src/test/java/dev/onvoid/webrtc/RTCDtmfSenderTests.java b/webrtc/src/test/java/dev/onvoid/webrtc/RTCDtmfSenderTests.java index 8b4add500..054c4cc8a 100644 --- a/webrtc/src/test/java/dev/onvoid/webrtc/RTCDtmfSenderTests.java +++ b/webrtc/src/test/java/dev/onvoid/webrtc/RTCDtmfSenderTests.java @@ -52,7 +52,7 @@ public void onToneChange(String tone, String toneBuffer) { tones.add(tone); toneBuffers.add(toneBuffer); - if (tone == null || tone.isEmpty()) { + if (tone.isEmpty()) { completedLatch.countDown(); } } @@ -149,7 +149,7 @@ void dtmfSenderOutlivesClosedConnection() { for (int i = 0; i < 1000; i++) { assertFalse(dtmfSender.canInsertDtmf()); assertFalse(dtmfSender.insertDtmf("1", 100, 70)); - assertNull(dtmfSender.tones()); + assertEquals("", dtmfSender.tones()); } assertDoesNotThrow(dtmfSender::unregisterObserver); @@ -188,7 +188,7 @@ private void assertObserverOfOtherInstanceStays(boolean dispose) throws Interrup assertTrue(dtmfSender.insertDtmf("1", 100, 70)); assertTrue(secondObserver.awaitCompletion(), "The observer of the other instance was removed"); - assertEquals(Arrays.asList("1", null), secondObserver.getTones()); + assertEquals(Arrays.asList("1", ""), secondObserver.getTones()); assertTrue(firstObserver.getTones().isEmpty(), "The replaced observer must not receive events"); } finally { @@ -250,14 +250,14 @@ void registerAndUnregisterObserver() throws InterruptedException { observer.waitUntilCompleted(); - assertEquals(Arrays.asList("1", "2", "3", null), observer.getTones()); + assertEquals(Arrays.asList("1", "2", "3", ""), observer.getTones()); dtmfSender.unregisterObserver(); dtmfSender.insertDtmf("456", 100, 70); Thread.sleep(500); - assertEquals(Arrays.asList("1", "2", "3", null), observer.getTones()); // No new events + assertEquals(Arrays.asList("1", "2", "3", ""), observer.getTones()); // No new events } @Test @@ -275,7 +275,7 @@ void replaceObserverStopsPreviousObserver() throws InterruptedException { assertTrue(dtmfSender.insertDtmf("1", 100, 70)); assertTrue(second.awaitCompletion(), "Timed out waiting for DTMF sequence to complete"); - assertEquals(Arrays.asList("1", null), second.getTones()); + assertEquals(Arrays.asList("1", ""), second.getTones()); assertTrue(first.getTones().isEmpty(), "The replaced observer must not receive events"); dtmfSender.unregisterObserver(); @@ -287,17 +287,17 @@ private void insertTones(List tones, int duration, int interToneGap) thr dtmfSender.registerObserver(observer); assertTrue(dtmfSender.insertDtmf(String.join("", tones), duration, interToneGap)); - // Wait until the DTMF sequence completes (observer receives null/empty tone). + // Wait until the DTMF sequence completes (observer receives an empty tone). assertTrue(observer.awaitCompletion(), "Timed out waiting for DTMF sequence to complete"); - // All tones should have been delivered to the observer in order, followed by null. + // All tones should have been delivered to the observer in order, followed by an empty one. List expectedTones = new ArrayList<>(tones); - expectedTones.add(null); + expectedTones.add(""); assertEquals(expectedTones, observer.getTones()); // After completion, the internal tone buffer should be empty. - assertNull(dtmfSender.tones()); + assertEquals("", dtmfSender.tones()); dtmfSender.unregisterObserver(); }