From 91e97e936fe671678758702bbede6a47b5487f13 Mon Sep 17 00:00:00 2001 From: Alex Clayton Date: Tue, 26 Jan 2016 16:33:29 +0000 Subject: Some fixes for File Transfer Patch Some fixes that were required for the File Transfer Patch (see patch notes on Gerrit). Test-information: By code inspection. Ran against MLC (after modification so it works for stroke interface changes introduces in a previous patch) it still runs correctly. Ran unit tests they still all pass. Change-Id: Ib49d9f9160f5e6b6b578f16695f8e8bc0f96a412 diff --git a/src/com/isode/stroke/elements/JingleS5BTransportPayload.java b/src/com/isode/stroke/elements/JingleS5BTransportPayload.java index 546a41a..5da70e3 100644 --- a/src/com/isode/stroke/elements/JingleS5BTransportPayload.java +++ b/src/com/isode/stroke/elements/JingleS5BTransportPayload.java @@ -53,15 +53,6 @@ public class JingleS5BTransportPayload extends JingleTransportPayload { else if (c1.priority < c2.priority) { return -1; } else { return 1; } } - - public boolean equals(Object c) { - if(!(c instanceof JingleS5BTransportPayload.Candidate)) { - return false; - } - else { - return this.equals(c); - } - } } private Mode mode; diff --git a/src/com/isode/stroke/filetransfer/ByteArrayReadBytestream.java b/src/com/isode/stroke/filetransfer/ByteArrayReadBytestream.java index 4c3d47d..41528fa 100644 --- a/src/com/isode/stroke/filetransfer/ByteArrayReadBytestream.java +++ b/src/com/isode/stroke/filetransfer/ByteArrayReadBytestream.java @@ -30,7 +30,7 @@ public class ByteArrayReadBytestream extends ReadBytestream { if (position + readSize > data.getSize()) { readSize = data.getSize() - position; } - String s = new String(data.getData()); + String s = data.toString(); s = s.substring(position, position+readSize); ByteArray result = new ByteArray(s); diff --git a/src/com/isode/stroke/filetransfer/DefaultFileTransferTransporter.java b/src/com/isode/stroke/filetransfer/DefaultFileTransferTransporter.java index f06407e..c14ee86 100644 --- a/src/com/isode/stroke/filetransfer/DefaultFileTransferTransporter.java +++ b/src/com/isode/stroke/filetransfer/DefaultFileTransferTransporter.java @@ -69,13 +69,14 @@ public class DefaultFileTransferTransporter extends FileTransferTransporter { this.responder = responder; this.role = role; this.s5bRegistry = s5bRegistry; + this.s5bServerManager = s5bServerManager; this.s5bProxy = s5bProxy; this.crypto = crypto; this.router = router; localCandidateGenerator = new LocalJingleTransportCandidateGenerator( s5bServerManager, s5bProxy, - (Role.Initiator.equals(role) ? initiator : responder), + (role == Role.Initiator ? initiator : responder), idGenerator, options); localCandidateGenerator.onLocalTransportCandidatesGenerated.connect(new Slot1>() { @@ -132,7 +133,7 @@ public class DefaultFileTransferTransporter extends FileTransferTransporter { logger_.fine("Start activating proxy " + proxyServiceJID.toString() + " with sid = " + s5bSessionID + ".\n"); S5BProxyRequest proxyRequest = new S5BProxyRequest(); proxyRequest.setSID(s5bSessionID); - proxyRequest.setActivate(Role.Initiator.equals(role) ? responder : initiator); + proxyRequest.setActivate(role == Role.Initiator ? responder : initiator); GenericRequest request = new GenericRequest(IQ.Type.Set, proxyServiceJID, proxyRequest, router); request.onResponse.connect(new Slot2() { diff --git a/src/com/isode/stroke/filetransfer/FileReadBytestream.java b/src/com/isode/stroke/filetransfer/FileReadBytestream.java index d349c17..53cb974 100644 --- a/src/com/isode/stroke/filetransfer/FileReadBytestream.java +++ b/src/com/isode/stroke/filetransfer/FileReadBytestream.java @@ -31,9 +31,10 @@ public class FileReadBytestream extends ReadBytestream { if (stream == null) { stream = new FileInputStream(file); } - ByteArray result = new ByteArray(); //assert(stream.good()); - stream.read(result.getData(), 0, size); + byte[] buffer = new byte[size]; + stream.read(buffer, 0, size); + ByteArray result = new ByteArray(buffer); onRead.emit(result); return result; } diff --git a/src/com/isode/stroke/filetransfer/FileTransferTransporter.java b/src/com/isode/stroke/filetransfer/FileTransferTransporter.java index d20550d..4aea154 100644 --- a/src/com/isode/stroke/filetransfer/FileTransferTransporter.java +++ b/src/com/isode/stroke/filetransfer/FileTransferTransporter.java @@ -49,7 +49,7 @@ public abstract class FileTransferTransporter { public abstract TransportSession createLocalCandidateSession( WriteBytestream w, final JingleS5BTransportPayload.Candidate candidate); - public final Signal3, String /* dstAddr */> onLocalCandidatesGenerated = new Signal3, String>(); - public final Signal2 onRemoteCandidateSelectFinished = new Signal2(); - public final Signal2 onProxyActivated = new Signal2(); + public final Signal3, String> onLocalCandidatesGenerated = new Signal3, String>(); + public final Signal2 onRemoteCandidateSelectFinished = new Signal2(); + public final Signal2 onProxyActivated = new Signal2(); } \ No newline at end of file diff --git a/src/com/isode/stroke/filetransfer/IncomingJingleFileTransfer.java b/src/com/isode/stroke/filetransfer/IncomingJingleFileTransfer.java index c5001e0..92b185d 100644 --- a/src/com/isode/stroke/filetransfer/IncomingJingleFileTransfer.java +++ b/src/com/isode/stroke/filetransfer/IncomingJingleFileTransfer.java @@ -392,9 +392,8 @@ public class IncomingJingleFileTransfer extends JingleFileTransfer implements In return; } - JingleIBBTransportPayload ibbTransport; if (options.isInBandAllowed() && transport instanceof JingleIBBTransportPayload) { - ibbTransport = (JingleIBBTransportPayload)transport; + JingleIBBTransportPayload ibbTransport = (JingleIBBTransportPayload)transport; logger_.fine("transport replaced with IBB\n"); startTransferring(transporter.createIBBReceiveSession(ibbTransport.getSessionID(), (int)description.getFileInfo().getSize(), stream)); diff --git a/src/com/isode/stroke/filetransfer/IncrementalBytestreamHashCalculator.java b/src/com/isode/stroke/filetransfer/IncrementalBytestreamHashCalculator.java index e79a7a5..0b8902e 100644 --- a/src/com/isode/stroke/filetransfer/IncrementalBytestreamHashCalculator.java +++ b/src/com/isode/stroke/filetransfer/IncrementalBytestreamHashCalculator.java @@ -42,15 +42,6 @@ public class IncrementalBytestreamHashCalculator { } } - /*void feedData(const SafeByteArray& data) { - if (md5Hasher) { - md5Hasher.update(createByteArray(data.data(), data.size())); - } - if (sha1Hasher) { - sha1Hasher.update(createByteArray(data.data(), data.size())); - } - }*/ - public ByteArray getSHA1Hash() { assert(sha1Hasher != null); if (sha1Hash == null) { diff --git a/src/com/isode/stroke/filetransfer/LocalJingleTransportCandidateGenerator.java b/src/com/isode/stroke/filetransfer/LocalJingleTransportCandidateGenerator.java index 1cacda7..3839b26 100644 --- a/src/com/isode/stroke/filetransfer/LocalJingleTransportCandidateGenerator.java +++ b/src/com/isode/stroke/filetransfer/LocalJingleTransportCandidateGenerator.java @@ -46,7 +46,12 @@ public class LocalJingleTransportCandidateGenerator { private SignalConnection onDiscoveredProxiesChangedConnection; private Logger logger_ = Logger.getLogger(this.getClass().getName()); - public LocalJingleTransportCandidateGenerator( + /** + * {@link SignalConnection} to {@link #s5bServerResourceUser_.onSuccessfulInitialized} + */ + private SignalConnection onSuccessfulInitializedConnection_; + + public LocalJingleTransportCandidateGenerator( SOCKS5BytestreamServerManager s5bServerManager, SOCKS5BytestreamProxiesManager s5bProxy, final JID ownJID, @@ -70,7 +75,7 @@ public class LocalJingleTransportCandidateGenerator { handleS5BServerInitialized(true); } else { - s5bServerResourceUser_.onSuccessfulInitialized.connect(new Slot1() { + onSuccessfulInitializedConnection_ = s5bServerResourceUser_.onSuccessfulInitialized.connect(new Slot1() { @Override public void call(Boolean b) { handleS5BServerInitialized(b); @@ -100,17 +105,17 @@ public class LocalJingleTransportCandidateGenerator { s5bServerPortForwardingUser_.onSetup.disconnectAll(); s5bServerPortForwardingUser_ = null; } - if (s5bServerResourceUser_ != null) { - s5bServerResourceUser_.onSuccessfulInitialized.disconnectAll(); - s5bServerResourceUser_ = null; + if (onSuccessfulInitializedConnection_ != null) { + onSuccessfulInitializedConnection_.disconnect(); } + s5bServerResourceUser_ = null; } public Signal1> onLocalTransportCandidatesGenerated = new Signal1>(); - + private void handleS5BServerInitialized(boolean success) { - if (s5bServerResourceUser_ != null) { - s5bServerResourceUser_.onSuccessfulInitialized.disconnectAll(); + if (onSuccessfulInitializedConnection_ != null) { + onSuccessfulInitializedConnection_.disconnect(); } triedServerInit_ = true; if (success) { @@ -130,9 +135,6 @@ public class LocalJingleTransportCandidateGenerator { } else { logger_.warning("Unable to start SOCKS5 server\n"); - if (s5bServerResourceUser_ != null) { - s5bServerResourceUser_.onSuccessfulInitialized.disconnectAll(); - } s5bServerResourceUser_ = null; handlePortForwardingSetup(false); } @@ -181,7 +183,7 @@ public class LocalJingleTransportCandidateGenerator { } if (options_.isAssistedAllowed()) { - // get assissted candidates + // get assisted candidates Vector assisstedCandidates = s5bServerManager.getAssistedHostAddressPorts(); for(HostAddressPort addressPort : assisstedCandidates) { JingleS5BTransportPayload.Candidate candidate = new JingleS5BTransportPayload.Candidate(); diff --git a/src/com/isode/stroke/filetransfer/RemoteJingleTransportCandidateSelector.java b/src/com/isode/stroke/filetransfer/RemoteJingleTransportCandidateSelector.java index 1d1c822..5cbc3ae 100644 --- a/src/com/isode/stroke/filetransfer/RemoteJingleTransportCandidateSelector.java +++ b/src/com/isode/stroke/filetransfer/RemoteJingleTransportCandidateSelector.java @@ -75,8 +75,7 @@ public class RemoteJingleTransportCandidateSelector { onCandidateSelectFinished.emit(null, null); } else { - lastCandidate = candidates.peek(); - candidates.poll(); + lastCandidate = candidates.poll(); logger_.fine("Trying candidate " + lastCandidate.cid + "\n"); if ((lastCandidate.type.equals(JingleS5BTransportPayload.Candidate.Type.DirectType) && options.isDirectAllowed()) || (lastCandidate.type.equals(JingleS5BTransportPayload.Candidate.Type.AssistedType) && options.isAssistedAllowed()) || diff --git a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamClientSession.java b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamClientSession.java index 25aadc4..0144bcb 100644 --- a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamClientSession.java +++ b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamClientSession.java @@ -46,7 +46,7 @@ public class SOCKS5BytestreamClientSession { private State(int x) { description = x; } - public int description; + public final int description; }; private Connection connection; @@ -274,7 +274,7 @@ public class SOCKS5BytestreamClientSession { process(); } else { - //---------writeBytestream.write(new ByteArray(vecptr(*data), data.size())); + writeBytestream.write(data); //onBytesReceived(data.size()); } } diff --git a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamProxiesManager.java b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamProxiesManager.java index 14c7ea3..eab3031 100644 --- a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamProxiesManager.java +++ b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamProxiesManager.java @@ -25,12 +25,15 @@ import com.isode.stroke.network.DomainNameResolveError; import com.isode.stroke.network.DomainNameAddressQuery; import com.isode.stroke.network.HostAddress; import com.isode.stroke.queries.IQRouter; +import com.isode.stroke.session.Session; import com.isode.stroke.signals.Slot1; import com.isode.stroke.signals.Slot2; import com.isode.stroke.signals.Signal; import com.isode.stroke.signals.SignalConnection; import com.isode.stroke.elements.S5BProxyRequest; import com.isode.stroke.jid.JID; + +import java.util.Iterator; import java.util.Vector; import java.util.Collection; import java.util.Map; @@ -49,9 +52,9 @@ public class SOCKS5BytestreamProxiesManager { private IQRouter iqRouter_; private JID serviceRoot_; private Logger logger_ = Logger.getLogger(this.getClass().getName()); - private SignalConnection onSessionReadyConnection; - private SignalConnection onFinishedConnection; + // TODO plonk this into the pair or not + // TODO think what this is trying to do? private static class Pair { public JID jid; public SOCKS5BytestreamClientSession sock5; @@ -61,6 +64,20 @@ public class SOCKS5BytestreamProxiesManager { private Map > proxySessions_ = new HashMap >(); + /** + * Map between {@link SOCKS5BytestreamClientSession} and a {@link SignalConnection} to their + * {@link SOCKS5BytestreamClientSession#onSessionReady} + */ + private Map onSessionReadyConnectionMap = + new HashMap(); + + /** + * Map between {@link SOCKS5BytestreamClientSession} and a {@link SignalConnection} to their + * {@link SOCKS5BytestreamClientSession#onFinished} + */ + private Map onFinishedConnectionMap = + new HashMap(); + private SOCKS5BytestreamProxyFinder proxyFinder_; private Collection localS5BProxies_; @@ -107,18 +124,20 @@ public class SOCKS5BytestreamProxiesManager { final SOCKS5BytestreamClientSession session = new SOCKS5BytestreamClientSession(conn, addressPort, sessionID, timerFactory_); final JID proxyJid = proxy.getStreamHost().jid; clientSessions.add(new Pair(proxyJid, session)); - onSessionReadyConnection = session.onSessionReady.connect(new Slot1() { + SignalConnection onSessionReadyConnection = session.onSessionReady.connect(new Slot1() { @Override public void call(Boolean b) { handleProxySessionReady(sessionID, proxyJid, session, b); } }); - onFinishedConnection = session.onFinished.connect(new Slot1() { + onSessionReadyConnectionMap.put(session, onSessionReadyConnection); + SignalConnection onFinishedConnection = session.onFinished.connect(new Slot1() { @Override public void call(FileTransferError e) { handleProxySessionFinished(sessionID, proxyJid, session, e); } }); + onFinishedConnectionMap.put(session, onFinishedConnection); session.start(); } } @@ -135,8 +154,16 @@ public class SOCKS5BytestreamProxiesManager { // get active session SOCKS5BytestreamClientSession activeSession = null; for(Pair i : proxySessions_.get(sessionID)) { - i.sock5.onSessionReady.disconnectAll(); - i.sock5.onFinished.disconnectAll(); + SignalConnection onSessionReadyConnection = + onSessionReadyConnectionMap.remove(i.sock5); + if (onSessionReadyConnection != null) { + onSessionReadyConnection.disconnect(); + } + SignalConnection onFinishedConnection = + onFinishedConnectionMap.remove(i.sock5); + if (onFinishedConnection != null) { + onFinishedConnection.disconnect(); + } if (i.jid.equals(proxyJID) && activeSession == null) { activeSession = i.sock5; } @@ -217,32 +244,42 @@ public class SOCKS5BytestreamProxiesManager { } private void handleProxySessionReady(final String sessionID, final JID jid, SOCKS5BytestreamClientSession session, boolean error) { - onSessionReadyConnection.disconnect(); + SignalConnection onSessionReadyConnection = onSessionReadyConnectionMap.remove(session); + if (onSessionReadyConnection != null) { + onSessionReadyConnection.disconnect(); + } if (!error) { // The SOCKS5 bytestream session to the proxy succeeded; stop and remove other sessions. if (proxySessions_.containsKey(sessionID)) { - for(Pair i : proxySessions_.get(sessionID)) { - if ((i.jid.equals(jid)) && (!i.sock5.equals(session))) { - i.sock5.stop(); - proxySessions_.get(sessionID).remove(i); //Swiften assigns i, so that iterator points to the next element. - } + Iterator iterator = proxySessions_.get(sessionID).iterator(); + while (iterator.hasNext()) { + Pair i = iterator.next(); + if ((i.jid.equals(jid)) && (!i.sock5.equals(session))) { + i.sock5.stop(); + iterator.remove();; //Swiften assigns i, so that iterator points to the next element. + } } } } } private void handleProxySessionFinished(final String sessionID, final JID jid, SOCKS5BytestreamClientSession session, FileTransferError error) { - onFinishedConnection.disconnect(); + SignalConnection onFinishedConnection = onFinishedConnectionMap.remove(session); + if (onFinishedConnection != null) { + onFinishedConnection.disconnect(); + } if (error != null) { // The SOCKS5 bytestream session to the proxy failed; remove it. if (proxySessions_.containsKey(sessionID)) { - for(Pair i : proxySessions_.get(sessionID)) { - if ((i.jid.equals(jid)) && (i.sock5.equals(session))) { - i.sock5.stop(); - proxySessions_.get(sessionID).remove(i); //Swiften assigns i, so that iterator points to the next element. - break; - } - } + Iterator iterator = proxySessions_.get(sessionID).iterator(); + while (iterator.hasNext()) { + Pair i = iterator.next(); + if ((i.jid.equals(jid)) && (i.sock5.equals(session))) { + i.sock5.stop(); + iterator.remove();; //Swiften assigns i, so that iterator points to the next element. + break; + } + } } } } diff --git a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServer.java b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServer.java index 3a745e2..1c9eb31 100644 --- a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServer.java +++ b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServer.java @@ -18,6 +18,10 @@ import com.isode.stroke.jid.JID; import com.isode.stroke.signals.SignalConnection; import com.isode.stroke.signals.Slot1; import com.isode.stroke.signals.Slot; + +import java.util.HashMap; +import java.util.Map; +import java.util.Set; import java.util.Vector; public class SOCKS5BytestreamServer { @@ -26,7 +30,13 @@ public class SOCKS5BytestreamServer { private SOCKS5BytestreamRegistry registry; private Vector sessions = new Vector(); private SignalConnection onNewConnectionConn; - private SignalConnection onFinishedConnection; + + /** + * Map between {@link SOCKS5BytestreamServerSession} and the {@link SignalConnection} + * to that session's {@link SOCKS5BytestreamServerSession#onFinished} signal. + */ + private Map sessionOnFinishedConnectionMap = + new HashMap(); public SOCKS5BytestreamServer(ConnectionServer connectionServer, SOCKS5BytestreamRegistry registry) { this.connectionServer = connectionServer; @@ -49,7 +59,10 @@ public class SOCKS5BytestreamServer { public void stop() { onNewConnectionConn.disconnect(); for (SOCKS5BytestreamServerSession session : sessions) { - session.onFinished.disconnectAll(); + SignalConnection onFinishedConnection = sessionOnFinishedConnectionMap.remove(session); + if (onFinishedConnection != null) { + onFinishedConnection.disconnect(); + } session.stop(); } sessions.clear(); @@ -67,20 +80,25 @@ public class SOCKS5BytestreamServer { private void handleNewConnection(Connection connection) { final SOCKS5BytestreamServerSession session = new SOCKS5BytestreamServerSession(connection, registry); - onFinishedConnection = session.onFinished.connect(new Slot1() { + SignalConnection onFinishedConnection = session.onFinished.connect(new Slot1() { @Override public void call(FileTransferError e) { handleSessionFinished(session); } }); sessions.add(session); + sessionOnFinishedConnectionMap.put(session, onFinishedConnection); + session.start(); } private void handleSessionFinished(SOCKS5BytestreamServerSession session) { - while(sessions.contains(session)) { - sessions.remove(session); + while(sessions.remove(session)) { + // Loop will run till session no longer is sessions + } + SignalConnection onFinishedConnection = sessionOnFinishedConnectionMap.remove(session); + if (onFinishedConnection != null) { + onFinishedConnection.disconnect(); } - onFinishedConnection.disconnect(); } } \ No newline at end of file diff --git a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServerPortForwardingUser.java b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServerPortForwardingUser.java index dc3c3b7..277d7f8 100644 --- a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServerPortForwardingUser.java +++ b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServerPortForwardingUser.java @@ -23,7 +23,7 @@ public class SOCKS5BytestreamServerPortForwardingUser { public SOCKS5BytestreamServerPortForwardingUser(SOCKS5BytestreamServerManager s5bServerManager) { this.s5bServerManager_ = s5bServerManager; // the server should be initialized, so we know what port to setup a forward for - assert(s5bServerManager != null); + assert(s5bServerManager.isInitialized()); if (s5bServerManager_.isPortForwardingReady()) { onSetup.emit(!s5bServerManager_.getAssistedHostAddressPorts().isEmpty()); } diff --git a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServerResourceUser.java b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServerResourceUser.java index acb8af1..c814b1d 100644 --- a/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServerResourceUser.java +++ b/src/com/isode/stroke/filetransfer/SOCKS5BytestreamServerResourceUser.java @@ -22,7 +22,7 @@ public class SOCKS5BytestreamServerResourceUser { public SOCKS5BytestreamServerResourceUser(SOCKS5BytestreamServerManager s5bServerManager) { this.s5bServerManager_ = s5bServerManager; - assert(s5bServerManager_ == null); + assert(!s5bServerManager_.isInitialized()); onInitializedConnection_ = s5bServerManager_.onInitialized.connect(new Slot1() { @Override public void call(Boolean b) { @@ -36,7 +36,7 @@ public class SOCKS5BytestreamServerResourceUser { * User should call delete to free the resources. */ public void delete() { - if (s5bServerManager_ != null) { + if (s5bServerManager_.isInitialized()) { s5bServerManager_.stop(); } } -- cgit v0.10.2-6-g49f6