From d10ff92d44ddc3d44fef53676c5e765327e3621a Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Mon, 4 Aug 2014 18:16:56 +0200 Subject: [PATCH] Use createPacketCollectorAndSend in FileTransferNegoiator also fixes a potential PacketCollector leak. --- .../smackx/filetransfer/FileTransferNegotiator.java | 10 +++++----- .../smackx/filetransfer/OutgoingFileTransfer.java | 6 ------ .../filetransfer/FileTransferNegotiatorTest.java | 7 ++++++- 3 files changed, 11 insertions(+), 12 deletions(-) diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java index 719a74896..d5b0e8f50 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiator.java @@ -27,9 +27,9 @@ import java.util.Random; import java.util.WeakHashMap; import org.jivesoftware.smack.Manager; +import org.jivesoftware.smack.SmackException.NoResponseException; import org.jivesoftware.smack.SmackException.NotConnectedException; import org.jivesoftware.smack.XMPPConnection; -import org.jivesoftware.smack.PacketCollector; import org.jivesoftware.smack.XMPPException.XMPPErrorException; import org.jivesoftware.smack.packet.IQ; import org.jivesoftware.smack.packet.Packet; @@ -298,10 +298,11 @@ public class FileTransferNegotiator extends Manager { * @return Returns the stream negotiator selected by the peer. * @throws XMPPErrorException Thrown if there is an error negotiating the file transfer. * @throws NotConnectedException + * @throws NoResponseException */ public StreamNegotiator negotiateOutgoingTransfer(final String userID, final String streamID, final String fileName, final long size, - final String desc, int responseTimeout) throws XMPPErrorException, NotConnectedException { + final String desc, int responseTimeout) throws XMPPErrorException, NotConnectedException, NoResponseException { StreamInitiation si = new StreamInitiation(); si.setSessionID(streamID); si.setMimeType(URLConnection.guessContentTypeFromName(fileName)); @@ -316,9 +317,8 @@ public class FileTransferNegotiator extends Manager { si.setTo(userID); si.setType(IQ.Type.set); - PacketCollector collector = connection().createPacketCollectorAndSend(si); - Packet siResponse = collector.nextResult(responseTimeout); - collector.cancel(); + Packet siResponse = connection().createPacketCollectorAndSend(si).nextResultOrThrow( + responseTimeout); if (siResponse instanceof IQ) { IQ iqResponse = (IQ) siResponse; diff --git a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/OutgoingFileTransfer.java b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/OutgoingFileTransfer.java index aeab5810a..2202d0d4b 100644 --- a/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/OutgoingFileTransfer.java +++ b/smack-extensions/src/main/java/org/jivesoftware/smackx/filetransfer/OutgoingFileTransfer.java @@ -373,12 +373,6 @@ public class OutgoingFileTransfer extends FileTransfer { getPeer(), streamID, fileName, fileSize, description, RESPONSE_TIMEOUT); - if (streamNegotiator == null) { - setStatus(Status.error); - setError(Error.no_response); - return null; - } - // Negotiate the stream if (!updateStatus(Status.negotiating_transfer, Status.negotiating_stream)) { throw new IllegalStateChangeException(); diff --git a/smack-extensions/src/test/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiatorTest.java b/smack-extensions/src/test/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiatorTest.java index ac4bad49f..761e41408 100644 --- a/smack-extensions/src/test/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiatorTest.java +++ b/smack-extensions/src/test/java/org/jivesoftware/smackx/filetransfer/FileTransferNegotiatorTest.java @@ -19,6 +19,7 @@ package org.jivesoftware.smackx.filetransfer; import static org.junit.Assert.assertTrue; import org.jivesoftware.smack.DummyConnection; +import org.jivesoftware.smack.SmackException.NoResponseException; import org.jivesoftware.smack.packet.Packet; import org.jivesoftware.smackx.disco.ServiceDiscoveryManager; import org.junit.After; @@ -48,7 +49,11 @@ public class FileTransferNegotiatorTest { @Test public void verifyForm() throws Exception { FileTransferNegotiator fileNeg = FileTransferNegotiator.getInstanceFor(connection); - fileNeg.negotiateOutgoingTransfer("me", "streamid", "file", 1024, null, 10); + try { + fileNeg.negotiateOutgoingTransfer("me", "streamid", "file", 1024, null, 10); + } catch (NoResponseException e) { + // Ignore + } Packet packet = connection.getSentPacket(); String xml = packet.toXML().toString(); assertTrue(xml.indexOf("var='stream-method' type='list-single'") != -1);