From b95d406e61fb84bfe5019b22792434845ca634c5 Mon Sep 17 00:00:00 2001 From: Daniel Gultsch Date: Thu, 16 Apr 2020 08:20:13 +0200 Subject: [PATCH] use more approriate reason when failing because of parse errors --- .../xmpp/jingle/JingleRtpConnection.java | 19 +++++++++++++++---- .../xmpp/jingle/RtpContentMap.java | 19 +++++++++++++++---- .../xmpp/jingle/stanzas/Reason.java | 14 ++++++++++++++ 3 files changed, 44 insertions(+), 8 deletions(-) diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java index 7682c1b91..f2a877d04 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java @@ -266,7 +266,7 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web contentMap.requireDTLSFingerprint(); } catch (final IllegalArgumentException | IllegalStateException | NullPointerException e) { respondOk(jinglePacket); - sendSessionTerminate(Reason.FAILED_APPLICATION, e.getMessage()); + sendSessionTerminate(Reason.of(e), e.getMessage()); Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": improperly formatted contents", e); return; } @@ -315,14 +315,22 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web contentMap = RtpContentMap.of(jinglePacket); contentMap.requireContentDescriptions(); contentMap.requireDTLSFingerprint(); - } catch (IllegalArgumentException | IllegalStateException | NullPointerException e) { + } catch (final IllegalArgumentException | IllegalStateException | NullPointerException e) { respondOk(jinglePacket); Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": improperly formatted contents in session-accept", e); webRTCWrapper.close(); - sendSessionTerminate(Reason.FAILED_APPLICATION, e.getMessage()); + sendSessionTerminate(Reason.of(e), e.getMessage()); + return; + } + final Set initiatorMedia = this.initiatorRtpContentMap.getMedia(); + if (!initiatorMedia.equals(contentMap.getMedia())) { + sendSessionTerminate(Reason.SECURITY_ERROR, String.format( + "Your session-included included media %s but our session-initiate was %s", + this.proposedMedia, + contentMap.getMedia() + )); return; } - //TODO check that session accept content media matched ours Log.d(Config.LOGTAG, "processing session-accept with " + contentMap.contents.size() + " contents"); if (transition(State.SESSION_ACCEPTED)) { respondOk(jinglePacket); @@ -913,6 +921,9 @@ public class JingleRtpConnection extends AbstractJingleConnection implements Web if (newState == PeerConnection.PeerConnectionState.CONNECTED && this.rtpConnectionStarted == 0) { this.rtpConnectionStarted = SystemClock.elapsedRealtime(); } + //TODO 'DISCONNECTED' might be an opportunity to renew the offer and send a transport-replace + //TODO exact syntax is yet to be determined but transport-replace sounds like the most reasonable + //as there is no content-replace if (Arrays.asList(PeerConnection.PeerConnectionState.FAILED, PeerConnection.PeerConnectionState.DISCONNECTED).contains(newState)) { if (TERMINATED.contains(this.state)) { Log.d(Config.LOGTAG, id.account.getJid().asBareJid() + ": not sending session-terminate after connectivity error because session is already in state " + this.state); diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java index db9902874..da48d017f 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java @@ -23,6 +23,7 @@ import eu.siacs.conversations.xmpp.jingle.stanzas.GenericTransportInfo; import eu.siacs.conversations.xmpp.jingle.stanzas.Group; import eu.siacs.conversations.xmpp.jingle.stanzas.IceUdpTransportInfo; import eu.siacs.conversations.xmpp.jingle.stanzas.JinglePacket; +import eu.siacs.conversations.xmpp.jingle.stanzas.Reason; import eu.siacs.conversations.xmpp.jingle.stanzas.RtpDescription; public class RtpContentMap { @@ -130,14 +131,12 @@ public class RtpContentMap { rtpDescription = (RtpDescription) description; } else { Log.d(Config.LOGTAG, "description was " + description); - //todo throw unsupported application - throw new IllegalArgumentException("Content does not contain RtpDescription"); + throw new UnsupportedApplicationException("Content does not contain rtp description"); } if (transportInfo instanceof IceUdpTransportInfo) { iceUdpTransportInfo = (IceUdpTransportInfo) transportInfo; } else { - //TODO throw UNSUPPORTED_TRANSPORT exception - throw new IllegalArgumentException("Content does not contain ICE-UDP transport"); + throw new UnsupportedTransportException("Content does not contain ICE-UDP transport"); } return new DescriptionTransport(rtpDescription, iceUdpTransportInfo); } @@ -158,4 +157,16 @@ public class RtpContentMap { })); } } + + public static class UnsupportedApplicationException extends IllegalArgumentException { + UnsupportedApplicationException(String message) { + super(message); + } + } + + public static class UnsupportedTransportException extends IllegalArgumentException { + UnsupportedTransportException(String message) { + super(message); + } + } } diff --git a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Reason.java b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Reason.java index 8fee7a552..9e4c8d95d 100644 --- a/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Reason.java +++ b/src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Reason.java @@ -4,6 +4,8 @@ import android.support.annotation.NonNull; import com.google.common.base.CaseFormat; +import eu.siacs.conversations.xmpp.jingle.RtpContentMap; + public enum Reason { ALTERNATIVE_SESSION, BUSY, @@ -37,4 +39,16 @@ public enum Reason { public String toString() { return CaseFormat.UPPER_UNDERSCORE.to(CaseFormat.LOWER_HYPHEN, super.toString()); } + + public static Reason of(final RuntimeException e) { + if (e instanceof SecurityException) { + return SECURITY_ERROR; + } else if (e instanceof RtpContentMap.UnsupportedTransportException) { + return UNSUPPORTED_TRANSPORTS; + } else if (e instanceof RtpContentMap.UnsupportedApplicationException) { + return UNSUPPORTED_APPLICATIONS; + } else { + return FAILED_APPLICATION; + } + } } \ No newline at end of file