diff --git a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java index 989c80d47..aedb15f67 100644 --- a/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java +++ b/bramble-android/src/main/java/org/briarproject/bramble/plugin/bluetooth/AndroidBluetoothTransportConnection.java @@ -10,6 +10,8 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import static org.briarproject.bramble.api.plugin.BluetoothConstants.PROP_ADDRESS; + @NotNullByDefault class AndroidBluetoothTransportConnection extends AbstractDuplexTransportConnection { @@ -23,6 +25,7 @@ class AndroidBluetoothTransportConnection super(plugin); this.connectionManager = connectionManager; this.socket = socket; + remote.put(PROP_ADDRESS, socket.getRemoteDevice().getAddress()); } @Override diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/AbstractDuplexTransportConnection.java b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/AbstractDuplexTransportConnection.java index ba84134e9..27aad596a 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/AbstractDuplexTransportConnection.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/AbstractDuplexTransportConnection.java @@ -4,6 +4,7 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.Plugin; import org.briarproject.bramble.api.plugin.TransportConnectionReader; import org.briarproject.bramble.api.plugin.TransportConnectionWriter; +import org.briarproject.bramble.api.properties.TransportProperties; import java.io.IOException; import java.io.InputStream; @@ -14,6 +15,8 @@ import java.util.concurrent.atomic.AtomicBoolean; public abstract class AbstractDuplexTransportConnection implements DuplexTransportConnection { + protected final TransportProperties remote = new TransportProperties(); + private final Plugin plugin; private final Reader reader; private final Writer writer; @@ -44,6 +47,11 @@ public abstract class AbstractDuplexTransportConnection return writer; } + @Override + public TransportProperties getRemoteProperties() { + return remote; + } + private class Reader implements TransportConnectionReader { @Override diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/DuplexTransportConnection.java b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/DuplexTransportConnection.java index f235e240b..d0e98bfba 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/DuplexTransportConnection.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/plugin/duplex/DuplexTransportConnection.java @@ -3,6 +3,7 @@ package org.briarproject.bramble.api.plugin.duplex; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.TransportConnectionReader; import org.briarproject.bramble.api.plugin.TransportConnectionWriter; +import org.briarproject.bramble.api.properties.TransportProperties; /** * An interface for reading and writing data over a duplex transport. The @@ -23,4 +24,10 @@ public interface DuplexTransportConnection { * for writing to the connection. */ TransportConnectionWriter getWriter(); + + /** + * Returns a possibly empty set of {@link TransportProperties} describing + * the remote peer. + */ + TransportProperties getRemoteProperties(); } diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyManager.java b/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyManager.java index a634bfaa7..de10f2aca 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyManager.java +++ b/bramble-api/src/main/java/org/briarproject/bramble/api/properties/TransportPropertyManager.java @@ -34,6 +34,14 @@ public interface TransportPropertyManager { void addRemoteProperties(Transaction txn, ContactId c, Map props) throws DbException; + /** + * Stores the given properties discovered from an incoming transport + * connection. They will be overridden by any properties received while + * adding the contact or synced from the contact. + */ + void addRemotePropertiesFromConnection(ContactId c, TransportId t, + TransportProperties props) throws DbException; + /** * Returns the local transport properties for all transports. */ diff --git a/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java index d3fec490b..979e8ea01 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/plugin/ConnectionManagerImpl.java @@ -15,6 +15,8 @@ import org.briarproject.bramble.api.plugin.TransportConnectionReader; import org.briarproject.bramble.api.plugin.TransportConnectionWriter; import org.briarproject.bramble.api.plugin.TransportId; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import org.briarproject.bramble.api.properties.TransportProperties; +import org.briarproject.bramble.api.properties.TransportPropertyManager; import org.briarproject.bramble.api.sync.SyncSession; import org.briarproject.bramble.api.sync.SyncSessionFactory; import org.briarproject.bramble.api.transport.KeyManager; @@ -52,6 +54,7 @@ class ConnectionManagerImpl implements ConnectionManager { private final HandshakeManager handshakeManager; private final ContactExchangeManager contactExchangeManager; private final ConnectionRegistry connectionRegistry; + private final TransportPropertyManager transportPropertyManager; @Inject ConnectionManagerImpl(@IoExecutor Executor ioExecutor, @@ -60,7 +63,8 @@ class ConnectionManagerImpl implements ConnectionManager { SyncSessionFactory syncSessionFactory, HandshakeManager handshakeManager, ContactExchangeManager contactExchangeManager, - ConnectionRegistry connectionRegistry) { + ConnectionRegistry connectionRegistry, + TransportPropertyManager transportPropertyManager) { this.ioExecutor = ioExecutor; this.keyManager = keyManager; this.streamReaderFactory = streamReaderFactory; @@ -69,6 +73,7 @@ class ConnectionManagerImpl implements ConnectionManager { this.handshakeManager = handshakeManager; this.contactExchangeManager = contactExchangeManager; this.connectionRegistry = connectionRegistry; + this.transportPropertyManager = transportPropertyManager; } @Override @@ -269,6 +274,7 @@ class ConnectionManagerImpl implements ConnectionManager { private final TransportId transportId; private final TransportConnectionReader reader; private final TransportConnectionWriter writer; + private final TransportProperties remote; @Nullable private volatile SyncSession outgoingSession = null; @@ -278,6 +284,7 @@ class ConnectionManagerImpl implements ConnectionManager { this.transportId = transportId; reader = connection.getReader(); writer = connection.getWriter(); + remote = connection.getRemoteProperties(); } @Override @@ -313,13 +320,16 @@ class ConnectionManagerImpl implements ConnectionManager { // Start the outgoing session on another thread ioExecutor.execute(() -> runOutgoingSession(contactId)); try { + // Store any transport properties discovered from the connection + transportPropertyManager.addRemotePropertiesFromConnection( + contactId, transportId, remote); // Create and run the incoming session createIncomingSession(ctx, reader).run(); reader.dispose(false, true); // Interrupt the outgoing session so it finishes cleanly SyncSession out = outgoingSession; if (out != null) out.interrupt(); - } catch (IOException e) { + } catch (DbException | IOException e) { logException(LOG, WARNING, e); onReadError(true); } finally { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java index 9e79bfd66..c12b5ddf6 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java @@ -140,6 +140,27 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, } } + @Override + public void addRemotePropertiesFromConnection(ContactId c, TransportId t, + TransportProperties props) throws DbException { + if (props.isEmpty()) return; + try { + db.transaction(false, txn -> { + Group g = getContactGroup(db.getContact(txn, c)); + BdfDictionary meta = clientHelper.getGroupMetadataAsDictionary( + txn, g.getId()); + BdfDictionary discovered = + meta.getOptionalDictionary("discovered"); + if (discovered == null) discovered = new BdfDictionary(); + discovered.putAll(props); + meta.put("discovered", discovered); + clientHelper.mergeGroupMetadata(txn, g.getId(), meta); + }); + } catch (FormatException e) { + throw new DbException(e); + } + } + @Override public Map getLocalProperties() throws DbException { @@ -203,12 +224,26 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, Group g = getContactGroup(c); try { // Find the latest remote update + TransportProperties remote; LatestUpdate latest = findLatest(txn, g.getId(), t, false); - if (latest == null) return new TransportProperties(); - // Retrieve and parse the latest remote properties - BdfList message = - clientHelper.getMessageAsList(txn, latest.messageId); - return parseProperties(message); + if (latest == null) { + remote = new TransportProperties(); + } else { + // Retrieve and parse the latest remote properties + BdfList message = + clientHelper.getMessageAsList(txn, latest.messageId); + remote = parseProperties(message); + } + // Merge in any discovered properties + BdfDictionary meta = + clientHelper.getGroupMetadataAsDictionary(txn, g.getId()); + BdfDictionary d = meta.getOptionalDictionary("discovered"); + if (d == null) return remote; + TransportProperties merged = + clientHelper.parseAndValidateTransportProperties(d); + // Received properties override discovered properties + merged.putAll(remote); + return merged; } catch (FormatException e) { throw new DbException(e); } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java index 007d264b7..e042a255c 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java @@ -24,12 +24,12 @@ import org.briarproject.bramble.test.DbExpectations; import org.jmock.Expectations; import org.junit.Test; -import java.util.Collections; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; import static org.briarproject.bramble.api.properties.TransportPropertyManager.CLIENT_ID; @@ -405,25 +405,25 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { List contacts = asList(contact1, contact2); Group contactGroup1 = getGroup(CLIENT_ID, MAJOR_VERSION); Group contactGroup2 = getGroup(CLIENT_ID, MAJOR_VERSION); - Map messageMetadata2 = + Map messageMetadata = new LinkedHashMap<>(); // A remote update for another transport should be ignored MessageId barUpdateId = new MessageId(getRandomId()); - messageMetadata2.put(barUpdateId, BdfDictionary.of( + messageMetadata.put(barUpdateId, BdfDictionary.of( new BdfEntry("transportId", "bar"), new BdfEntry("version", 1), new BdfEntry("local", false) )); // A local update for the right transport should be ignored MessageId localUpdateId = new MessageId(getRandomId()); - messageMetadata2.put(localUpdateId, BdfDictionary.of( + messageMetadata.put(localUpdateId, BdfDictionary.of( new BdfEntry("transportId", "foo"), new BdfEntry("version", 1), new BdfEntry("local", true) )); // A remote update for the right transport should be returned MessageId fooUpdateId = new MessageId(getRandomId()); - messageMetadata2.put(fooUpdateId, BdfDictionary.of( + messageMetadata.put(fooUpdateId, BdfDictionary.of( new BdfEntry("transportId", "foo"), new BdfEntry("version", 1), new BdfEntry("local", false) @@ -440,19 +440,25 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { will(returnValue(contactGroup1)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup1.getId()); - will(returnValue(Collections.emptyMap())); + will(returnValue(emptyMap())); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + contactGroup1.getId()); + will(returnValue(new BdfDictionary())); // Second contact: returns an update oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, MAJOR_VERSION, contact2); will(returnValue(contactGroup2)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup2.getId()); - will(returnValue(messageMetadata2)); + will(returnValue(messageMetadata)); oneOf(clientHelper).getMessageAsList(txn, fooUpdateId); will(returnValue(fooUpdate)); oneOf(clientHelper).parseAndValidateTransportProperties( fooPropertiesDict); will(returnValue(fooProperties)); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + contactGroup2.getId()); + will(returnValue(new BdfDictionary())); }}); TransportPropertyManagerImpl t = createInstance(); @@ -463,6 +469,61 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { assertEquals(fooProperties, properties.get(contact2.getId())); } + @Test + public void testReceivePropertiesOverrideDiscoveredProperties() + throws Exception { + Transaction txn = new Transaction(null, true); + Contact contact = getContact(); + List contacts = singletonList(contact); + Group contactGroup = getGroup(CLIENT_ID, MAJOR_VERSION); + MessageId updateId = new MessageId(getRandomId()); + Map messageMetadata = singletonMap(updateId, + BdfDictionary.of( + new BdfEntry("transportId", "foo"), + new BdfEntry("version", 1), + new BdfEntry("local", false) + )); + BdfList update = BdfList.of("foo", 1, fooPropertiesDict); + TransportProperties discovered = new TransportProperties(); + discovered.put("fooKey1", "overridden"); + discovered.put("fooKey3", "fooValue3"); + BdfDictionary discoveredDict = new BdfDictionary(discovered); + BdfDictionary groupMeta = + BdfDictionary.of(new BdfEntry("discovered", discoveredDict)); + TransportProperties merged = new TransportProperties(); + merged.putAll(fooProperties); + merged.put("fooKey3", "fooValue3"); + + context.checking(new DbExpectations() {{ + oneOf(db).transactionWithResult(with(true), withDbCallable(txn)); + oneOf(db).getContacts(txn); + will(returnValue(contacts)); + // One update + oneOf(contactGroupFactory).createContactGroup(CLIENT_ID, + MAJOR_VERSION, contact); + will(returnValue(contactGroup)); + oneOf(clientHelper).getMessageMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(messageMetadata)); + oneOf(clientHelper).getMessageAsList(txn, updateId); + will(returnValue(update)); + oneOf(clientHelper).parseAndValidateTransportProperties( + fooPropertiesDict); + will(returnValue(fooProperties)); + oneOf(clientHelper).getGroupMetadataAsDictionary(txn, + contactGroup.getId()); + will(returnValue(groupMeta)); + oneOf(clientHelper).parseAndValidateTransportProperties( + discoveredDict); + will(returnValue(discovered)); + }}); + + TransportPropertyManagerImpl t = createInstance(); + Map properties = + t.getRemoteProperties(new TransportId("foo")); + assertEquals(merged, properties.get(contact.getId())); + } + @Test public void testMergingUnchangedPropertiesDoesNotCreateUpdate() throws Exception { @@ -505,7 +566,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { // There are no existing properties to merge with oneOf(clientHelper).getMessageMetadataAsDictionary(txn, localGroup.getId()); - will(returnValue(Collections.emptyMap())); + will(returnValue(emptyMap())); // Store the new properties in the local group, version 1 expectStoreMessage(txn, localGroup.getId(), "foo", fooPropertiesDict, 1, true, false); @@ -517,7 +578,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase { will(returnValue(contactGroup)); oneOf(clientHelper).getMessageMetadataAsDictionary(txn, contactGroup.getId()); - will(returnValue(Collections.emptyMap())); + will(returnValue(emptyMap())); expectStoreMessage(txn, contactGroup.getId(), "foo", fooPropertiesDict, 1, true, true); }}); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/test/TestDuplexTransportConnection.java b/bramble-core/src/test/java/org/briarproject/bramble/test/TestDuplexTransportConnection.java index 007ec5f9d..0ed6f9045 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/test/TestDuplexTransportConnection.java +++ b/bramble-core/src/test/java/org/briarproject/bramble/test/TestDuplexTransportConnection.java @@ -4,6 +4,7 @@ import org.briarproject.bramble.api.nullsafety.NotNullByDefault; import org.briarproject.bramble.api.plugin.TransportConnectionReader; import org.briarproject.bramble.api.plugin.TransportConnectionWriter; import org.briarproject.bramble.api.plugin.duplex.DuplexTransportConnection; +import org.briarproject.bramble.api.properties.TransportProperties; import java.io.IOException; import java.io.InputStream; @@ -37,6 +38,11 @@ public class TestDuplexTransportConnection return writer; } + @Override + public TransportProperties getRemoteProperties() { + return new TransportProperties(); + } + /** * Creates and returns a pair of TestDuplexTransportConnections that are * connected to each other.