From ff2f66fa67e5c364bab17d6407dca0cdc2e097a9 Mon Sep 17 00:00:00 2001 From: vanitasvitae Date: Thu, 3 Aug 2017 15:11:34 +0200 Subject: [PATCH] Do not store peers proposal as transport --- .../smackx/jingle/JingleManager.java | 9 +- .../jingle/components/JingleContent.java | 7 ++ .../jingle/components/JingleTransport.java | 71 +++++++----- .../jingle_ibb/JingleIBBTransport.java | 11 +- .../jingle_s5b/JingleS5BTransport.java | 105 ++++++++++-------- .../jingle_s5b/JingleS5BTransportAdapter.java | 2 +- .../jingle_s5b/JingleS5BTransportManager.java | 2 +- .../jingle/transport/JingleTransportTest.java | 2 +- 8 files changed, 126 insertions(+), 83 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/JingleManager.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/JingleManager.java index d2812758c..90fb825ca 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/JingleManager.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/JingleManager.java @@ -18,6 +18,7 @@ package org.jivesoftware.smackx.jingle; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -225,7 +226,12 @@ public final class JingleManager extends Manager { remaining.add(transportManagers.get(namespace)); } - Collections.sort(remaining); + Collections.sort(remaining, new Comparator() { + @Override + public int compare(JingleTransportManager t0, JingleTransportManager t1) { + return t1.compareTo(t0); //Invert otherwise ascending order to descending. + } + }); return remaining; } @@ -236,7 +242,6 @@ public final class JingleManager extends Manager { public JingleTransportManager getBestAvailableTransportManager(Set except) { List managers = getAvailableTransportManagers(except); - Collections.sort(managers); if (managers.size() > 0) { return managers.get(0); diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/components/JingleContent.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/components/JingleContent.java index f05bed425..f24b687f5 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/components/JingleContent.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/components/JingleContent.java @@ -161,6 +161,13 @@ public class JingleContent implements JingleTransportCallback, JingleSecurityCal public IQ handleSessionAccept(JingleElement request, XMPPConnection connection) { LOGGER.log(Level.INFO, "RECEIVED SESSION ACCEPT!"); + for (JingleContentElement contentElement : request.getContents()) { + if (contentElement.getName().equals(getName())) { + JingleContent content = fromElement(contentElement); + getTransport().setPeersProposal(content.getTransport()); + break; + } + } onAccept(connection); return IQ.createResultIQ(request); } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/components/JingleTransport.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/components/JingleTransport.java index 117ea37f9..44557cdf0 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/components/JingleTransport.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/components/JingleTransport.java @@ -18,7 +18,6 @@ package org.jivesoftware.smackx.jingle.components; import java.util.ArrayList; import java.util.List; -import java.util.logging.Level; import java.util.logging.Logger; import org.jivesoftware.smack.SmackException; @@ -39,29 +38,26 @@ public abstract class JingleTransport e private static final Logger LOGGER = Logger.getLogger(JingleTransport.class.getName()); private JingleContent parent; - private final ArrayList> candidates = new ArrayList<>(); - - private JingleTransport peersProposal; - private boolean isPeersProposal; + private final ArrayList> ourCandidates = new ArrayList<>(); + private final ArrayList> theirCandidates = new ArrayList<>(); protected BytestreamSession bytestreamSession; public abstract D getElement(); - public void addCandidate(JingleTransportCandidate candidate) { - LOGGER.log(Level.INFO, "Insert candidate."); - // Insert sorted by descending priority + public void addOurCandidate(JingleTransportCandidate candidate) { + // Insert sorted by descending priority // Empty list -> insert - if (candidates.isEmpty()) { - candidates.add(candidate); + if (ourCandidates.isEmpty()) { + ourCandidates.add(candidate); candidate.setParent(this); return; } // Find appropriate index - for (int i = 0; i < candidates.size(); i++) { - JingleTransportCandidate c = candidates.get(i); + for (int i = 0; i < ourCandidates.size(); i++) { + JingleTransportCandidate c = ourCandidates.get(i); // list already contains element -> return if (c == candidate || c.equals(candidate)) { @@ -70,15 +66,46 @@ public abstract class JingleTransport e //Found the index if (c.getPriority() <= candidate.getPriority()) { - candidates.add(i, candidate); + ourCandidates.add(i, candidate); candidate.setParent(this); return; } } } - public List> getCandidates() { - return candidates; + public void addTheirCandidate(JingleTransportCandidate candidate) { + // Insert sorted by descending priority + // Empty list -> insert + if (theirCandidates.isEmpty()) { + theirCandidates.add(candidate); + candidate.setParent(this); + return; + } + + // Find appropriate index + for (int i = 0; i < theirCandidates.size(); i++) { + JingleTransportCandidate c = theirCandidates.get(i); + + // list already contains element -> return + if (c == candidate || c.equals(candidate)) { + return; + } + + //Found the index + if (c.getPriority() <= candidate.getPriority()) { + theirCandidates.add(i, candidate); + candidate.setParent(this); + return; + } + } + } + + public List> getOurCandidates() { + return ourCandidates; + } + + public List> getTheirCandidates() { + return theirCandidates; } public abstract String getNamespace(); @@ -89,19 +116,7 @@ public abstract class JingleTransport e public abstract void establishOutgoingBytestreamSession(XMPPConnection connection, JingleTransportCallback callback, JingleSession session) throws SmackException.NotConnectedException, InterruptedException; - public void setPeersProposal(JingleTransport peersProposal) { - this.peersProposal = peersProposal; - peersProposal.setParent(getParent()); - peersProposal.isPeersProposal = true; - } - - public boolean isPeersProposal() { - return isPeersProposal; - } - - public JingleTransport getPeersProposal() { - return peersProposal; - } + public abstract void setPeersProposal(JingleTransport peersProposal); public abstract IQ handleTransportInfo(JingleContentTransportInfoElement info, JingleElement wrapping); diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transport/jingle_ibb/JingleIBBTransport.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transport/jingle_ibb/JingleIBBTransport.java index 91bb26193..77333e66c 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transport/jingle_ibb/JingleIBBTransport.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transport/jingle_ibb/JingleIBBTransport.java @@ -47,7 +47,7 @@ public class JingleIBBTransport extends JingleTransport candidate) { + public void setPeersProposal(JingleTransport peersProposal) { + JingleIBBTransport transport = (JingleIBBTransport) peersProposal; + //Respect peers wish for smaller block size. + blockSize = (blockSize < transport.blockSize ? blockSize : transport.blockSize); + } + + @Override + public void addOurCandidate(JingleTransportCandidate candidate) { // Sorry, we don't want any candidates. } diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transport/jingle_s5b/JingleS5BTransport.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transport/jingle_s5b/JingleS5BTransport.java index c5f7a96a2..107bc06e0 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transport/jingle_s5b/JingleS5BTransport.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/jingle/transport/jingle_s5b/JingleS5BTransport.java @@ -66,7 +66,8 @@ public class JingleS5BTransport extends JingleTransport> candidates) { - this(sid, Socks5Utils.createDigest(sid, initiator, responder), Bytestream.Mode.tcp, candidates); + public JingleS5BTransport(FullJid initiator, FullJid responder, String sid, List> ourCandidates, List> theirCandidates) { + this(sid, Socks5Utils.createDigest(sid, initiator, responder), Bytestream.Mode.tcp, ourCandidates, theirCandidates); } /** @@ -88,7 +89,7 @@ public class JingleS5BTransport extends JingleTransport> candidates) { this(other.getSid(), Socks5Utils.createDigest(other.getSid(), content.getParent().getInitiator(), content.getParent().getResponder()), - other.mode, candidates); + other.mode, candidates, other.getOurCandidates()); setPeersProposal(other); } @@ -99,14 +100,19 @@ public class JingleS5BTransport extends JingleTransport> candidates) { + public JingleS5BTransport(String sid, String dstAddr, Bytestream.Mode mode, List> candidates, List> theirCandidates) { this.sid = sid; this.dstAddr = dstAddr; this.mode = mode; for (JingleTransportCandidate c : (candidates != null ? candidates : Collections.emptySet())) { - addCandidate(c); + addOurCandidate(c); + } + + for (JingleTransportCandidate c : (theirCandidates != null ? + theirCandidates : Collections.emptySet())) { + addTheirCandidate(c); } } @@ -115,9 +121,9 @@ public class JingleS5BTransport extends JingleTransport c : transport.getCandidates()) { - this.addCandidate(new JingleS5BTransportCandidate((JingleS5BTransportCandidate) c)); + this(transport.sid, transport.dstAddr, transport.mode, null, null); + for (JingleTransportCandidate c : transport.getOurCandidates()) { + this.addOurCandidate(new JingleS5BTransportCandidate((JingleS5BTransportCandidate) c)); } } @@ -128,7 +134,7 @@ public class JingleS5BTransport extends JingleTransport candidate : getCandidates()) { + for (JingleTransportCandidate candidate : getOurCandidates()) { builder.addTransportCandidate((JingleS5BTransportCandidateElement) candidate.getElement()); } @@ -166,30 +172,45 @@ public class JingleS5BTransport extends JingleTransport peersProposal) { + JingleS5BTransport transport = (JingleS5BTransport) peersProposal; + getTheirCandidates().clear(); + for (JingleTransportCandidate c : transport.getOurCandidates()) { + addTheirCandidate(c); + } + } + void establishBytestreamSession(XMPPConnection connection) throws SmackException.NotConnectedException, InterruptedException { Socks5Proxy.getSocks5Proxy().addTransfer(dstAddr); JingleS5BTransportManager transportManager = JingleS5BTransportManager.getInstanceFor(connection); - this.selectedCandidate = connectToCandidates(MAX_TIMEOUT); + this.ourSelectedCandidate = connectToCandidates(MAX_TIMEOUT); - if (selectedCandidate == CANDIDATE_FAILURE) { + if (ourSelectedCandidate == CANDIDATE_FAILURE) { connection.createStanzaCollectorAndSend(transportManager.createCandidateError(this)); return; } - if (selectedCandidate == null) { + if (ourSelectedCandidate == null) { throw new AssertionError("MUST NOT BE NULL."); } - connection.createStanzaCollectorAndSend(transportManager.createCandidateUsed(this, selectedCandidate)); + connection.createStanzaCollectorAndSend(transportManager.createCandidateUsed(this, ourSelectedCandidate)); connectIfReady(); } public JingleS5BTransportCandidate connectToCandidates(int timeout) { - for (JingleTransportCandidate c : getPeersProposal().getCandidates()) { - int _timeout = timeout / getCandidates().size(); //TODO: Wise? + + if (getTheirCandidates().size() == 0) { + return CANDIDATE_FAILURE; + } + + int _timeout = timeout / getTheirCandidates().size(); //TODO: Wise? + for (JingleTransportCandidate c : getTheirCandidates()) { + JingleS5BTransportCandidate candidate = (JingleS5BTransportCandidate) c; try { - return ((JingleS5BTransportCandidate) c).connect(_timeout, true); + return candidate.connect(_timeout, true); } catch (IOException | TimeoutException | InterruptedException | SmackException | XMPPException e) { LOGGER.log(Level.WARNING, "Exception while connecting to candidate: " + e, e); } @@ -201,16 +222,15 @@ public class JingleS5BTransport extends JingleTransport peers.getSelectedCandidate().getPriority()) { - nominated = getSelectedCandidate(); - } else if (getSelectedCandidate().getPriority() < peers.getSelectedCandidate().getPriority()) { - nominated = peers.getSelectedCandidate(); + if (ourSelectedCandidate.getPriority() > theirSelectedCandidate.getPriority()) { + nominated = ourSelectedCandidate; + } else if (ourSelectedCandidate.getPriority() < theirSelectedCandidate.getPriority()) { + nominated = theirSelectedCandidate; } else { - nominated = getParent().getParent().isInitiator() ? getSelectedCandidate() : peers.getSelectedCandidate(); + nominated = getParent().getParent().isInitiator() ? ourSelectedCandidate : theirSelectedCandidate; } - } else if (getSelectedCandidate() != CANDIDATE_FAILURE) { - nominated = getSelectedCandidate(); + } else if (ourSelectedCandidate != CANDIDATE_FAILURE) { + nominated = ourSelectedCandidate; } else { - nominated = peers.getSelectedCandidate(); + nominated = theirSelectedCandidate; } - if (nominated == peers.getSelectedCandidate()) { + if (nominated == theirSelectedCandidate) { LOGGER.log(Level.INFO, "Their choice, so our proposed candidate is used."); @@ -325,10 +345,8 @@ public class JingleS5BTransport extends JingleTransport out-of-order! - if (peers.getSelectedCandidate() != null) { + if (theirSelectedCandidate != null) { try { jingleManager.getConnection().sendStanza(JingleElement.createJingleErrorOutOfOrder(wrapping)); //jingleManager.getConnection().createStanzaCollectorAndSend(JingleElement.createJingleErrorOutOfOrder(wrapping)); @@ -338,15 +356,15 @@ public class JingleS5BTransport extends JingleTransport> ourCandidates = getCandidates().iterator(); + Iterator> ourCandidates = getOurCandidates().iterator(); while (ourCandidates.hasNext()) { JingleS5BTransportCandidate candidate = (JingleS5BTransportCandidate) ourCandidates.next(); if (candidate.getCandidateId().equals(candidateId)) { - peers.setSelectedCandidate(candidate); + theirSelectedCandidate = candidate; } } - if (peers.getSelectedCandidate() == null) { + if (theirSelectedCandidate == null) { LOGGER.log(Level.SEVERE, "ILLEGAL CANDIDATE ID!!!"); //TODO: Alert! Illegal candidateId! } @@ -355,28 +373,19 @@ public class JingleS5BTransport extends JingleTransport createTransport(JingleContent content) { JingleSession session = content.getParent(); List> candidates = collectCandidates(); - return new JingleS5BTransport(session.getInitiator(), session.getResponder(), StringUtils.randomString(24), candidates); + return new JingleS5BTransport(session.getInitiator(), session.getResponder(), StringUtils.randomString(24), candidates, null); } @Override diff --git a/smack-integration-test/src/main/java/org/jivesoftware/smackx/jingle/transport/JingleTransportTest.java b/smack-integration-test/src/main/java/org/jivesoftware/smackx/jingle/transport/JingleTransportTest.java index c6199afcb..0e9a1e678 100644 --- a/smack-integration-test/src/main/java/org/jivesoftware/smackx/jingle/transport/JingleTransportTest.java +++ b/smack-integration-test/src/main/java/org/jivesoftware/smackx/jingle/transport/JingleTransportTest.java @@ -55,7 +55,7 @@ public class JingleTransportTest extends AbstractSmackIntegrationTest { super(environment); } - //@SmackIntegrationTest + @SmackIntegrationTest public void JingleIBBTest() throws Exception { XMPPConnection sender = conOne; XMPPConnection receiver = conTwo;