Skip to content

Commit

Permalink
fix: Avoid calling IQ.toXML().toString() (#2064)
Browse files Browse the repository at this point in the history
* chore: Update jitsi-xmpp-extensions.

* fix: Avoid using XmlStringBuilder.toString().
  • Loading branch information
bgrozev authored Oct 26, 2023
1 parent fe87250 commit c196776
Show file tree
Hide file tree
Showing 10 changed files with 35 additions and 24 deletions.
7 changes: 4 additions & 3 deletions jvb/src/main/java/org/jitsi/videobridge/Conference.java
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import org.jitsi.videobridge.util.*;
import org.jitsi.videobridge.xmpp.*;
import org.jitsi.xmpp.extensions.colibri2.*;
import org.jitsi.xmpp.util.*;
import org.jivesoftware.smack.packet.*;
import org.json.simple.*;
import org.jxmpp.jid.*;
Expand Down Expand Up @@ -230,7 +231,7 @@ public Conference(Videobridge videobridge,
{
try
{
logger.info("RECV colibri2 request: " + request.getRequest().toXML());
logger.info("RECV colibri2 request: " + XmlStringBuilderUtil.toStringOpt(request.getRequest()));
long start = System.currentTimeMillis();
Pair<IQ, Boolean> p = colibri2Handler.handleConferenceModifyIQ(request.getRequest());
IQ response = p.getFirst();
Expand All @@ -243,9 +244,9 @@ public Conference(Videobridge videobridge,
if (processingDelay > 100)
{
logger.warn("Took " + processingDelay + " ms to process an IQ (total delay "
+ totalDelay + " ms): " + request.getRequest().toXML());
+ totalDelay + " ms): " + XmlStringBuilderUtil.toStringOpt(request.getRequest()));
}
logger.info("SENT colibri2 response: " + response.toXML());
logger.info("SENT colibri2 response: " + XmlStringBuilderUtil.toStringOpt(response));
request.getCallback().invoke(response);
if (expire) videobridge.expireConference(this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.jitsi.xmpp.extensions.colibri.*;
import org.jitsi.xmpp.extensions.jingle.*;
import org.jitsi.xmpp.extensions.jitsimeet.*;
import org.jitsi.xmpp.util.*;
import org.jivesoftware.smack.packet.*;
import org.jxmpp.jid.parts.*;
import org.jxmpp.util.*;
Expand Down Expand Up @@ -402,7 +403,7 @@ private static List<SourceSsrcs> getSourceSsrcs(
logger.warn(
"Unprocessed source groups: " +
sourceGroupsCopy.stream()
.map(e -> e.toXML(XmlEnvironment.EMPTY).toString())
.map(XmlStringBuilderUtil::toStringOpt)
.reduce(String::concat));
}

Expand Down
5 changes: 3 additions & 2 deletions jvb/src/main/kotlin/org/jitsi/videobridge/Endpoint.kt
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ import org.jitsi.videobridge.websocket.colibriWebSocketServiceSupplier
import org.jitsi.xmpp.extensions.colibri.WebSocketPacketExtension
import org.jitsi.xmpp.extensions.jingle.DtlsFingerprintPacketExtension
import org.jitsi.xmpp.extensions.jingle.IceUdpTransportPacketExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jitsi_modified.sctp4j.SctpDataCallback
import org.jitsi_modified.sctp4j.SctpServerSocket
import org.jitsi_modified.sctp4j.SctpSocket
Expand Down Expand Up @@ -781,7 +782,7 @@ class Endpoint @JvmOverloads constructor(
if (fingerprintExtension.hash != null && fingerprintExtension.fingerprint != null) {
remoteFingerprints[fingerprintExtension.hash] = fingerprintExtension.fingerprint
} else {
logger.info("Ignoring empty DtlsFingerprint extension: ${transportInfo.toXML()}")
logger.info("Ignoring empty DtlsFingerprint extension: ${transportInfo.toStringOpt()}")
}
if (CryptexConfig.endpoint) {
cryptex = cryptex && fingerprintExtension.cryptex
Expand Down Expand Up @@ -810,7 +811,7 @@ class Endpoint @JvmOverloads constructor(
}
}

logger.cdebug { "Transport description:\n${iceUdpTransportPacketExtension.toXML()}" }
logger.cdebug { "Transport description:\n${iceUdpTransportPacketExtension.toStringOpt()}" }

return iceUdpTransportPacketExtension
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import org.jitsi.xmpp.extensions.colibri2.Sources
import org.jitsi.xmpp.extensions.colibri2.Transport
import org.jitsi.xmpp.extensions.jingle.RTPHdrExtPacketExtension
import org.jitsi.xmpp.extensions.jingle.SourceGroupPacketExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jitsi.xmpp.util.createError
import org.jivesoftware.smack.packet.IQ
import org.jivesoftware.smack.packet.StanzaError.Condition
Expand Down Expand Up @@ -109,7 +110,7 @@ class Colibri2ConferenceHandler(
val error = createEndpointNotFoundError(conferenceModifyIQ, e.endpointId)
Pair(error, false)
} catch (e: FeatureNotImplementedException) {
logger.warn("Unsupported request (${e.message}): ${conferenceModifyIQ.toXML()}")
logger.warn("Unsupported request (${e.message}): ${conferenceModifyIQ.toStringOpt()}")
Pair(createFeatureNotImplementedError(conferenceModifyIQ, e.message), false)
} catch (e: IqProcessingException) {
// Item not found conditions are assumed to be less critical, as they often happen in case a request
Expand Down Expand Up @@ -226,12 +227,12 @@ class Colibri2ConferenceHandler(
// TODO: support removing payload types/header extensions
media.payloadTypes.forEach { ptExt ->
create(ptExt, media.type)?.let { endpoint.addPayloadType(it) }
?: logger.warn("Ignoring unrecognized payload type extension: ${ptExt.toXML()}")
?: logger.warn("Ignoring unrecognized payload type extension: ${ptExt.toStringOpt()}")
}

media.rtpHdrExts.forEach { rtpHdrExt ->
rtpHdrExt.toRtpExtension()?.let { endpoint.addRtpExtension(it) }
?: logger.warn("Ignoring unrecognized RTP header extension: ${rtpHdrExt.toXML()}")
?: logger.warn("Ignoring unrecognized RTP header extension: ${rtpHdrExt.toStringOpt()}")
}

endpoint.setExtmapAllowMixed(media.extmapAllowMixed != null)
Expand Down Expand Up @@ -317,7 +318,7 @@ class Colibri2ConferenceHandler(

private fun SourceGroupPacketExtension.toSsrcAssociation(): SsrcAssociation? {
if (sources.size < 2) {
logger.warn("Ignoring source group with <2 sources: ${toXML()}")
logger.warn("Ignoring source group with <2 sources: ${toStringOpt()}")
return null
}

Expand Down Expand Up @@ -423,12 +424,12 @@ class Colibri2ConferenceHandler(
// TODO: support removing payload types/header extensions
media.payloadTypes.forEach { ptExt ->
create(ptExt, media.type)?.let { relay.addPayloadType(it) }
?: logger.warn("Ignoring unrecognized payload type extension: ${ptExt.toXML()}")
?: logger.warn("Ignoring unrecognized payload type extension: ${ptExt.toStringOpt()}")
}

media.rtpHdrExts.forEach { rtpHdrExt ->
rtpHdrExt.toRtpExtension()?.let { relay.addRtpExtension(it) }
?: logger.warn("Ignoring unrecognized RTP header extension: ${rtpHdrExt.toXML()}")
?: logger.warn("Ignoring unrecognized RTP header extension: ${rtpHdrExt.toStringOpt()}")
}

relay.setExtmapAllowMixed(media.extmapAllowMixed != null)
Expand Down Expand Up @@ -473,7 +474,9 @@ class Colibri2ConferenceHandler(
it.mediaSources.forEach { m ->
if (m.type == MediaType.AUDIO) {
if (m.sources.isEmpty()) {
logger.warn("Ignoring audio source ${m.id} in endpoint $id of a relay (no SSRCs): ${toXML()}")
logger.warn(
"Ignoring audio source ${m.id} in endpoint $id of a relay (no SSRCs): ${toStringOpt()}"
)
} else {
m.sources.forEach { audioSources.add(AudioSourceDesc(it.ssrc, id, m.id)) }
}
Expand Down
5 changes: 3 additions & 2 deletions jvb/src/main/kotlin/org/jitsi/videobridge/relay/Relay.kt
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ import org.jitsi.xmpp.extensions.colibri.WebSocketPacketExtension
import org.jitsi.xmpp.extensions.colibri2.Sctp
import org.jitsi.xmpp.extensions.jingle.DtlsFingerprintPacketExtension
import org.jitsi.xmpp.extensions.jingle.IceUdpTransportPacketExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jitsi_modified.sctp4j.SctpClientSocket
import org.jitsi_modified.sctp4j.SctpDataCallback
import org.jitsi_modified.sctp4j.SctpServerSocket
Expand Down Expand Up @@ -563,7 +564,7 @@ class Relay @JvmOverloads constructor(
if (fingerprintExtension.hash != null && fingerprintExtension.fingerprint != null) {
remoteFingerprints[fingerprintExtension.hash] = fingerprintExtension.fingerprint
} else {
logger.info("Ignoring empty DtlsFingerprint extension: ${transportInfo.toXML()}")
logger.info("Ignoring empty DtlsFingerprint extension: ${transportInfo.toStringOpt()}")
}

if (CryptexConfig.relay) {
Expand Down Expand Up @@ -615,7 +616,7 @@ class Relay @JvmOverloads constructor(
}
}

logger.cdebug { "Transport description:\n${iceUdpTransportPacketExtension.toXML()}" }
logger.cdebug { "Transport description:\n${iceUdpTransportPacketExtension.toStringOpt()}" }

return iceUdpTransportPacketExtension
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import org.jitsi.xmpp.extensions.jingle.CandidatePacketExtension
import org.jitsi.xmpp.extensions.jingle.IceCandidatePacketExtension
import org.jitsi.xmpp.extensions.jingle.IceRtcpmuxPacketExtension
import org.jitsi.xmpp.extensions.jingle.IceUdpTransportPacketExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import java.beans.PropertyChangeEvent
import java.beans.PropertyChangeListener
import java.io.IOException
Expand Down Expand Up @@ -204,7 +205,7 @@ class IceTransport @JvmOverloads constructor(
logger.info("Starting the Agent without remote candidates.")
iceAgent.startConnectivityEstablishment()
} else {
logger.cdebug { "Not starting ICE, no ufrag and pwd yet. ${transportPacketExtension.toXML()}" }
logger.cdebug { "Not starting ICE, no ufrag and pwd yet. ${transportPacketExtension.toStringOpt()}" }
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import org.jitsi.utils.MediaType.VIDEO
import org.jitsi.utils.logging2.Logger
import org.jitsi.utils.logging2.LoggerImpl
import org.jitsi.xmpp.extensions.jingle.PayloadTypePacketExtension
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import java.util.concurrent.ConcurrentHashMap

/**
Expand Down Expand Up @@ -71,7 +72,7 @@ class PayloadTypeUtil {
if (parameter.name != null) {
parameters[parameter.name] = parameter.value
} else {
logger.warn("Ignoring a format parameter with no name: " + parameter.toXML())
logger.warn("Ignoring a format parameter with no name: " + parameter.toStringOpt())
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import org.jitsi.xmpp.mucclient.IQListener
import org.jitsi.xmpp.mucclient.MucClient
import org.jitsi.xmpp.mucclient.MucClientConfiguration
import org.jitsi.xmpp.mucclient.MucClientManager
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jitsi.xmpp.util.createError
import org.jivesoftware.smack.packet.ExtensionElement
import org.jivesoftware.smack.packet.IQ
Expand Down Expand Up @@ -184,12 +185,12 @@ class XmppConnection : IQListener {
}
// colibri2 requests are logged at the conference level.
if (iq !is ConferenceModifyIQ) {
logger.cdebug { "RECV: ${iq.toXML()}" }
logger.cdebug { "RECV: ${iq.toStringOpt()}" }
}

return when (iq.type) {
IQ.Type.get, IQ.Type.set -> handleIqRequest(iq, mucClient)?.also {
logger.cdebug { "SENT: ${it.toXML()}" }
logger.cdebug { "SENT: ${it.toStringOpt()}" }
}
else -> null
}
Expand All @@ -202,7 +203,7 @@ class XmppConnection : IQListener {
"Service unavailable"
)
val response = when (iq) {
is Version -> measureDelay(versionDelayStats, { iq.toXML() }) {
is Version -> measureDelay(versionDelayStats, { iq.toStringOpt() }) {
handler.versionIqReceived(iq)
}
is ConferenceModifyIQ -> {
Expand All @@ -215,7 +216,7 @@ class XmppConnection : IQListener {
)
null
}
is HealthCheckIQ -> measureDelay(healthDelayStats, { iq.toXML() }) {
is HealthCheckIQ -> measureDelay(healthDelayStats, { iq.toStringOpt() }) {
handler.healthCheckIqReceived(iq)
}
else -> createError(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import org.jitsi.videobridge.colibri2.createConferenceNotFoundError
import org.jitsi.xmpp.extensions.colibri2.Colibri2Error
import org.jitsi.xmpp.extensions.colibri2.ConferenceModifyIQ
import org.jitsi.xmpp.extensions.colibri2.IqProviderUtils
import org.jitsi.xmpp.util.XmlStringBuilderUtil.Companion.toStringOpt
import org.jivesoftware.smack.util.PacketParserUtils

class Colibri2UtilTest : ShouldSpec({
Expand All @@ -29,7 +30,7 @@ class Colibri2UtilTest : ShouldSpec({
context("createConferenceAlreadyExistsError") {
val error = createConferenceAlreadyExistsError(iq, "i")

val parsedIq = parseIQ(error.toXML().toString())
val parsedIq = parseIQ(error.toStringOpt())
val colibri2ErrorExtension =
parsedIq.error.getExtension<Colibri2Error>(Colibri2Error.ELEMENT, Colibri2Error.NAMESPACE)
colibri2ErrorExtension shouldNotBe null
Expand All @@ -39,7 +40,7 @@ class Colibri2UtilTest : ShouldSpec({
context("createConferenceNotFoundError") {
val error = createConferenceNotFoundError(iq, "i")

val parsedIq = parseIQ(error.toXML().toString())
val parsedIq = parseIQ(error.toStringOpt())
val colibri2ErrorExtension =
parsedIq.error.getExtension<Colibri2Error>(Colibri2Error.ELEMENT, Colibri2Error.NAMESPACE)
colibri2ErrorExtension shouldNotBe null
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
<dependency>
<groupId>${project.groupId}</groupId>
<artifactId>jitsi-xmpp-extensions</artifactId>
<version>1.0-76-ge98f8af</version>
<version>1.0-78-g62d03d4</version>
</dependency>
</dependencies>
</dependencyManagement>
Expand Down

0 comments on commit c196776

Please sign in to comment.