From ac8a4db457094e85ee1f73eb12c5c5c30bea3980 Mon Sep 17 00:00:00 2001 From: akwizgran Date: Fri, 17 Feb 2023 22:51:49 +0000 Subject: [PATCH] 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. --- .../api/client/BdfMessageValidator.java | 14 +- .../bramble/api/client/ClientHelper.java | 5 + .../bramble/api/data/BdfReaderFactory.java | 4 +- .../bramble/client/ClientHelperImpl.java | 23 ++- .../bramble/data/BdfReaderFactoryImpl.java | 12 +- .../bramble/data/BdfReaderImpl.java | 33 +++- .../bramble/data/BdfWriterImpl.java | 24 ++- .../TransportPropertyManagerImpl.java | 12 +- .../TransportPropertyValidator.java | 5 +- .../client/BdfMessageValidatorTest.java | 8 +- .../bramble/client/ClientHelperImplTest.java | 2 +- .../data/BdfReaderImplFuzzingTest.java | 2 +- .../bramble/data/BdfReaderImplTest.java | 147 +++++++++++++----- .../data/BdfReaderWriterIntegrationTest.java | 80 ++++++++++ .../TransportPropertyManagerImplTest.java | 18 ++- 15 files changed, 309 insertions(+), 80 deletions(-) create mode 100644 bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderWriterIntegrationTest.java diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/client/BdfMessageValidator.java b/bramble-api/src/main/java/org/briarproject/bramble/api/client/BdfMessageValidator.java index 4caed6438..468c64126 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/client/BdfMessageValidator.java +++ b/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()); diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java b/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java index 9ccd18f1d..ff8ee773c 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/client/ClientHelper.java +++ b/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) diff --git a/bramble-api/src/main/java/org/briarproject/bramble/api/data/BdfReaderFactory.java b/bramble-api/src/main/java/org/briarproject/bramble/api/data/BdfReaderFactory.java index bc41a66d3..5ff95dfa1 100644 --- a/bramble-api/src/main/java/org/briarproject/bramble/api/data/BdfReaderFactory.java +++ b/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); } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java index 1e1939cd8..30d7d641b 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/client/ClientHelperImpl.java +++ b/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()); diff --git a/bramble-core/src/main/java/org/briarproject/bramble/data/BdfReaderFactoryImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/data/BdfReaderFactoryImpl.java index 48d61ca63..a106b1d1c 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/data/BdfReaderFactoryImpl.java +++ b/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); } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/data/BdfReaderImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/data/BdfReaderImpl.java index d5badb88e..71734ec62 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/data/BdfReaderImpl.java +++ b/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; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/data/BdfWriterImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/data/BdfWriterImpl.java index aa862e572..54cc732ac 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/data/BdfWriterImpl.java +++ b/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 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); } 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 7f17fed48..e1641d6e9 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 @@ -201,7 +201,7 @@ class TransportPropertyManagerImpl implements TransportPropertyManager, // Retrieve and parse the latest local properties for (Entry 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 e : p.entrySet()) { diff --git a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyValidator.java b/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyValidator.java index d0dd2b91b..6a4c0abb4 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/properties/TransportPropertyValidator.java +++ b/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 diff --git a/bramble-core/src/test/java/org/briarproject/bramble/client/BdfMessageValidatorTest.java b/bramble-core/src/test/java/org/briarproject/bramble/client/BdfMessageValidatorTest.java index a6b93c6cc..6794367df 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/client/BdfMessageValidatorTest.java +++ b/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)); }}); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/client/ClientHelperImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/client/ClientHelperImplTest.java index 7c39128a1..a4f8149f1 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/client/ClientHelperImplTest.java +++ b/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)); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderImplFuzzingTest.java b/bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderImplFuzzingTest.java index f83a6bc6f..fcbf6bfb9 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderImplFuzzingTest.java +++ b/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); diff --git a/bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderImplTest.java b/bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderImplTest.java index f59584b57..ebd0af441 100644 --- a/bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderImplTest.java +++ b/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); } } diff --git a/bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderWriterIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/data/BdfReaderWriterIntegrationTest.java new file mode 100644 index 000000000..777aa2daf --- /dev/null +++ b/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> it = d.entrySet().iterator(); + Entry first = it.next(); + assertEquals("bar", first.getKey()); + assertEquals(2L, first.getValue()); + Entry 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 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); + } +} 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 7c7396f6e..27251be97 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 @@ -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);