Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Handling of IQ error responses. #1204

Merged
merged 1 commit into from
Dec 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import org.jitsi.xmpp.extensions.jingle.ExtmapAllowMixedPacketExtension
import org.jitsi.xmpp.extensions.jingle.IceUdpTransportPacketExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jivesoftware.smack.StanzaCollector
import org.jivesoftware.smack.packet.ErrorIQ
import org.jivesoftware.smack.packet.IQ
import org.jivesoftware.smackx.muc.MUCRole
import java.util.Collections.singletonList
Expand Down Expand Up @@ -294,36 +293,43 @@ class Colibri2Session(
*/
private fun sendRequest(iq: IQ, name: String) {
logger.debug { "Sending $name request: ${iq.toStringOpt()}" }
xmppConnection.sendIqAndHandleResponseAsync(iq) {
when (it) {
is ConferenceModifiedIQ -> logger.debug { "Received $name response: ${it.toStringOpt()}" }
null -> logger.info("$name request timed out. Ignoring.")
else -> {
if (it is ErrorIQ) {
val reason = it.error?.getExtension<Colibri2Error>(
Colibri2Error.ELEMENT,
Colibri2Error.NAMESPACE
)?.reason
val endpointId = it.error?.getExtension<Colibri2Endpoint>(
Colibri2Endpoint.ELEMENT,
Colibri2Endpoint.NAMESPACE
)?.id
// If colibri2 error extension is present then the message came from
// a jitsi-videobridge instance. Otherwise, it might come from another component
// (e.g. the XMPP server or MUC component).
val reInvite = reason == Colibri2Error.Reason.UNKNOWN_ENDPOINT && endpointId != null
if (reInvite) {
logger.warn(
"Endpoint [$endpointId] is not found, session failed: ${it.toStringOpt()}, " +
"request was: ${iq.toStringOpt()}"
)
colibriSessionManager.endpointFailed(endpointId!!)
return@sendIqAndHandleResponseAsync
}
}
logger.error("Received error response for $name, session failed: ${it.toStringOpt()}")
xmppConnection.sendIqAndHandleResponseAsync(iq) { response ->
if (response == null) {
logger.info("$name request timed out. Ignoring.")
return@sendIqAndHandleResponseAsync
}

response.error?.let { error ->
val reason = error.getExtension<Colibri2Error>(
Colibri2Error.ELEMENT,
Colibri2Error.NAMESPACE
)?.reason
val endpointId = error.getExtension<Colibri2Endpoint>(
Colibri2Endpoint.ELEMENT,
Colibri2Endpoint.NAMESPACE
)?.id
// If colibri2 error extension is present then the message came from
// a jitsi-videobridge instance. Otherwise, it might come from another component
// (e.g. the XMPP server or MUC component).
val reInvite = reason == Colibri2Error.Reason.UNKNOWN_ENDPOINT && endpointId != null
if (reInvite) {
logger.warn(
"Endpoint [$endpointId] is not found, session failed: ${error.toStringOpt()}, " +
"request was: ${iq.toStringOpt()}"
)
colibriSessionManager.endpointFailed(endpointId!!)
} else {
logger.error("Received error response for $name, session failed: ${error.toStringOpt()}")
colibriSessionManager.sessionFailed(this@Colibri2Session)
}
return@sendIqAndHandleResponseAsync
}

if (response !is ConferenceModifiedIQ) {
logger.error("Received response with unexpected type ${response.javaClass.name}")
colibriSessionManager.sessionFailed(this@Colibri2Session)
} else {
logger.debug { "Received $name response: ${response.toStringOpt()}" }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import org.jitsi.xmpp.extensions.jingle.IceUdpTransportPacketExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jivesoftware.smack.AbstractXMPPConnection
import org.jivesoftware.smack.StanzaCollector
import org.jivesoftware.smack.packet.ErrorIQ
import org.jivesoftware.smack.packet.IQ
import org.jivesoftware.smack.packet.StanzaError.Condition.bad_request
import org.jivesoftware.smack.packet.StanzaError.Condition.conflict
Expand Down Expand Up @@ -434,21 +433,22 @@ class ColibriV2SessionManager(
if (response == null) {
session.bridge.isOperational = false
throw ColibriAllocationFailedException("Timeout", true)
} else if (response is ErrorIQ) {
}
response.error?.let { error ->
// The reason in a colibri2 error extension, if one is present. If a reason is present we know the response
// comes from a jitsi-videobridge instance. Otherwise, it might come from another component (e.g. the
// XMPP server or MUC component).
val reason = response.error?.getExtension<Colibri2Error>(
val reason = error.getExtension<Colibri2Error>(
Colibri2Error.ELEMENT,
Colibri2Error.NAMESPACE
)?.reason
logger.info("Received error response: ${response.toStringOpt()}")
when (response.error?.condition) {
when (error.condition) {
bad_request -> {
// Most probably we sent a bad request.
// If we flag the bridge as non-operational we may disrupt other conferences.
// If we trigger a re-invite we may cause the same error repeating.
throw ColibriAllocationFailedException("Bad request: ${response.error?.toStringOpt()}", false)
throw ColibriAllocationFailedException("Bad request: ${error.toStringOpt()}", false)
}
item_not_found -> {
if (reason == Colibri2Error.Reason.CONFERENCE_NOT_FOUND) {
Expand All @@ -467,7 +467,7 @@ class ColibriV2SessionManager(
if (reason == null) {
// An error NOT coming from the bridge.
throw ColibriAllocationFailedException(
"XMPP error: ${response.error?.toStringOpt()}",
"XMPP error: ${error.toStringOpt()}",
true
)
} else if (reason == Colibri2Error.Reason.CONFERENCE_ALREADY_EXISTS) {
Expand All @@ -480,7 +480,7 @@ class ColibriV2SessionManager(
// we can't expire a conference without listing its individual endpoints and we think there
// were none.
// We remove the bridge from the conference (expiring it) and re-invite the participants.
throw ColibriAllocationFailedException("Colibri error: ${response.error?.toStringOpt()}", true)
throw ColibriAllocationFailedException("Colibri error: ${error.toStringOpt()}", true)
}
}
service_unavailable -> {
Expand All @@ -496,14 +496,17 @@ class ColibriV2SessionManager(
}
else -> {
session.bridge.isOperational = false
throw ColibriAllocationFailedException("Error: ${response.error?.toStringOpt()}", true)
throw ColibriAllocationFailedException("Error: ${error.toStringOpt()}", true)
}
}
}

if (response !is ConferenceModifiedIQ) {
session.bridge.isOperational = false
throw ColibriAllocationFailedException("Response of wrong type: ${response::class.java.name}", false)
throw ColibriAllocationFailedException(
"Response of wrong type: ${response::class.java.name}: ${response.toXML()}",
false
)
}

if (created) {
Expand Down
18 changes: 8 additions & 10 deletions jicofo/src/main/kotlin/org/jitsi/jicofo/ktor/Application.kt
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ import org.jitsi.jicofo.xmpp.XmppCapsStats
import org.jitsi.utils.OrderedJsonObject
import org.jitsi.utils.logging2.createLogger
import org.jitsi.xmpp.extensions.jitsimeet.ConferenceIq
import org.jivesoftware.smack.packet.ErrorIQ
import org.jivesoftware.smack.packet.IQ
import org.jivesoftware.smack.packet.StanzaError
import org.json.simple.JSONArray
Expand Down Expand Up @@ -164,17 +163,16 @@ class Application(
throw BadRequest(e.message)
}

if (response !is ConferenceIq) {
if (response is ErrorIQ) {
throw when (response.error.condition) {
StanzaError.Condition.not_authorized -> Forbidden()
StanzaError.Condition.not_acceptable -> BadRequest("invalid-session")
else -> BadRequest(response.error.toString())
}
} else {
throw InternalError()
response.error?.let {
throw when (it.condition) {
StanzaError.Condition.not_authorized -> Forbidden()
StanzaError.Condition.not_acceptable -> BadRequest("invalid-session")
else -> BadRequest(it.toString())
}
}
if (response !is ConferenceIq) {
throw InternalError()
}
call.respond(ConferenceRequest.fromConferenceIq(response))
}
}
Expand Down
68 changes: 33 additions & 35 deletions jicofo/src/main/kotlin/org/jitsi/jicofo/xmpp/JigasiIqHandler.kt
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import org.jitsi.xmpp.extensions.rayo.DialIq
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jivesoftware.smack.AbstractXMPPConnection
import org.jivesoftware.smack.SmackException
import org.jivesoftware.smack.packet.ErrorIQ
import org.jivesoftware.smack.packet.IQ
import org.jivesoftware.smack.packet.StanzaError
import org.jivesoftware.smack.packet.id.StandardStanzaIdSource
Expand Down Expand Up @@ -157,47 +156,46 @@ class JigasiIqHandler(
return
}

when (responseFromJigasi) {
null, is ErrorIQ -> {
if (responseFromJigasi == null) {
logger.warn("Jigasi instance timed out: $jigasiJid")
Stats.singleInstanceTimeouts.inc()
} else {
logger.warn("Jigasi instance returned error ($jigasiJid): ${responseFromJigasi.toStringOpt()}")
Stats.singleInstanceErrors.inc()
}
if (responseFromJigasi == null || responseFromJigasi.error != null) {
// Timeout or error.
if (responseFromJigasi == null) {
logger.warn("Jigasi instance timed out: $jigasiJid")
Stats.singleInstanceTimeouts.inc()
} else {
logger.warn("Jigasi instance returned error ($jigasiJid): ${responseFromJigasi.toStringOpt()}")
Stats.singleInstanceErrors.inc()
}

if (retryCount > 0) {
logger.info("Will retry up to $retryCount more times.")
Stats.retries.inc()
// Do not try the same instance again.
inviteJigasi(request, conference, retryCount - 1, exclude + jigasiJid)
if (retryCount > 0) {
logger.info("Will retry up to $retryCount more times.")
Stats.retries.inc()
// Do not try the same instance again.
inviteJigasi(request, conference, retryCount - 1, exclude + jigasiJid)
} else {
val condition = if (responseFromJigasi == null) {
StanzaError.Condition.remote_server_timeout
} else {
val condition = if (responseFromJigasi == null) {
StanzaError.Condition.remote_server_timeout
} else {
StanzaError.Condition.undefined_condition
}
logger.warn("Request failed, all instances failed.")
stats.allInstancesFailed()
request.connection.tryToSendStanza(
IQ.createErrorResponse(request.iq, StanzaError.getBuilder(condition).build())
)
StanzaError.Condition.undefined_condition
}
}
else -> {
logger.info("response from jigasi: ${responseFromJigasi.toStringOpt()}")
// Successful response from Jigasi, forward it as the response to the client.
logger.warn("Request failed, all instances failed.")
stats.allInstancesFailed()
request.connection.tryToSendStanza(
responseFromJigasi.apply {
from = null
to = request.iq.from
stanzaId = request.iq.stanzaId
}
IQ.createErrorResponse(request.iq, StanzaError.getBuilder(condition).build())
)
return
}

return
}

logger.info("Response from jigasi: ${responseFromJigasi.toStringOpt()}")
// Successful response from Jigasi, forward it as the response to the client.
request.connection.tryToSendStanza(
responseFromJigasi.apply {
from = null
to = request.iq.from
stanzaId = request.iq.stanzaId
}
)
}

companion object {
Expand Down
Loading