Add support for reading and writing BDF in canonical form.

Existing transport property updates may not be in canonical form, so we need to parse them leniently.

bramble-api/src/main/java/org/briarproject/bramble/api/client/BdfMessageValidator.java

@@ -16,6 +16,7 @@ import java.util.logging.Logger;
 
 import javax.annotation.concurrent.Immutable;
 
+import static java.util.logging.Logger.getLogger;
 import static org.briarproject.bramble.api.transport.TransportConstants.MAX_CLOCK_DIFFERENCE;
 
 @Immutable
@@ -23,17 +24,24 @@ import static org.briarproject.bramble.api.transport.TransportConstants.MAX_CLOC
 public abstract class BdfMessageValidator implements MessageValidator {
 
 	protected static final Logger LOG =
-			Logger.getLogger(BdfMessageValidator.class.getName());
+			getLogger(BdfMessageValidator.class.getName());
 
 	protected final ClientHelper clientHelper;
 	protected final MetadataEncoder metadataEncoder;
 	protected final Clock clock;
+	protected final boolean canonical;
 
 	protected BdfMessageValidator(ClientHelper clientHelper,
-			MetadataEncoder metadataEncoder, Clock clock) {
+			MetadataEncoder metadataEncoder, Clock clock, boolean canonical) {
 		this.clientHelper = clientHelper;
 		this.metadataEncoder = metadataEncoder;
 		this.clock = clock;
+		this.canonical = canonical;
+	}
+
+	protected BdfMessageValidator(ClientHelper clientHelper,
+			MetadataEncoder metadataEncoder, Clock clock) {
+		this(clientHelper, metadataEncoder, clock, true);
 	}
 
 	protected abstract BdfMessageContext validateMessage(Message m, Group g,
@@ -49,7 +57,7 @@ public abstract class BdfMessageValidator implements MessageValidator {
 					"Timestamp is too far in the future");
 		}
 		try {
-			BdfList bodyList = clientHelper.toList(m.getBody());
+			BdfList bodyList = clientHelper.toList(m, canonical);
 			BdfMessageContext result = validateMessage(m, g, bodyList);
 			Metadata meta = metadataEncoder.encode(result.getDictionary());
 			return new MessageContext(meta, result.getDependencies());

bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java

@@ -49,6 +49,9 @@ public interface ClientHelper {
 	BdfList getMessageAsList(Transaction txn, MessageId m) throws DbException,
 			FormatException;
 
+	BdfList getMessageAsList(Transaction txn, MessageId m, boolean canonical)
+			throws DbException, FormatException;
+
 	BdfDictionary getGroupMetadataAsDictionary(GroupId g) throws DbException,
 			FormatException;
 
@@ -106,6 +109,8 @@ public interface ClientHelper {
 
 	BdfList toList(Message m) throws FormatException;
 
+	BdfList toList(Message m, boolean canonical) throws FormatException;
+
 	BdfList toList(Author a);
 
 	byte[] sign(String label, BdfList toSign, PrivateKey privateKey)

bramble-api/src/main/java/org/briarproject/bramble/api/data/BdfReaderFactory.java

@@ -9,6 +9,8 @@ public interface BdfReaderFactory {
 
 	BdfReader createReader(InputStream in);
 
+	BdfReader createReader(InputStream in, boolean canonical);
+
 	BdfReader createReader(InputStream in, int nestedLimit,
-			int maxBufferSize);
+			int maxBufferSize, boolean canonical);
 }

bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java

@@ -155,7 +155,13 @@ class ClientHelperImpl implements ClientHelper {
 	@Override
 	public BdfList getMessageAsList(Transaction txn, MessageId m)
 			throws DbException, FormatException {
-		return toList(db.getMessage(txn, m).getBody());
+		return getMessageAsList(txn, m, true);
+	}
+
+	@Override
+	public BdfList getMessageAsList(Transaction txn, MessageId m,
+			boolean canonical) throws DbException, FormatException {
+		return toList(db.getMessage(txn, m), canonical);
 	}
 
 	@Override
@@ -313,8 +319,13 @@ class ClientHelperImpl implements ClientHelper {
 
 	@Override
 	public BdfList toList(byte[] b, int off, int len) throws FormatException {
+		return toList(b, off, len, true);
+	}
+
+	private BdfList toList(byte[] b, int off, int len, boolean canonical)
+			throws FormatException {
 		ByteArrayInputStream in = new ByteArrayInputStream(b, off, len);
-		BdfReader reader = bdfReaderFactory.createReader(in);
+		BdfReader reader = bdfReaderFactory.createReader(in, canonical);
 		try {
 			BdfList list = reader.readList();
 			if (!reader.eof()) throw new FormatException();
@@ -328,7 +339,7 @@ class ClientHelperImpl implements ClientHelper {
 
 	@Override
 	public BdfList toList(byte[] b) throws FormatException {
-		return toList(b, 0, b.length);
+		return toList(b, 0, b.length, true);
 	}
 
 	@Override
@@ -336,6 +347,12 @@ class ClientHelperImpl implements ClientHelper {
 		return toList(m.getBody());
 	}
 
+	@Override
+	public BdfList toList(Message m, boolean canonical) throws FormatException {
+		byte[] b = m.getBody();
+		return toList(b, 0, b.length, canonical);
+	}
+
 	@Override
 	public BdfList toList(Author a) {
 		return BdfList.of(a.getFormatVersion(), a.getName(), a.getPublicKey());

bramble-core/src/main/java/org/briarproject/bramble/data/BdfReaderFactoryImpl.java

@@ -18,12 +18,18 @@ class BdfReaderFactoryImpl implements BdfReaderFactory {
 	@Override
 	public BdfReader createReader(InputStream in) {
 		return new BdfReaderImpl(in, DEFAULT_NESTED_LIMIT,
-				DEFAULT_MAX_BUFFER_SIZE);
+				DEFAULT_MAX_BUFFER_SIZE, true);
+	}
+
+	@Override
+	public BdfReader createReader(InputStream in, boolean canonical) {
+		return new BdfReaderImpl(in, DEFAULT_NESTED_LIMIT,
+				DEFAULT_MAX_BUFFER_SIZE, canonical);
 	}
 
 	@Override
 	public BdfReader createReader(InputStream in, int nestedLimit,
-			int maxBufferSize) {
-		return new BdfReaderImpl(in, nestedLimit, maxBufferSize);
+			int maxBufferSize, boolean canonical) {
+		return new BdfReaderImpl(in, nestedLimit, maxBufferSize, canonical);
 	}
 }

bramble-core/src/main/java/org/briarproject/bramble/data/BdfReaderImpl.java

@@ -39,15 +39,18 @@ class BdfReaderImpl implements BdfReader {
 
 	private final InputStream in;
 	private final int nestedLimit, maxBufferSize;
+	private final boolean canonical;
 
 	private boolean hasLookahead = false, eof = false;
 	private byte next;
 	private byte[] buf = new byte[8];
 
-	BdfReaderImpl(InputStream in, int nestedLimit, int maxBufferSize) {
+	BdfReaderImpl(InputStream in, int nestedLimit, int maxBufferSize,
+			boolean canonical) {
 		this.in = in;
 		this.nestedLimit = nestedLimit;
 		this.maxBufferSize = maxBufferSize;
+		this.canonical = canonical;
 	}
 
 	private void readLookahead() throws IOException {
@@ -188,13 +191,22 @@ class BdfReaderImpl implements BdfReader {
 
 	private short readInt16() throws IOException {
 		readIntoBuffer(2);
-		return (short) (((buf[0] & 0xFF) << 8) + (buf[1] & 0xFF));
+		short value = (short) (((buf[0] & 0xFF) << 8) + (buf[1] & 0xFF));
+		if (canonical && value >= Byte.MIN_VALUE && value <= Byte.MAX_VALUE) {
+			// Value could have been encoded as an INT_8
+			throw new FormatException();
+		}
+		return value;
 	}
 
 	private int readInt32() throws IOException {
 		readIntoBuffer(4);
 		int value = 0;
 		for (int i = 0; i < 4; i++) value |= (buf[i] & 0xFF) << (24 - i * 8);
+		if (canonical && value >= Short.MIN_VALUE && value <= Short.MAX_VALUE) {
+			// Value could have been encoded as an INT_16
+			throw new FormatException();
+		}
 		return value;
 	}
 
@@ -202,6 +214,11 @@ class BdfReaderImpl implements BdfReader {
 		readIntoBuffer(8);
 		long value = 0;
 		for (int i = 0; i < 8; i++) value |= (buf[i] & 0xFFL) << (56 - i * 8);
+		if (canonical && value >= Integer.MIN_VALUE &&
+				value <= Integer.MAX_VALUE) {
+			// Value could have been encoded as an INT_32
+			throw new FormatException();
+		}
 		return value;
 	}
 
@@ -382,8 +399,16 @@ class BdfReaderImpl implements BdfReader {
 		if (level > nestedLimit) throw new FormatException();
 		BdfDictionary dictionary = new BdfDictionary();
 		readDictionaryStart();
-		while (!hasDictionaryEnd())
-			dictionary.put(readString(), readObject(level + 1));
+		String prevKey = null;
+		while (!hasDictionaryEnd()) {
+			String key = readString();
+			if (canonical && prevKey != null && key.compareTo(prevKey) <= 0) {
+				// Keys not unique and sorted
+				throw new FormatException();
+			}
+			dictionary.put(key, readObject(level + 1));
+			prevKey = key;
+		}
 		readDictionaryEnd();
 		return dictionary;
 	}

bramble-core/src/main/java/org/briarproject/bramble/data/BdfWriterImpl.java

@@ -7,14 +7,15 @@ import org.briarproject.nullsafety.NotNullByDefault;
 
 import java.io.IOException;
 import java.io.OutputStream;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.NotThreadSafe;
 
+import static java.util.Collections.sort;
 import static org.briarproject.bramble.api.data.BdfDictionary.NULL_VALUE;
 import static org.briarproject.bramble.data.Types.DICTIONARY;
 import static org.briarproject.bramble.data.Types.END;
@@ -33,6 +34,7 @@ import static org.briarproject.bramble.data.Types.STRING_16;
 import static org.briarproject.bramble.data.Types.STRING_32;
 import static org.briarproject.bramble.data.Types.STRING_8;
 import static org.briarproject.bramble.data.Types.TRUE;
+import static org.briarproject.bramble.util.StringUtils.UTF_8;
 
 @NotThreadSafe
 @NotNullByDefault
@@ -113,7 +115,7 @@ class BdfWriterImpl implements BdfWriter {
 
 	@Override
 	public void writeString(String s) throws IOException {
-		byte[] b = s.getBytes("UTF-8");
+		byte[] b = s.getBytes(UTF_8);
 		if (b.length <= Byte.MAX_VALUE) {
 			out.write(STRING_8);
 			out.write((byte) b.length);
@@ -161,8 +163,8 @@ class BdfWriterImpl implements BdfWriter {
 		else if (o instanceof String) writeString((String) o);
 		else if (o instanceof byte[]) writeRaw((byte[]) o);
 		else if (o instanceof Bytes) writeRaw(((Bytes) o).getBytes());
-		else if (o instanceof List) writeList((List) o);
-		else if (o instanceof Map) writeDictionary((Map) o);
+		else if (o instanceof List) writeList((List<?>) o);
+		else if (o instanceof Map) writeDictionary((Map<?, ?>) o);
 		else throw new FormatException();
 	}
 
@@ -179,10 +181,16 @@ class BdfWriterImpl implements BdfWriter {
 	@Override
 	public void writeDictionary(Map<?, ?> m) throws IOException {
 		out.write(DICTIONARY);
-		for (Entry<?, ?> e : m.entrySet()) {
-			if (!(e.getKey() instanceof String)) throw new FormatException();
-			writeString((String) e.getKey());
-			writeObject(e.getValue());
+		// Write entries in canonical order
+		List<String> keys = new ArrayList<>(m.size());
+		for (Object k : m.keySet()) {
+			if (!(k instanceof String)) throw new FormatException();
+			keys.add((String) k);
+		}
+		sort(keys);
+		for (String key : keys) {
+			writeString(key);
+			writeObject(m.get(key));
 		}
 		out.write(END);
 	}

bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyManagerImpl.java

@@ -201,7 +201,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 			// Retrieve and parse the latest local properties
 			for (Entry<TransportId, LatestUpdate> e : latest.entrySet()) {
 				BdfList message = clientHelper.getMessageAsList(txn,
-						e.getValue().messageId);
+						e.getValue().messageId, false);
 				local.put(e.getKey(), parseProperties(message));
 			}
 			return local;
@@ -222,7 +222,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 				if (latest != null) {
 					// Retrieve and parse the latest local properties
 					BdfList message = clientHelper.getMessageAsList(txn,
-							latest.messageId);
+							latest.messageId, false);
 					p = parseProperties(message);
 				}
 				return p == null ? new TransportProperties() : p;
@@ -252,7 +252,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 				local = new TransportProperties();
 			} else {
 				BdfList message = clientHelper.getMessageAsList(txn,
-						latest.messageId);
+						latest.messageId, false);
 				local = parseProperties(message);
 			}
 			storeLocalProperties(txn, c, t, local);
@@ -272,8 +272,8 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 				remote = new TransportProperties();
 			} else {
 				// Retrieve and parse the latest remote properties
-				BdfList message =
-						clientHelper.getMessageAsList(txn, latest.messageId);
+				BdfList message = clientHelper.getMessageAsList(txn,
+						latest.messageId, false);
 				remote = parseProperties(message);
 			}
 			// Merge in any discovered properties
@@ -317,7 +317,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager,
 					changed = true;
 				} else {
 					BdfList message = clientHelper.getMessageAsList(txn,
-							latest.messageId);
+							latest.messageId, false);
 					TransportProperties old = parseProperties(message);
 					merged = new TransportProperties(old);
 					for (Entry<String, String> e : p.entrySet()) {

bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyValidator.java

@@ -27,7 +27,10 @@ class TransportPropertyValidator extends BdfMessageValidator {
 
 	TransportPropertyValidator(ClientHelper clientHelper,
 			MetadataEncoder metadataEncoder, Clock clock) {
-		super(clientHelper, metadataEncoder, clock);
+		// Accept transport properties in non-canonical form
+		// TODO: Remove this after a reasonable migration period
+		//  (added 2023-02-17)
+		super(clientHelper, metadataEncoder, clock, false);
 	}
 
 	@Override

bramble-core/src/test/java/org/briarproject/bramble/client/BdfMessageValidatorTest.java

@@ -56,7 +56,7 @@ public class BdfMessageValidatorTest extends ValidatorTestCase {
 		context.checking(new Expectations() {{
 			oneOf(clock).currentTimeMillis();
 			will(returnValue(timestamp - MAX_CLOCK_DIFFERENCE));
-			oneOf(clientHelper).toList(message.getBody());
+			oneOf(clientHelper).toList(message, true);
 			will(returnValue(body));
 			oneOf(metadataEncoder).encode(dictionary);
 			will(returnValue(meta));
@@ -86,7 +86,7 @@ public class BdfMessageValidatorTest extends ValidatorTestCase {
 		context.checking(new Expectations() {{
 			oneOf(clock).currentTimeMillis();
 			will(returnValue(timestamp));
-			oneOf(clientHelper).toList(shortMessage.getBody());
+			oneOf(clientHelper).toList(shortMessage, true);
 			will(returnValue(body));
 			oneOf(metadataEncoder).encode(dictionary);
 			will(returnValue(meta));
@@ -114,7 +114,7 @@ public class BdfMessageValidatorTest extends ValidatorTestCase {
 		context.checking(new Expectations() {{
 			oneOf(clock).currentTimeMillis();
 			will(returnValue(timestamp));
-			oneOf(clientHelper).toList(message.getBody());
+			oneOf(clientHelper).toList(message, true);
 			will(throwException(new FormatException()));
 		}});
 
@@ -126,7 +126,7 @@ public class BdfMessageValidatorTest extends ValidatorTestCase {
 		context.checking(new Expectations() {{
 			oneOf(clock).currentTimeMillis();
 			will(returnValue(timestamp));
-			oneOf(clientHelper).toList(message.getBody());
+			oneOf(clientHelper).toList(message, true);
 			will(returnValue(body));
 		}});
 

bramble-core/src/test/java/org/briarproject/bramble/client/ClientHelperImplTest.java

@@ -546,7 +546,7 @@ public class ClientHelperImplTest extends BrambleMockTestCase {
 
 		context.checking(new Expectations() {{
 			oneOf(bdfReaderFactory)
-					.createReader(with(any(InputStream.class)));
+					.createReader(with(any(InputStream.class)), with(true));
 			will(returnValue(bdfReader));
 			oneOf(bdfReader).readList();
 			will(returnValue(list));

bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderImplFuzzingTest.java

@@ -32,7 +32,7 @@ public class BdfReaderImplFuzzingTest extends BrambleTestCase {
 			buf[1] = 0x14; // Length 20 bytes
 			in.reset();
 			BdfReaderImpl r = new BdfReaderImpl(in, DEFAULT_NESTED_LIMIT,
-					DEFAULT_MAX_BUFFER_SIZE);
+					DEFAULT_MAX_BUFFER_SIZE, true);
 			try {
 				int length = r.readString().length();
 				assertTrue(length <= 20);

bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderImplTest.java

@@ -11,6 +11,7 @@ import java.io.ByteArrayInputStream;
 import static org.briarproject.bramble.api.data.BdfDictionary.NULL_VALUE;
 import static org.briarproject.bramble.api.data.BdfReader.DEFAULT_MAX_BUFFER_SIZE;
 import static org.briarproject.bramble.data.BdfReaderImpl.DEFAULT_NESTED_LIMIT;
+import static org.briarproject.bramble.util.StringUtils.UTF_8;
 import static org.briarproject.bramble.util.StringUtils.fromHexString;
 import static org.briarproject.bramble.util.StringUtils.getRandomString;
 import static org.briarproject.bramble.util.StringUtils.toHexString;
@@ -88,6 +89,18 @@ public class BdfReaderImplTest extends BrambleTestCase {
 		assertTrue(r.eof());
 	}
 
+	@Test(expected = FormatException.class)
+	public void testReadLong16CouldHaveBeenLong8Max() throws Exception {
+		setContents("22" + "007F");
+		r.readLong();
+	}
+
+	@Test(expected = FormatException.class)
+	public void testReadLong16CouldHaveBeenLong8Min() throws Exception {
+		setContents("22" + "FF80");
+		r.readLong();
+	}
+
 	@Test
 	public void testSkipLong16() throws Exception {
 		setContents("22" + "0080");
@@ -106,6 +119,18 @@ public class BdfReaderImplTest extends BrambleTestCase {
 		assertTrue(r.eof());
 	}
 
+	@Test(expected = FormatException.class)
+	public void testReadLong32CouldHaveBeenLong16Max() throws Exception {
+		setContents("24" + "00007FFF");
+		r.readLong();
+	}
+
+	@Test(expected = FormatException.class)
+	public void testReadLong32CouldHaveBeenLong16Min() throws Exception {
+		setContents("24" + "FFFF8000");
+		r.readLong();
+	}
+
 	@Test
 	public void testSkipLong32() throws Exception {
 		setContents("24" + "00008000");
@@ -124,6 +149,18 @@ public class BdfReaderImplTest extends BrambleTestCase {
 		assertTrue(r.eof());
 	}
 
+	@Test(expected = FormatException.class)
+	public void testReadLong64CouldHaveBeenLong32Max() throws Exception {
+		setContents("28" + "000000007FFFFFFF");
+		r.readLong();
+	}
+
+	@Test(expected = FormatException.class)
+	public void testReadLong64CouldHaveBeenLong32Min() throws Exception {
+		setContents("28" + "FFFFFFFF80000000");
+		r.readLong();
+	}
+
 	@Test
 	public void testSkipLong() throws Exception {
 		setContents("28" + "0000000080000000");
@@ -162,7 +199,7 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	@Test
 	public void testReadString8() throws Exception {
 		String longest = getRandomString(Byte.MAX_VALUE);
-		String longHex = toHexString(longest.getBytes("UTF-8"));
+		String longHex = toHexString(longest.getBytes(UTF_8));
 		// "foo", the empty string, and 127 random letters
 		setContents("41" + "03" + "666F6F" + "41" + "00" +
 				"41" + "7F" + longHex);
@@ -186,7 +223,7 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	@Test
 	public void testSkipString8() throws Exception {
 		String longest = getRandomString(Byte.MAX_VALUE);
-		String longHex = toHexString(longest.getBytes("UTF-8"));
+		String longHex = toHexString(longest.getBytes(UTF_8));
 		// "foo", the empty string, and 127 random letters
 		setContents("41" + "03" + "666F6F" + "41" + "00" +
 				"41" + "7F" + longHex);
@@ -199,9 +236,9 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	@Test
 	public void testReadString16() throws Exception {
 		String shortest = getRandomString(Byte.MAX_VALUE + 1);
-		String shortHex = toHexString(shortest.getBytes("UTF-8"));
+		String shortHex = toHexString(shortest.getBytes(UTF_8));
 		String longest = getRandomString(Short.MAX_VALUE);
-		String longHex = toHexString(longest.getBytes("UTF-8"));
+		String longHex = toHexString(longest.getBytes(UTF_8));
 		// 128 random letters and 2^15 -1 random letters
 		setContents("42" + "0080" + shortHex + "42" + "7FFF" + longHex);
 		assertEquals(shortest, r.readString());
@@ -213,7 +250,7 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	public void testReadString16ChecksMaxLength() throws Exception {
 		int maxBufferSize = Byte.MAX_VALUE + 1;
 		String valid = getRandomString(Byte.MAX_VALUE + 1);
-		String validHex = toHexString(valid.getBytes("UTF-8"));
+		String validHex = toHexString(valid.getBytes(UTF_8));
 		String invalidhex = validHex + "20";
 		// 128 random letters, the same plus a space
 		setContents("42" + "0080" + validHex
@@ -223,12 +260,20 @@ public class BdfReaderImplTest extends BrambleTestCase {
 		r.readString();
 	}
 
+	@Test(expected = FormatException.class)
+	public void testReadString16CouldHaveBeenString8() throws Exception {
+		String longest = getRandomString(Byte.MAX_VALUE);
+		String longHex = toHexString(longest.getBytes(UTF_8));
+		setContents("42" + "007F" + longHex);
+		r.readString();
+	}
+
 	@Test
 	public void testSkipString16() throws Exception {
 		String shortest = getRandomString(Byte.MAX_VALUE + 1);
-		String shortHex = toHexString(shortest.getBytes("UTF-8"));
+		String shortHex = toHexString(shortest.getBytes(UTF_8));
 		String longest = getRandomString(Short.MAX_VALUE);
-		String longHex = toHexString(longest.getBytes("UTF-8"));
+		String longHex = toHexString(longest.getBytes(UTF_8));
 		// 128 random letters and 2^15 - 1 random letters
 		setContents("42" + "0080" + shortHex + "42" + "7FFF" + longHex);
 		r.skipString();
@@ -239,7 +284,7 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	@Test
 	public void testReadString32() throws Exception {
 		String shortest = getRandomString(Short.MAX_VALUE + 1);
-		String shortHex = toHexString(shortest.getBytes("UTF-8"));
+		String shortHex = toHexString(shortest.getBytes(UTF_8));
 		// 2^15 random letters
 		setContents("44" + "00008000" + shortHex);
 		assertEquals(shortest, r.readString());
@@ -250,7 +295,7 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	public void testReadString32ChecksMaxLength() throws Exception {
 		int maxBufferSize = Short.MAX_VALUE + 1;
 		String valid = getRandomString(maxBufferSize);
-		String validHex = toHexString(valid.getBytes("UTF-8"));
+		String validHex = toHexString(valid.getBytes(UTF_8));
 		String invalidHex = validHex + "20";
 		// 2^15 random letters, the same plus a space
 		setContents("44" + "00008000" + validHex +
@@ -260,10 +305,18 @@ public class BdfReaderImplTest extends BrambleTestCase {
 		r.readString();
 	}
 
+	@Test(expected = FormatException.class)
+	public void testReadString32CouldHaveBeenString16() throws Exception {
+		String longest = getRandomString(Short.MAX_VALUE);
+		String longHex = toHexString(longest.getBytes(UTF_8));
+		setContents("44" + "00007FFF" + longHex);
+		r.readString();
+	}
+
 	@Test
 	public void testSkipString32() throws Exception {
 		String shortest = getRandomString(Short.MAX_VALUE + 1);
-		String shortHex = toHexString(shortest.getBytes("UTF-8"));
+		String shortHex = toHexString(shortest.getBytes(UTF_8));
 		// 2^15 random letters, twice
 		setContents("44" + "00008000" + shortHex +
 				"44" + "00008000" + shortHex);
@@ -275,7 +328,7 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	@Test
 	public void testReadUtf8String() throws Exception {
 		String unicode = "\uFDD0\uFDD1\uFDD2\uFDD3";
-		String hex = toHexString(unicode.getBytes("UTF-8"));
+		String hex = toHexString(unicode.getBytes(UTF_8));
 		// STRING_8 tag, "foo", the empty string, and the test string
 		setContents("41" + "03" + "666F6F" + "41" + "00" + "41" + "0C" + hex);
 		assertEquals("foo", r.readString());
@@ -348,6 +401,14 @@ public class BdfReaderImplTest extends BrambleTestCase {
 		r.readRaw();
 	}
 
+	@Test(expected = FormatException.class)
+	public void testReadRaw16CouldHaveBeenRaw8() throws Exception {
+		byte[] longest = new byte[Byte.MAX_VALUE];
+		String longHex = toHexString(longest);
+		setContents("52" + "007F" + longHex);
+		r.readRaw();
+	}
+
 	@Test
 	public void testSkipRaw16() throws Exception {
 		byte[] shortest = new byte[Byte.MAX_VALUE + 1];
@@ -385,6 +446,14 @@ public class BdfReaderImplTest extends BrambleTestCase {
 		r.readRaw();
 	}
 
+	@Test(expected = FormatException.class)
+	public void testReadRaw32CouldHaveBeenRaw16() throws Exception {
+		byte[] longest = new byte[Short.MAX_VALUE];
+		String longHex = toHexString(longest);
+		setContents("54" + "00007FFF" + longHex);
+		r.readRaw();
+	}
+
 	@Test
 	public void testSkipRaw32() throws Exception {
 		byte[] shortest = new byte[Short.MAX_VALUE + 1];
@@ -465,9 +534,9 @@ public class BdfReaderImplTest extends BrambleTestCase {
 
 	@Test
 	public void testReadDictionary() throws Exception {
-		// A dictionary containing "foo" -> 123 and "bar" -> null
-		setContents("70" + "41" + "03" + "666F6F" + "21" + "7B" +
-				"41" + "03" + "626172" + "00" + "80");
+		// A dictionary containing "bar" -> null and "foo" -> 123
+		setContents("70" + "41" + "03" + "626172" + "00" +
+				"41" + "03" + "666F6F" + "21" + "7B" + "80");
 		BdfDictionary dictionary = r.readDictionary();
 		assertEquals(2, dictionary.size());
 		assertTrue(dictionary.containsKey("foo"));
@@ -557,10 +626,10 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	@Test
 	public void testNestedListWithinDepthLimit() throws Exception {
 		// A list containing a list containing a list containing a list...
-		String lists = "";
-		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++) lists += "60";
-		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++) lists += "80";
-		setContents(lists);
+		StringBuilder lists = new StringBuilder();
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++) lists.append("60");
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++) lists.append("80");
+		setContents(lists.toString());
 		r.readList();
 		assertTrue(r.eof());
 	}
@@ -568,23 +637,25 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	@Test(expected = FormatException.class)
 	public void testNestedListOutsideDepthLimit() throws Exception {
 		// A list containing a list containing a list containing a list...
-		String lists = "";
-		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++) lists += "60";
-		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++) lists += "80";
-		setContents(lists);
+		StringBuilder lists = new StringBuilder();
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++) lists.append("60");
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++) lists.append("80");
+		setContents(lists.toString());
 		r.readList();
 	}
 
 	@Test
 	public void testNestedDictionaryWithinDepthLimit() throws Exception {
 		// A dictionary containing a dictionary containing a dictionary...
-		String dicts = "";
-		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++)
-			dicts += "70" + "41" + "03" + "666F6F";
-		dicts += "11";
-		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++)
-			dicts += "80";
-		setContents(dicts);
+		StringBuilder dicts = new StringBuilder();
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++) {
+			dicts.append("70").append("41").append("03").append("666F6F");
+		}
+		dicts.append("11");
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT; i++) {
+			dicts.append("80");
+		}
+		setContents(dicts.toString());
 		r.readDictionary();
 		assertTrue(r.eof());
 	}
@@ -592,13 +663,15 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	@Test(expected = FormatException.class)
 	public void testNestedDictionaryOutsideDepthLimit() throws Exception {
 		// A dictionary containing a dictionary containing a dictionary...
-		String dicts = "";
-		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++)
-			dicts += "70" + "41" + "03" + "666F6F";
-		dicts += "11";
-		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++)
-			dicts += "80";
-		setContents(dicts);
+		StringBuilder dicts = new StringBuilder();
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++) {
+			dicts.append("70").append("41").append("03").append("666F6F");
+		}
+		dicts.append("11");
+		for (int i = 1; i <= DEFAULT_NESTED_LIMIT + 1; i++) {
+			dicts.append("80");
+		}
+		setContents(dicts.toString());
 		r.readDictionary();
 	}
 
@@ -625,6 +698,6 @@ public class BdfReaderImplTest extends BrambleTestCase {
 	private void setContents(String hex, int maxBufferSize)
 			throws FormatException {
 		ByteArrayInputStream in = new ByteArrayInputStream(fromHexString(hex));
-		r = new BdfReaderImpl(in, DEFAULT_NESTED_LIMIT, maxBufferSize);
+		r = new BdfReaderImpl(in, DEFAULT_NESTED_LIMIT, maxBufferSize, true);
 	}
 }

bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderWriterIntegrationTest.java

@@ -0,0 +1,80 @@
+package org.briarproject.bramble.data;
+
+import org.briarproject.bramble.api.data.BdfDictionary;
+import org.briarproject.bramble.api.data.BdfReader;
+import org.briarproject.bramble.api.data.BdfWriter;
+import org.briarproject.bramble.test.BrambleTestCase;
+import org.junit.Test;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.InputStream;
+import java.util.Iterator;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.Map.Entry;
+
+import static org.briarproject.bramble.api.data.BdfReader.DEFAULT_MAX_BUFFER_SIZE;
+import static org.briarproject.bramble.api.data.BdfReader.DEFAULT_NESTED_LIMIT;
+import static org.briarproject.bramble.util.StringUtils.fromHexString;
+import static org.briarproject.bramble.util.StringUtils.toHexString;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+
+public class BdfReaderWriterIntegrationTest extends BrambleTestCase {
+
+	@Test
+	public void testConvertStringToCanonicalForm() throws Exception {
+		// 'foo' as a STRING_16 (not canonical, should be a STRING_8)
+		String hexIn = "42" + "0003" + "666F6F";
+		InputStream in = new ByteArrayInputStream(fromHexString(hexIn));
+		BdfReader r = new BdfReaderImpl(in, DEFAULT_NESTED_LIMIT,
+				DEFAULT_MAX_BUFFER_SIZE, false); // Accept non-canonical
+		String s = r.readString();
+		assertEquals("foo", s);
+		assertTrue(r.eof());
+		// Convert the string back to BDF
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		BdfWriter w = new BdfWriterImpl(out);
+		w.writeString(s);
+		w.flush();
+		String hexOut = toHexString(out.toByteArray());
+		// The BDF should now be in canonical form
+		assertEquals("41" + "03" + "666F6F", hexOut);
+	}
+
+	@Test
+	public void testConvertDictionaryToCanonicalForm() throws Exception {
+		// A dictionary with keys in non-canonical order: 'foo' then 'bar'
+		String hexIn = "70" + "41" + "03" + "666F6F" + "21" + "01"
+				+ "41" + "03" + "626172" + "21" + "02" + "80";
+		InputStream in = new ByteArrayInputStream(fromHexString(hexIn));
+		BdfReader r = new BdfReaderImpl(in, DEFAULT_NESTED_LIMIT,
+				DEFAULT_MAX_BUFFER_SIZE, false); // Accept non-canonical
+		BdfDictionary d = r.readDictionary();
+		assertEquals(2, d.size());
+		assertTrue(r.eof());
+		// The entries should be returned in canonical order
+		Iterator<Entry<String, Object>> it = d.entrySet().iterator();
+		Entry<String, Object> first = it.next();
+		assertEquals("bar", first.getKey());
+		assertEquals(2L, first.getValue());
+		Entry<String, Object> second = it.next();
+		assertEquals("foo", second.getKey());
+		assertEquals(1L, second.getValue());
+
+		// Convert a non-canonical map to BDF (use LinkedHashMap so we know
+		// the entries will be iterated over in non-canonical order)
+		Map<String, Object> m = new LinkedHashMap<>();
+		m.put("foo", 1);
+		m.put("bar", 2);
+		ByteArrayOutputStream out = new ByteArrayOutputStream();
+		BdfWriter w = new BdfWriterImpl(out);
+		w.writeDictionary(m);
+		w.flush();
+		String hexOut = toHexString(out.toByteArray());
+		// The entries should be in canonical order: 'bar' then 'foo'
+		assertEquals("70" + "41" + "03" + "626172" + "21" + "02"
+				+ "41" + "03" + "666F6F" + "21" + "01" + "80", hexOut);
+	}
+}

bramble-core/src/test/java/org/briarproject/bramble/properties/TransportPropertyManagerImplTest.java

@@ -404,7 +404,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
 					localGroup.getId());
 			will(returnValue(messageMetadata));
-			oneOf(clientHelper).getMessageAsList(txn, fooUpdateId);
+			oneOf(clientHelper).getMessageAsList(txn, fooUpdateId, false);
 			will(returnValue(fooUpdate));
 			oneOf(clientHelper).parseAndValidateTransportProperties(
 					fooPropertiesDict);
@@ -471,7 +471,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
 					contactGroup2.getId());
 			will(returnValue(messageMetadata));
-			oneOf(clientHelper).getMessageAsList(txn, fooUpdateId);
+			oneOf(clientHelper).getMessageAsList(txn, fooUpdateId, false);
 			will(returnValue(fooUpdate));
 			oneOf(clientHelper).parseAndValidateTransportProperties(
 					fooPropertiesDict);
@@ -526,7 +526,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
 					contactGroup.getId());
 			will(returnValue(messageMetadata));
-			oneOf(clientHelper).getMessageAsList(txn, updateId);
+			oneOf(clientHelper).getMessageAsList(txn, updateId, false);
 			will(returnValue(update));
 			oneOf(clientHelper).parseAndValidateTransportProperties(
 					fooPropertiesDict);
@@ -564,7 +564,7 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
 					localGroup.getId());
 			will(returnValue(messageMetadata));
-			oneOf(clientHelper).getMessageAsList(txn, updateId);
+			oneOf(clientHelper).getMessageAsList(txn, updateId, false);
 			will(returnValue(update));
 			oneOf(clientHelper).parseAndValidateTransportProperties(
 					fooPropertiesDict);
@@ -695,7 +695,8 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
 					localGroup.getId());
 			will(returnValue(localGroupMessageMetadata));
-			oneOf(clientHelper).getMessageAsList(txn, localGroupUpdateId);
+			oneOf(clientHelper).getMessageAsList(txn, localGroupUpdateId,
+					false);
 			will(returnValue(oldUpdate));
 			oneOf(clientHelper).parseAndValidateTransportProperties(
 					oldPropertiesDict);
@@ -760,7 +761,8 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 			oneOf(clientHelper).getMessageMetadataAsDictionary(txn,
 					localGroup.getId());
 			will(returnValue(localGroupMessageMetadata));
-			oneOf(clientHelper).getMessageAsList(txn, localGroupUpdateId);
+			oneOf(clientHelper).getMessageAsList(txn, localGroupUpdateId,
+					false);
 			will(returnValue(oldUpdate));
 			oneOf(clientHelper).parseAndValidateTransportProperties(
 					oldPropertiesDict);
@@ -819,12 +821,12 @@ public class TransportPropertyManagerImplTest extends BrambleMockTestCase {
 					localGroup.getId());
 			will(returnValue(messageMetadata));
 			// Retrieve and parse the latest local properties
-			oneOf(clientHelper).getMessageAsList(txn, fooVersion999);
+			oneOf(clientHelper).getMessageAsList(txn, fooVersion999, false);
 			will(returnValue(fooUpdate));
 			oneOf(clientHelper).parseAndValidateTransportProperties(
 					fooPropertiesDict);
 			will(returnValue(fooProperties));
-			oneOf(clientHelper).getMessageAsList(txn, barVersion3);
+			oneOf(clientHelper).getMessageAsList(txn, barVersion3, false);
 			will(returnValue(barUpdate));
 			oneOf(clientHelper).parseAndValidateTransportProperties(
 					barPropertiesDict);