Resolve the address before binding.

components/net/sf/briar/plugins/file/FilePlugin.java

@@ -79,7 +79,7 @@ abstract class FilePlugin implements BatchTransportPlugin {
 	}
 
 	public BatchTransportWriter createWriter(ContactId c) {
-		if(!started) throw new IllegalStateException();
+		if(!started) return null;
 		File dir = chooseOutputDirectory();
 		if(dir == null || !dir.exists() || !dir.isDirectory()) return null;
 		File f = new File(dir, createFilename());

components/net/sf/briar/plugins/file/RemovableDrivePlugin.java

@@ -69,7 +69,7 @@ implements RemovableDriveMonitor.Callback {
 			for(int i = 0; i < paths.length; i++) {
 				paths[i] = drives.get(i).getPath();
 			}
-			int i = callback.showChoice("REMOVABLE_DRIVE_CHOOSE_DRIVE", paths);
+			int i = callback.showChoice(paths, "REMOVABLE_DRIVE_CHOOSE_DRIVE");
 			if(i == -1) return null;
 			return drives.get(i);
 		} catch(IOException e) {

components/net/sf/briar/plugins/socket/SimpleSocketPlugin.java

@@ -1,6 +1,8 @@
 package net.sf.briar.plugins.socket;
 
+import java.io.IOException;
 import java.net.InetSocketAddress;
+import java.net.ServerSocket;
 import java.net.Socket;
 import java.net.SocketAddress;
 import java.util.Map;
@@ -36,17 +38,34 @@ public class SimpleSocketPlugin extends SocketPlugin {
 
 	@Override
 	protected SocketAddress getLocalSocketAddress() {
+		assert localProperties != null;
 		return createSocketAddress(localProperties);
 	}
 
 	@Override
 	protected SocketAddress getSocketAddress(ContactId c) {
+		assert remoteProperties != null;
 		Map<String, String> properties = remoteProperties.get(c);
 		if(properties == null) return null;
 		return createSocketAddress(properties);
 	}
 
+	@Override
+	protected void setLocalSocketAddress(SocketAddress s) {
+		assert localProperties != null;
+		if(!(s instanceof InetSocketAddress))
+			throw new IllegalArgumentException();
+		InetSocketAddress i = (InetSocketAddress) s;
+		String host = i.getAddress().getHostAddress();
+		String port = String.valueOf(i.getPort());
+		// FIXME: Special handling for private IP addresses?
+		localProperties.put("host", host);
+		localProperties.put("port", port);
+		callback.setLocalProperties(localProperties);
+	}
+
 	private SocketAddress createSocketAddress(Map<String, String> properties) {
+		assert properties != null;
 		String host = properties.get("host");
 		String portString = properties.get("port");
 		if(host == null || portString == null) return null;
@@ -56,16 +75,16 @@ public class SimpleSocketPlugin extends SocketPlugin {
 		} catch(NumberFormatException e) {
 			return null;
 		}
-		return InetSocketAddress.createUnresolved(host, port);
+		return new InetSocketAddress(host, port);
 	}
 
 	@Override
-	protected Socket createClientSocket() {
+	protected Socket createClientSocket() throws IOException {
 		return new Socket();
 	}
 
 	@Override
-	protected Socket createServerSocket() {
+	protected ServerSocket createServerSocket() throws IOException {
-		return new Socket();
+		return new ServerSocket();
 	}
 }

components/net/sf/briar/plugins/socket/SocketPlugin.java

@@ -1,6 +1,7 @@
 package net.sf.briar.plugins.socket;
 
 import java.io.IOException;
+import java.net.ServerSocket;
 import java.net.Socket;
 import java.net.SocketAddress;
 import java.util.Map;
@@ -21,14 +22,16 @@ abstract class SocketPlugin implements StreamTransportPlugin {
 	protected Map<ContactId, Map<String, String>> remoteProperties = null;
 	protected Map<String, String> config = null;
 	protected StreamTransportCallback callback = null;
+	protected ServerSocket socket = null;
 
 	private volatile boolean started = false;
 
 	// These methods should be called with this's lock held and started == true
 	protected abstract SocketAddress getLocalSocketAddress();
 	protected abstract SocketAddress getSocketAddress(ContactId c);
-	protected abstract Socket createClientSocket();
+	protected abstract void setLocalSocketAddress(SocketAddress s);
-	protected abstract Socket createServerSocket();
+	protected abstract Socket createClientSocket() throws IOException;
+	protected abstract ServerSocket createServerSocket() throws IOException;
 
 	SocketPlugin(Executor executor) {
 		this.executor = executor;
@@ -51,25 +54,32 @@ abstract class SocketPlugin implements StreamTransportPlugin {
 		return new Runnable() {
 			public void run() {
 				SocketAddress addr;
-				Socket s;
+				ServerSocket s;
-				synchronized(SocketPlugin.this) {
-					if(!started) return;
-					addr = getLocalSocketAddress();
-					s = createServerSocket();
-				}
-				if(addr == null || s == null) return;
 				try {
+					synchronized(SocketPlugin.this) {
+						if(!started) return;
+						addr = getLocalSocketAddress();
+						s = createServerSocket();
+					}
+					if(addr == null || s == null) return;
 					s.bind(addr);
 				} catch(IOException e) {
+					// FIXME: Logging
 					return;
 				}
+				synchronized(SocketPlugin.this) {
+					if(!started) return;
+					socket = s;
+					setLocalSocketAddress(s.getLocalSocketAddress());
+				}
 			}
 		};
 	}
 
-	public synchronized void stop() {
+	public synchronized void stop() throws IOException {
 		if(!started) throw new IllegalStateException();
 		started = false;
+		if(socket != null) socket.close();
 	}
 
 	public synchronized void setLocalProperties(Map<String, String> properties)
@@ -103,7 +113,11 @@ abstract class SocketPlugin implements StreamTransportPlugin {
 		return new Runnable() {
 			public void run() {
 				StreamTransportConnection conn = createAndConnectSocket(c);
-				if(conn != null) callback.outgoingConnectionCreated(c, conn);
+				if(conn != null) {
+					synchronized(SocketPlugin.this) {
+						if(started) callback.outgoingConnectionCreated(c, conn);
+					}
+				}
 			}
 		};
 	}
@@ -111,13 +125,13 @@ abstract class SocketPlugin implements StreamTransportPlugin {
 	protected StreamTransportConnection createAndConnectSocket(ContactId c) {
 		SocketAddress addr;
 		Socket s;
-		synchronized(this) {
-			if(!started) return null;
-			addr = getSocketAddress(c);
-			s = createClientSocket();
-		}
-		if(addr == null || s == null) return null;
 		try {
+			synchronized(this) {
+				if(!started) return null;
+				addr = getSocketAddress(c);
+				s = createClientSocket();
+			}
+			if(addr == null || s == null) return null;
 			s.connect(addr);
 		} catch(IOException e) {
 			return null;
@@ -126,7 +140,6 @@ abstract class SocketPlugin implements StreamTransportPlugin {
 	}
 
 	public StreamTransportConnection createConnection(ContactId c) {
-		if(!started) throw new IllegalStateException();
+		return started ? createAndConnectSocket(c) : null;
-		return createAndConnectSocket(c);
 	}
 }

test/build.xml

@@ -31,6 +31,7 @@
 			<test name='net.sf.briar.plugins.file.PollingRemovableDriveMonitorTest'/>
 			<test name='net.sf.briar.plugins.file.RemovableDrivePluginTest'/>
 			<test name='net.sf.briar.plugins.file.UnixRemovableDriveMonitorTest'/>
+			<test name='net.sf.briar.plugins.socket.SimpleSocketPluginTest'/>
 			<test name='net.sf.briar.protocol.AckReaderTest'/>
 			<test name='net.sf.briar.protocol.BatchReaderTest'/>
 			<test name='net.sf.briar.protocol.ConsumersTest'/>

test/net/sf/briar/plugins/socket/SimpleSocketPluginTest.java

@@ -0,0 +1,86 @@
+package net.sf.briar.plugins.socket;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.concurrent.Executor;
+
+import junit.framework.TestCase;
+import net.sf.briar.api.ContactId;
+import net.sf.briar.api.transport.stream.StreamTransportCallback;
+import net.sf.briar.api.transport.stream.StreamTransportConnection;
+
+import org.junit.Before;
+import org.junit.Test;
+
+public class SimpleSocketPluginTest extends TestCase {
+
+	private final ContactId contactId = new ContactId(0);
+
+	private Map<String, String> localProperties = null;
+	private Map<ContactId, Map<String, String>> remoteProperties = null;
+	private Map<String, String> config = null;
+
+	@Before
+	public void setUp() {
+		localProperties = new TreeMap<String, String>();
+		remoteProperties = new HashMap<ContactId, Map<String, String>>();
+		remoteProperties.put(contactId, new TreeMap<String, String>());
+		config = new TreeMap<String, String>();
+	}
+
+	@Test
+	public void testBind() throws Exception {
+		StubCallback callback = new StubCallback();
+		localProperties.put("host", "127.0.0.1");
+		localProperties.put("port", "0");
+		SimpleSocketPlugin plugin =
+			new SimpleSocketPlugin(new ImmediateExecutor(), 10);
+		plugin.start(localProperties, remoteProperties, config, callback);
+		assertNotNull(callback.localProperties);
+		String host = callback.localProperties.get("host");
+		assertNotNull(host);
+		assertEquals("127.0.0.1", host);
+		String port = callback.localProperties.get("port");
+		assertNotNull(port);
+		assertTrue(Integer.valueOf(port) > 0 && Integer.valueOf(port) < 65536);
+		plugin.stop();
+	}
+
+	private static class ImmediateExecutor implements Executor {
+
+		public void execute(Runnable r) {
+			r.run();
+		}
+	}
+
+	private static class StubCallback implements StreamTransportCallback {
+
+		private Map<String, String> localProperties = null;
+
+		public void setLocalProperties(Map<String, String> properties) {
+			localProperties = properties;
+		}
+
+		public void setConfig(Map<String, String> config) {
+		}
+
+		public void showMessage(String... message) {
+		}
+
+		public boolean showConfirmationMessage(String... message) {
+			return false;
+		}
+
+		public int showChoice(String[] choices, String... message) {
+			return -1;
+		}
+
+		public void incomingConnectionCreated(StreamTransportConnection c) {
+		}
+
+		public void outgoingConnectionCreated(ContactId contactId,
+				StreamTransportConnection c) {
+		}
+	}
+}