fix: address critical audit findings

#3 JSON escaping: CallIceCandidateEvent.serializeCandidate() now
escapes backslashes and quotes in SDP/sdpMid strings before
interpolation. Prevents malformed JSON when SDP contains special
characters.

#4 Thread safety: remoteDescriptionSet changed from Boolean to
AtomicBoolean to prevent race conditions between WebRTC callback
threads and main thread accessing the flag simultaneously.

#5 Async cleanup: hangup() no longer calls cleanup() synchronously
after launching the coroutine. Cleanup is now triggered by the
CallManager state collector when Ended state is reached, avoiding
resource disposal during active WebRTC callbacks.

#7 EglBase leak: createWebRtcSession() now wraps initialize() and
createPeerConnection() in try-catch that disposes the session on
failure, preventing EGL context leaks from failed initialization.

#10 SDP failure: createOffer/createAnswer onCreateFailure() now
calls onError() to surface SDP creation failures to the user
instead of silently logging them.

#11 Notification age: EventNotificationConsumer now uses
CallOfferEvent.EXPIRATION_SECONDS (20s) instead of hardcoded 30s
to align with the actual event expiration.

https://claude.ai/code/session_017hZm7yu7CzmcQgZGSaqSXS
This commit is contained in:
Claude
2026-04-02 03:25:11 +00:00
parent 0e31afbb25
commit 3f82bb5ff6
4 changed files with 30 additions and 14 deletions
@@ -55,7 +55,9 @@ class CallController(
) { ) {
private var webRtcSession: WebRtcCallSession? = null private var webRtcSession: WebRtcCallSession? = null
private val callFactory = WebRtcCallFactory() private val callFactory = WebRtcCallFactory()
private var remoteDescriptionSet = false private val remoteDescriptionSet =
java.util.concurrent.atomic
.AtomicBoolean(false)
private val pendingIceCandidates = CopyOnWriteArrayList<IceCandidate>() private val pendingIceCandidates = CopyOnWriteArrayList<IceCandidate>()
val audioManager = CallAudioManager(context) val audioManager = CallAudioManager(context)
@@ -117,7 +119,7 @@ class CallController(
scope.launch { scope.launch {
val callId = UUID.randomUUID().toString() val callId = UUID.randomUUID().toString()
_errorMessage.value = null _errorMessage.value = null
remoteDescriptionSet = false remoteDescriptionSet.set(false)
pendingIceCandidates.clear() pendingIceCandidates.clear()
try { try {
@@ -154,7 +156,7 @@ class CallController(
scope.launch { scope.launch {
_errorMessage.value = null _errorMessage.value = null
remoteDescriptionSet = false remoteDescriptionSet.set(false)
// Don't clear pendingIceCandidates here — they were buffered while ringing // Don't clear pendingIceCandidates here — they were buffered while ringing
try { try {
@@ -197,11 +199,11 @@ class CallController(
fun onIceCandidateReceived(event: CallIceCandidateEvent) { fun onIceCandidateReceived(event: CallIceCandidateEvent) {
try { try {
val candidate = IceCandidate(event.sdpMid(), event.sdpMLineIndex(), event.candidateSdp()) val candidate = IceCandidate(event.sdpMid(), event.sdpMLineIndex(), event.candidateSdp())
if (webRtcSession != null && remoteDescriptionSet) { if (webRtcSession != null && remoteDescriptionSet.get()) {
Log.d(TAG) { "Adding ICE candidate directly: ${candidate.sdp.take(50)}" } Log.d(TAG) { "Adding ICE candidate directly: ${candidate.sdp.take(50)}" }
webRtcSession?.addIceCandidate(candidate) webRtcSession?.addIceCandidate(candidate)
} else { } else {
Log.d(TAG) { "Buffering ICE candidate (session=${webRtcSession != null}, remoteSet=$remoteDescriptionSet)" } Log.d(TAG) { "Buffering ICE candidate (session=${webRtcSession != null}, remoteSet=${remoteDescriptionSet.get()})" }
pendingIceCandidates.add(candidate) pendingIceCandidates.add(candidate)
} }
} catch (e: Exception) { } catch (e: Exception) {
@@ -210,7 +212,7 @@ class CallController(
} }
private fun flushPendingIceCandidates() { private fun flushPendingIceCandidates() {
remoteDescriptionSet = true remoteDescriptionSet.set(true)
val session = webRtcSession ?: return val session = webRtcSession ?: return
val candidates = pendingIceCandidates.toList() val candidates = pendingIceCandidates.toList()
Log.d(TAG) { "Flushing ${candidates.size} buffered ICE candidates" } Log.d(TAG) { "Flushing ${candidates.size} buffered ICE candidates" }
@@ -238,8 +240,10 @@ class CallController(
fun getEglBase() = webRtcSession?.eglBase fun getEglBase() = webRtcSession?.eglBase
fun hangup() { fun hangup() {
scope.launch { callManager.hangup() } scope.launch {
cleanup() callManager.hangup()
// cleanup is triggered by the state collector when Ended is reached
}
} }
fun clearError() { fun clearError() {
@@ -256,12 +260,12 @@ class CallController(
_isVideoEnabled.value = true _isVideoEnabled.value = true
webRtcSession?.dispose() webRtcSession?.dispose()
webRtcSession = null webRtcSession = null
remoteDescriptionSet = false remoteDescriptionSet.set(false)
pendingIceCandidates.clear() pendingIceCandidates.clear()
} }
private fun createWebRtcSession() { private fun createWebRtcSession() {
webRtcSession = val session =
WebRtcCallSession( WebRtcCallSession(
context = context, context = context,
iceServers = IceServerConfig.buildIceServers(), iceServers = IceServerConfig.buildIceServers(),
@@ -276,8 +280,14 @@ class CallController(
onDisconnected = { scope.launch { callManager.hangup() } }, onDisconnected = { scope.launch { callManager.hangup() } },
onError = { error -> _errorMessage.value = error }, onError = { error -> _errorMessage.value = error },
) )
webRtcSession?.initialize() try {
webRtcSession?.createPeerConnection() session.initialize()
session.createPeerConnection()
webRtcSession = session
} catch (e: Exception) {
session.dispose()
throw e
}
} }
private fun onLocalIceCandidate(candidate: IceCandidate) { private fun onLocalIceCandidate(candidate: IceCandidate) {
@@ -203,6 +203,7 @@ class WebRtcCallSession(
override fun onCreateFailure(error: String?) { override fun onCreateFailure(error: String?) {
Log.e(TAG, "Create offer failed: $error") Log.e(TAG, "Create offer failed: $error")
error?.let { onError("Create offer failed: $it") }
} }
override fun onSetSuccess() {} override fun onSetSuccess() {}
@@ -231,6 +232,7 @@ class WebRtcCallSession(
override fun onCreateFailure(error: String?) { override fun onCreateFailure(error: String?) {
Log.e(TAG, "Create answer failed: $error") Log.e(TAG, "Create answer failed: $error")
error?.let { onError("Create answer failed: $it") }
} }
override fun onSetSuccess() {} override fun onSetSuccess() {}
@@ -631,7 +631,7 @@ class EventNotificationConsumer(
) { ) {
if (!account.isFollowing(event.pubKey)) return if (!account.isFollowing(event.pubKey)) return
if (TimeUtils.now() - event.createdAt > 30) return if (TimeUtils.now() - event.createdAt > CallOfferEvent.EXPIRATION_SECONDS) return
val callerUser = LocalCache.getUserIfExists(event.pubKey) val callerUser = LocalCache.getUserIfExists(event.pubKey)
val callerName = callerUser?.toBestDisplayName() ?: event.pubKey.take(8) + "..." val callerName = callerUser?.toBestDisplayName() ?: event.pubKey.take(8) + "..."
@@ -69,7 +69,11 @@ class CallIceCandidateEvent(
sdp: String, sdp: String,
sdpMid: String, sdpMid: String,
sdpMLineIndex: Int, sdpMLineIndex: Int,
): String = """{"candidate":"$sdp","sdpMid":"$sdpMid","sdpMLineIndex":$sdpMLineIndex}""" ): String {
val escapedSdp = sdp.replace("\\", "\\\\").replace("\"", "\\\"")
val escapedMid = sdpMid.replace("\\", "\\\\").replace("\"", "\\\"")
return """{"candidate":"$escapedSdp","sdpMid":"$escapedMid","sdpMLineIndex":$sdpMLineIndex}"""
}
fun build( fun build(
candidateJson: String, candidateJson: String,