use more approriate reason when failing because of parse errors

Daniel Gultsch created

Change summary

src/main/java/eu/siacs/conversations/xmpp/jingle/JingleRtpConnection.java | 19 
src/main/java/eu/siacs/conversations/xmpp/jingle/RtpContentMap.java       | 19 
src/main/java/eu/siacs/conversations/xmpp/jingle/stanzas/Reason.java      | 14 
3 files changed, 44 insertions(+), 8 deletions(-)

Detailed changes

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<Media> 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);

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);
+        }
+    }
 }

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;
+        }
+    }
 }