Kaladaran/briar
0
0
Fork 0
mirror of https://code.briarproject.org/briar/briar.git synced 2026-02-12 10:49:06 +01:00
Code Issues Projects Releases Wiki Activity

Don't interrupt connections until priority is set.

Browse Source 
This maintains compatibility with older peers that don't know about
priorities or transport preferences and will try to replace any
connections we close.
This commit is contained in:
akwizgran
2020-06-01 14:23:08 +01:00
parent 4aaa8c3b93
commit d3751fbead
2 changed files with 107 additions and 52 deletions
Download Patch File Download Diff File Expand all files Collapse all files

61
bramble-core/src/main/java/org/briarproject/bramble/connection/ConnectionRegistryImpl.java
View File

@@ -83,41 +83,16 @@ class ConnectionRegistryImpl implements ConnectionRegistry {
if (incoming) LOG.info("Incoming connection registered: " + t);
else LOG.info("Outgoing connection registered: " + t);
}
List<InterruptibleConnection> toInterrupt;
boolean firstConnection = false, interruptNewConnection = false;
boolean firstConnection;
synchronized (lock) {
List<ConnectionRecord> recs = contactConnections.get(c);
if (recs == null) {
recs = new ArrayList<>();
contactConnections.put(c, recs);
}
if (recs.isEmpty()) {
toInterrupt = emptyList();
firstConnection = true;
} else {
toInterrupt = new ArrayList<>(recs.size());
for (ConnectionRecord rec : recs) {
int compare = compare(t, rec.transportId);
if (compare == -1) {
// The old connection is better than the new one
interruptNewConnection = true;
} else if (compare == 1 && !rec.interrupted) {
// The new connection is better than the old one
toInterrupt.add(rec.conn);
rec.interrupted = true;
}
}
}
firstConnection = recs.isEmpty();
recs.add(new ConnectionRecord(t, conn));
}
if (interruptNewConnection) {
LOG.info("Interrupting new connection");
conn.interruptOutgoingSession();
}
for (InterruptibleConnection old : toInterrupt) {
LOG.info("Interrupting old connection");
old.interruptOutgoingSession();
}
eventBus.broadcast(new ConnectionOpenedEvent(c, t, incoming));
if (firstConnection) {
LOG.info("Contact connected");
@@ -125,17 +100,6 @@ class ConnectionRegistryImpl implements ConnectionRegistry {
}
}
private int compare(TransportId a, TransportId b) {
if (getBetterTransports(a).contains(b)) return -1;
else if (getBetterTransports(b).contains(a)) return 1;
else return 0;
}
private List<TransportId> getBetterTransports(TransportId t) {
List<TransportId> better = betterTransports.get(t);
return better == null ? emptyList() : better;
}
@Override
public void setPriority(ContactId c, TransportId t,
InterruptibleConnection conn, Priority priority) {
@@ -150,8 +114,9 @@ class ConnectionRegistryImpl implements ConnectionRegistry {
if (rec.conn == conn) {
// Store the priority of this connection
rec.priority = priority;
} else if (rec.transportId.equals(t)) {
int compare = compare(priority, rec.priority);
} else if (rec.priority != null) {
int compare = compareConnections(t, priority,
rec.transportId, rec.priority);
if (compare == -1) {
// The old connection is better than the new one
interruptNewConnection = true;
@@ -173,8 +138,16 @@ class ConnectionRegistryImpl implements ConnectionRegistry {
}
}
private int compare(Priority a, @Nullable Priority b) {
return b == null ? 0 : Bytes.compare(a.getNonce(), b.getNonce());
private int compareConnections(TransportId tA, Priority pA, TransportId tB,
Priority pB) {
if (getBetterTransports(tA).contains(tB)) return -1;
if (getBetterTransports(tB).contains(tA)) return 1;
return tA.equals(tB) ? Bytes.compare(pA.getNonce(), pB.getNonce()) : 0;
}
private List<TransportId> getBetterTransports(TransportId t) {
List<TransportId> better = betterTransports.get(t);
return better == null ? emptyList() : better;
}
@Override
@@ -184,12 +157,12 @@ class ConnectionRegistryImpl implements ConnectionRegistry {
if (incoming) LOG.info("Incoming connection unregistered: " + t);
else LOG.info("Outgoing connection unregistered: " + t);
}
boolean lastConnection = false;
boolean lastConnection;
synchronized (lock) {
List<ConnectionRecord> recs = contactConnections.get(c);
if (recs == null || !recs.remove(new ConnectionRecord(t, conn)))
throw new IllegalArgumentException();
if (recs.isEmpty()) lastConnection = true;
lastConnection = recs.isEmpty();
}
eventBus.broadcast(new ConnectionClosedEvent(c, t, incoming));
if (lastConnection) {

98
bramble-core/src/test/java/org/briarproject/bramble/connection/ConnectionRegistryImplTest.java
View File

@@ -177,6 +177,79 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase {
c.getConnectedOrBetterContacts(transportId2));
}
@Test
public void testConnectionsAreNotInterruptedUnlessPriorityIsSet() {
// Prefer transport 2 to transport 1
context.checking(new Expectations() {{
allowing(pluginConfig).getTransportPreferences();
will(returnValue(
singletonList(new Pair<>(transportId2, transportId1))));
}});
ConnectionRegistry c =
new ConnectionRegistryImpl(eventBus, pluginConfig);
// Connect via transport 1 (worse than 2) and set priority to low
context.checking(new Expectations() {{
oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class)));
oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class)));
}});
c.registerConnection(contactId1, transportId1, conn1, true);
context.assertIsSatisfied();
assertEquals(singletonList(contactId1),
c.getConnectedContacts(transportId1));
assertEquals(singletonList(contactId1),
c.getConnectedOrBetterContacts(transportId1));
assertEquals(emptyList(), c.getConnectedContacts(transportId2));
assertEquals(emptyList(), c.getConnectedOrBetterContacts(transportId2));
// Connect via transport 2 (better than 1) and set priority to high -
// the old connection should not be interrupted, despite using a worse
// transport, to remain compatible with old peers
context.checking(new Expectations() {{
oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class)));
}});
c.registerConnection(contactId1, transportId2, conn2, true);
c.setPriority(contactId1, transportId2, conn2, high);
context.assertIsSatisfied();
assertEquals(singletonList(contactId1),
c.getConnectedContacts(transportId1));
assertEquals(singletonList(contactId1),
c.getConnectedOrBetterContacts(transportId1));
assertEquals(singletonList(contactId1),
c.getConnectedContacts(transportId2));
assertEquals(singletonList(contactId1),
c.getConnectedOrBetterContacts(transportId2));
// Connect via transport 3 (no preference) and set priority to high -
// again, no interruptions are expected
context.checking(new Expectations() {{
oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class)));
}});
c.registerConnection(contactId1, transportId3, conn3, true);
c.setPriority(contactId1, transportId3, conn3, high);
context.assertIsSatisfied();
assertEquals(singletonList(contactId1),
c.getConnectedContacts(transportId1));
assertEquals(singletonList(contactId1),
c.getConnectedOrBetterContacts(transportId1));
assertEquals(singletonList(contactId1),
c.getConnectedContacts(transportId2));
assertEquals(singletonList(contactId1),
c.getConnectedOrBetterContacts(transportId2));
assertEquals(singletonList(contactId1),
c.getConnectedContacts(transportId3));
assertEquals(singletonList(contactId1),
c.getConnectedOrBetterContacts(transportId3));
}
@Test
public void testNewConnectionIsInterruptedIfOldConnectionUsesBetterTransport() {
// Prefer transport 1 to transport 2
@@ -189,12 +262,13 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase {
ConnectionRegistry c =
new ConnectionRegistryImpl(eventBus, pluginConfig);
// Connect via transport 1 (better than 2)
// Connect via transport 1 (better than 2) and set priority to low
context.checking(new Expectations() {{
oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class)));
oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class)));
}});
c.registerConnection(contactId1, transportId1, conn1, true);
c.setPriority(contactId1, transportId1, conn1, low);
context.assertIsSatisfied();
assertEquals(singletonList(contactId1),
@@ -208,13 +282,15 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase {
assertEquals(singletonList(contactId1),
c.getConnectedOrBetterContacts(transportId2));
// Connect via transport 2 (worse than 1) - the new connection should
// be interrupted
// Connect via transport 2 (worse than 1) and set priority to high -
// the new connection should be interrupted because it uses a worse
// transport
context.checking(new Expectations() {{
oneOf(conn2).interruptOutgoingSession();
oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class)));
}});
c.registerConnection(contactId1, transportId2, conn2, true);
c.setPriority(contactId1, transportId2, conn2, high);
context.assertIsSatisfied();
assertEquals(singletonList(contactId1),
@@ -227,11 +303,13 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase {
assertEquals(singletonList(contactId1),
c.getConnectedOrBetterContacts(transportId2));
// Connect via transport 3 (no preference)
// Connect via transport 3 (no preference) and set priority to low -
// no further interruptions
context.checking(new Expectations() {{
oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class)));
}});
c.registerConnection(contactId1, transportId3, conn3, true);
c.setPriority(contactId1, transportId3, conn3, low);
context.assertIsSatisfied();
assertEquals(singletonList(contactId1),
@@ -285,12 +363,13 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase {
ConnectionRegistry c =
new ConnectionRegistryImpl(eventBus, pluginConfig);
// Connect via transport 1 (worse than 2)
// Connect via transport 1 (worse than 2) and set priority to high
context.checking(new Expectations() {{
oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class)));
oneOf(eventBus).broadcast(with(any(ContactConnectedEvent.class)));
}});
c.registerConnection(contactId1, transportId1, conn1, true);
c.setPriority(contactId1, transportId1, conn1, high);
context.assertIsSatisfied();
assertEquals(singletonList(contactId1),
@@ -301,13 +380,15 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase {
assertEquals(emptyList(), c.getConnectedContacts(transportId2));
assertEquals(emptyList(), c.getConnectedOrBetterContacts(transportId2));
// Connect via transport 2 (better than 1) - the old connection should
// be interrupted
// Connect via transport 2 (better than 1) and set priority to low -
// the old connection should be interrupted because it uses a worse
// transport
context.checking(new Expectations() {{
oneOf(conn1).interruptOutgoingSession();
oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class)));
}});
c.registerConnection(contactId1, transportId2, conn2, true);
c.setPriority(contactId1, transportId2, conn2, low);
context.assertIsSatisfied();
assertEquals(singletonList(contactId1),
@@ -320,7 +401,8 @@ public class ConnectionRegistryImplTest extends BrambleMockTestCase {
assertEquals(singletonList(contactId1),
c.getConnectedOrBetterContacts(transportId2));
// Connect via transport 3 (no preference)
// Connect via transport 3 (no preference) and set priority to high -
// no further interruptions
context.checking(new Expectations() {{
oneOf(eventBus).broadcast(with(any(ConnectionOpenedEvent.class)));
}});