Merge branch '748-qr-code-payload-order' into 'master'

Preserve the order of descriptors in QR code payloads

This fixes a regression caused by my recent changes to the Payload class.

Closes #748

See merge request !399

briar-api/src/org/briarproject/api/keyagreement/Payload.java

@@ -1,11 +1,9 @@
 package org.briarproject.api.keyagreement;
 
 import org.briarproject.api.Bytes;
-import org.briarproject.api.TransportId;
-import org.briarproject.api.data.BdfList;
 import org.briarproject.api.nullsafety.NotNullByDefault;
 
-import java.util.Map;
+import java.util.List;
 
 import javax.annotation.concurrent.Immutable;
 
@@ -17,9 +15,9 @@ import javax.annotation.concurrent.Immutable;
 public class Payload implements Comparable<Payload> {
 
 	private final Bytes commitment;
-	private final Map<TransportId, BdfList> descriptors;
+	private final List<TransportDescriptor> descriptors;
 
-	public Payload(byte[] commitment, Map<TransportId, BdfList> descriptors) {
+	public Payload(byte[] commitment, List<TransportDescriptor> descriptors) {
 		this.commitment = new Bytes(commitment);
 		this.descriptors = descriptors;
 	}
@@ -34,7 +32,7 @@ public class Payload implements Comparable<Payload> {
 	/**
 	 * Returns the transport descriptors contained in this payload.
 	 */
-	public Map<TransportId, BdfList> getTransportDescriptors() {
+	public List<TransportDescriptor> getTransportDescriptors() {
 		return descriptors;
 	}
 

briar-api/src/org/briarproject/api/keyagreement/TransportDescriptor.java

@@ -0,0 +1,28 @@
+package org.briarproject.api.keyagreement;
+
+import org.briarproject.api.TransportId;
+import org.briarproject.api.data.BdfList;
+import org.briarproject.api.nullsafety.NotNullByDefault;
+
+import javax.annotation.concurrent.Immutable;
+
+@Immutable
+@NotNullByDefault
+public class TransportDescriptor {
+
+	private final TransportId id;
+	private final BdfList descriptor;
+
+	public TransportDescriptor(TransportId id, BdfList descriptor) {
+		this.id = id;
+		this.descriptor = descriptor;
+	}
+
+	public TransportId getId() {
+		return id;
+	}
+
+	public BdfList getDescriptor() {
+		return descriptor;
+	}
+}

briar-core/src/org/briarproject/keyagreement/KeyAgreementConnector.java

@@ -7,6 +7,7 @@ import org.briarproject.api.data.BdfList;
 import org.briarproject.api.keyagreement.KeyAgreementConnection;
 import org.briarproject.api.keyagreement.KeyAgreementListener;
 import org.briarproject.api.keyagreement.Payload;
+import org.briarproject.api.keyagreement.TransportDescriptor;
 import org.briarproject.api.plugins.Plugin;
 import org.briarproject.api.plugins.PluginManager;
 import org.briarproject.api.plugins.duplex.DuplexPlugin;
@@ -16,10 +17,7 @@ import org.briarproject.api.system.Clock;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
 import java.util.concurrent.Callable;
 import java.util.concurrent.CompletionService;
 import java.util.concurrent.ExecutionException;
@@ -73,13 +71,14 @@ class KeyAgreementConnector {
 		byte[] commitment = crypto.deriveKeyCommitment(
 				localKeyPair.getPublic().getEncoded());
 		// Start all listeners and collect their descriptors
-		Map<TransportId, BdfList> descriptors =
+		List<TransportDescriptor> descriptors =
-				new HashMap<TransportId, BdfList>();
+				new ArrayList<TransportDescriptor>();
 		for (DuplexPlugin plugin : pluginManager.getKeyAgreementPlugins()) {
 			KeyAgreementListener l =
 					plugin.createKeyAgreementListener(commitment);
 			if (l != null) {
-				descriptors.put(plugin.getId(), l.getDescriptor());
+				TransportId id = plugin.getId();
+				descriptors.add(new TransportDescriptor(id, l.getDescriptor()));
 				pending.add(connect.submit(new ReadableTask(l.listen())));
 				listeners.add(l);
 			}
@@ -104,15 +103,13 @@ class KeyAgreementConnector {
 
 		// Start connecting over supported transports
 		LOG.info("Starting outgoing BQP connections");
-		Map<TransportId, BdfList> descriptors =
+		for (TransportDescriptor d : remotePayload.getTransportDescriptors()) {
-				remotePayload.getTransportDescriptors();
+			Plugin p = pluginManager.getPlugin(d.getId());
-		for (Entry<TransportId, BdfList> e : descriptors.entrySet()) {
-			Plugin p = pluginManager.getPlugin(e.getKey());
 			if (p instanceof DuplexPlugin) {
 				DuplexPlugin plugin = (DuplexPlugin) p;
 				pending.add(connect.submit(new ReadableTask(
 						new ConnectorTask(plugin, remotePayload.getCommitment(),
-								e.getValue(), end))));
+								d.getDescriptor(), end))));
 			}
 		}
 

briar-core/src/org/briarproject/keyagreement/KeyAgreementProtocol.java

@@ -15,51 +15,53 @@ import java.util.Arrays;
  * <p/>
  * Alice:
  * <ul>
- *     <li>Send A_KEY</li>
+ * <li>Send A_KEY</li>
- *     <li>Receive B_KEY
+ * <li>Receive B_KEY
- *     <ul>
+ * <ul>
- *         <li>Check B_KEY matches B_COMMIT</li>
+ * <li>Check B_KEY matches B_COMMIT</li>
- *     </ul></li>
+ * </ul></li>
- *     <li>Calculate s</li>
+ * <li>Calculate s</li>
- *     <li>Send A_CONFIRM</li>
+ * <li>Send A_CONFIRM</li>
- *     <li>Receive B_CONFIRM
+ * <li>Receive B_CONFIRM
- *     <ul>
+ * <ul>
- *         <li>Check B_CONFIRM matches expected</li>
+ * <li>Check B_CONFIRM matches expected</li>
- *     </ul></li>
+ * </ul></li>
- *     <li>Derive master</li>
+ * <li>Derive master</li>
  * </ul><p/>
  * Bob:
  * <ul>
- *     <li>Receive A_KEY
+ * <li>Receive A_KEY
- *     <ul>
+ * <ul>
- *         <li>Check A_KEY matches A_COMMIT</li>
+ * <li>Check A_KEY matches A_COMMIT</li>
- *     </ul></li>
+ * </ul></li>
- *     <li>Send B_KEY</li>
+ * <li>Send B_KEY</li>
- *     <li>Calculate s</li>
+ * <li>Calculate s</li>
- *     <li>Receive A_CONFIRM
+ * <li>Receive A_CONFIRM
- *     <ul>
+ * <ul>
- *         <li>Check A_CONFIRM matches expected</li>
+ * <li>Check A_CONFIRM matches expected</li>
- *     </ul></li>
+ * </ul></li>
- *     <li>Send B_CONFIRM</li>
+ * <li>Send B_CONFIRM</li>
- *     <li>Derive master</li>
+ * <li>Derive master</li>
  * </ul>
  */
 class KeyAgreementProtocol {
 
 	interface Callbacks {
+
 		void connectionWaiting();
+
 		void initialPacketReceived();
 	}
 
-	private Callbacks callbacks;
+	private final Callbacks callbacks;
-	private CryptoComponent crypto;
+	private final CryptoComponent crypto;
-	private PayloadEncoder payloadEncoder;
+	private final PayloadEncoder payloadEncoder;
-	private KeyAgreementTransport transport;
+	private final KeyAgreementTransport transport;
-	private Payload theirPayload, ourPayload;
+	private final Payload theirPayload, ourPayload;
-	private KeyPair ourKeyPair;
+	private final KeyPair ourKeyPair;
-	private boolean alice;
+	private final boolean alice;
 
-	public KeyAgreementProtocol(Callbacks callbacks, CryptoComponent crypto,
+	KeyAgreementProtocol(Callbacks callbacks, CryptoComponent crypto,
 			PayloadEncoder payloadEncoder, KeyAgreementTransport transport,
 			Payload theirPayload, Payload ourPayload, KeyPair ourKeyPair,
 			boolean alice) {
@@ -78,9 +80,9 @@ class KeyAgreementProtocol {
 	 *
 	 * @return the negotiated master secret.
 	 * @throws AbortException when the protocol may have been tampered with.
-	 * @throws IOException for all other other connection errors.
+	 * @throws IOException    for all other other connection errors.
 	 */
-	public SecretKey perform() throws AbortException, IOException {
+	SecretKey perform() throws AbortException, IOException {
 		try {
 			byte[] theirPublicKey;
 			if (alice) {

briar-core/src/org/briarproject/keyagreement/PayloadEncoderImpl.java

@@ -1,16 +1,14 @@
 package org.briarproject.keyagreement;
 
-import org.briarproject.api.TransportId;
-import org.briarproject.api.data.BdfList;
 import org.briarproject.api.data.BdfWriter;
 import org.briarproject.api.data.BdfWriterFactory;
 import org.briarproject.api.keyagreement.Payload;
 import org.briarproject.api.keyagreement.PayloadEncoder;
+import org.briarproject.api.keyagreement.TransportDescriptor;
 import org.briarproject.api.nullsafety.NotNullByDefault;
 
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
-import java.util.Map;
 
 import javax.annotation.concurrent.Immutable;
 import javax.inject.Inject;
@@ -36,9 +34,8 @@ class PayloadEncoderImpl implements PayloadEncoder {
 			w.writeListStart(); // Payload start
 			w.writeLong(PROTOCOL_VERSION);
 			w.writeRaw(p.getCommitment());
-			Map<TransportId, BdfList> descriptors = p.getTransportDescriptors();
+			for (TransportDescriptor d : p.getTransportDescriptors())
-			for (BdfList descriptor : descriptors.values())
+				w.writeList(d.getDescriptor());
-				w.writeList(descriptor);
 			w.writeListEnd(); // Payload end
 		} catch (IOException e) {
 			// Shouldn't happen with ByteArrayOutputStream

briar-core/src/org/briarproject/keyagreement/PayloadParserImpl.java

@@ -7,12 +7,13 @@ import org.briarproject.api.data.BdfReader;
 import org.briarproject.api.data.BdfReaderFactory;
 import org.briarproject.api.keyagreement.Payload;
 import org.briarproject.api.keyagreement.PayloadParser;
+import org.briarproject.api.keyagreement.TransportDescriptor;
 import org.briarproject.api.nullsafety.NotNullByDefault;
 
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
-import java.util.HashMap;
+import java.util.ArrayList;
-import java.util.Map;
+import java.util.List;
 
 import javax.annotation.concurrent.Immutable;
 import javax.inject.Inject;
@@ -48,15 +49,17 @@ class PayloadParserImpl implements PayloadParser {
 		byte[] commitment = payload.getRaw(1);
 		if (commitment.length != COMMIT_LENGTH) throw new FormatException();
 		// Remaining elements: transport descriptors
-		Map<TransportId, BdfList> recognised =
+		List<TransportDescriptor> recognised =
-				new HashMap<TransportId, BdfList>();
+				new ArrayList<TransportDescriptor>();
 		for (int i = 2; i < payload.size(); i++) {
 			BdfList descriptor = payload.getList(i);
 			long transportId = descriptor.getLong(0);
 			if (transportId == TRANSPORT_ID_BLUETOOTH) {
-				recognised.put(new TransportId("bt"), descriptor);
+				TransportId id = new TransportId("bt");
+				recognised.add(new TransportDescriptor(id, descriptor));
 			} else if (transportId == TRANSPORT_ID_LAN) {
-				recognised.put(new TransportId("lan"), descriptor);
+				TransportId id = new TransportId("lan");
+				recognised.add(new TransportDescriptor(id, descriptor));
 			}
 		}
 		return new Payload(commitment, recognised);