diff --git a/briar-api/src/org/briarproject/api/clients/MessageQueueManager.java b/briar-api/src/org/briarproject/api/clients/MessageQueueManager.java index 259e900fbb59866f04a50843eb31864ee3e030bf..b5b49ad16adb1a06e3525ae94cfca59928325a39 100644 --- a/briar-api/src/org/briarproject/api/clients/MessageQueueManager.java +++ b/briar-api/src/org/briarproject/api/clients/MessageQueueManager.java @@ -1,5 +1,6 @@ package org.briarproject.api.clients; +import org.briarproject.api.FormatException; import org.briarproject.api.db.DbException; import org.briarproject.api.db.Metadata; import org.briarproject.api.db.Transaction; @@ -49,16 +50,19 @@ public interface MessageQueueManager { * Called once for each incoming message that passes validation. * Messages are passed to the hook in order. * - * @throws DbException should only be used for real database errors - * @throws InvalidMessageException for any non-database error + * @throws DbException Should only be used for real database errors. + * If this is thrown, delivery will be attempted again at next startup, + * whereas if an FormatException is thrown, + * the message will be permanently invalidated. + * @throws FormatException for any non-database error * that occurs while handling remotely created data. * This includes errors that occur while handling locally created data * in a context controlled by remotely created data * (for example, parsing the metadata of a dependency * of an incoming message). - * Never rethrow DbException as InvalidMessageException + * Never rethrow DbException as FormatException! */ void incomingMessage(Transaction txn, QueueMessage q, Metadata meta) - throws DbException, InvalidMessageException; + throws DbException, FormatException; } } diff --git a/briar-api/src/org/briarproject/api/sync/ValidationManager.java b/briar-api/src/org/briarproject/api/sync/ValidationManager.java index 6d1960b25d4aa73b07c98611d016dbf59cbd228e..911ba4771831f7b718ab2329292d690fb280af90 100644 --- a/briar-api/src/org/briarproject/api/sync/ValidationManager.java +++ b/briar-api/src/org/briarproject/api/sync/ValidationManager.java @@ -57,7 +57,10 @@ public interface ValidationManager { * Called once for each incoming message that passes validation. * * @return whether or not this message should be shared - * @throws DbException should only be used for real database errors + * @throws DbException Should only be used for real database errors. + * If this is thrown, delivery will be attempted again at next startup, + * whereas if an InvalidMessageException is thrown, + * the message will be permanently invalidated. * @throws InvalidMessageException for any non-database error * that occurs while handling remotely created data. * This includes errors that occur while handling locally created data @@ -66,7 +69,7 @@ public interface ValidationManager { * of an incoming message). * Throwing this will delete the incoming message and its metadata * marking it as invalid in the database. - * Never rethrow DbException as InvalidMessageException + * Never rethrow DbException as InvalidMessageException! */ boolean incomingMessage(Transaction txn, Message m, Metadata meta) throws DbException, InvalidMessageException; diff --git a/briar-core/src/org/briarproject/clients/BdfIncomingMessageHook.java b/briar-core/src/org/briarproject/clients/BdfIncomingMessageHook.java index e09a305d8b487cd27e475290a6bc50aa23938926..3ddbad32004804ece34eefab5f364a64f28f71e6 100644 --- a/briar-core/src/org/briarproject/clients/BdfIncomingMessageHook.java +++ b/briar-core/src/org/briarproject/clients/BdfIncomingMessageHook.java @@ -62,26 +62,26 @@ public abstract class BdfIncomingMessageHook implements IncomingMessageHook, @Override public boolean incomingMessage(Transaction txn, Message m, Metadata meta) throws DbException, InvalidMessageException { - return incomingMessage(txn, m, meta, MESSAGE_HEADER_LENGTH); + try { + return incomingMessage(txn, m, meta, MESSAGE_HEADER_LENGTH); + } catch (FormatException e) { + throw new InvalidMessageException(e); + } } @Override public void incomingMessage(Transaction txn, QueueMessage q, Metadata meta) - throws DbException, InvalidMessageException { + throws DbException, FormatException { incomingMessage(txn, q, meta, QUEUE_MESSAGE_HEADER_LENGTH); } private boolean incomingMessage(Transaction txn, Message m, Metadata meta, - int headerLength) throws DbException, InvalidMessageException { - try { - byte[] raw = m.getRaw(); - BdfList body = clientHelper.toList(raw, headerLength, - raw.length - headerLength); - BdfDictionary metaDictionary = metadataParser.parse(meta); - return incomingMessage(txn, m, body, metaDictionary); - } catch (FormatException e) { - throw new InvalidMessageException(e); - } + int headerLength) throws DbException, FormatException { + byte[] raw = m.getRaw(); + BdfList body = clientHelper.toList(raw, headerLength, + raw.length - headerLength); + BdfDictionary metaDictionary = metadataParser.parse(meta); + return incomingMessage(txn, m, body, metaDictionary); } protected void trackIncomingMessage(Transaction txn, Message m) diff --git a/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java b/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java index c1ceef543f5dd654c94fd9d154a1d9b0d8478f41..bcb9ff72dbfa38b20df92a3c57462af16669ba2f 100644 --- a/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java +++ b/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java @@ -227,16 +227,20 @@ class MessageQueueManagerImpl implements MessageQueueManager { // Save the queue state before passing control to the delegate saveQueueState(txn, m.getGroupId(), queueState); // Deliver the messages to the delegate - delegate.incomingMessage(txn, q, meta); - for (MessageId id : consecutive) { - byte[] raw = db.getRawMessage(txn, id); - meta = db.getMessageMetadata(txn, id); - q = queueMessageFactory.createMessage(id, raw); - if (LOG.isLoggable(INFO)) { - LOG.info("Delivering pending message with position " - + q.getQueuePosition()); - } + try { delegate.incomingMessage(txn, q, meta); + for (MessageId id : consecutive) { + byte[] raw = db.getRawMessage(txn, id); + meta = db.getMessageMetadata(txn, id); + q = queueMessageFactory.createMessage(id, raw); + if (LOG.isLoggable(INFO)) { + LOG.info("Delivering pending message with position " + + q.getQueuePosition()); + } + delegate.incomingMessage(txn, q, meta); + } + } catch (FormatException e) { + throw new InvalidMessageException(e); } } // message queues are only useful for groups with two members diff --git a/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java b/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java index 78a6e861e7af7a733523bc5bcddf33d73b886d53..7c3e2361f58ca3e4559cc975c298fa0420ca1af7 100644 --- a/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java +++ b/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java @@ -246,7 +246,9 @@ class IntroductionManagerImpl extends ConversationClientImpl } else if (role == ROLE_INTRODUCEE) { introduceeManager.incomingMessage(txn, state, message); } else { - throw new AssertionError("Unknown role '" + role + "'"); + if (LOG.isLoggable(WARNING)) + LOG.warning("Unknown role '" + role + "'"); + throw new DbException(); } if (type == TYPE_RESPONSE) trackIncomingMessage(txn, m); } catch (DbException e) { diff --git a/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java b/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java index dea4ab979948b6fd24a236c8f8a647c9281381f6..edfc7154847521626cf96da7c9899c5fbc98aa6c 100644 --- a/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java +++ b/briar-tests/src/org/briarproject/sync/ValidationManagerImplTest.java @@ -110,8 +110,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver the first message oneOf(hook).incomingMessage(txn2, message, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn2, messageId); - will(returnValue(raw)); oneOf(db).setMessageState(txn2, messageId, DELIVERED); // Get any pending dependents oneOf(db).getMessageDependents(txn2, messageId); @@ -215,8 +213,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver the message oneOf(hook).incomingMessage(txn2, message, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn2, messageId); - will(returnValue(raw)); oneOf(db).setMessageState(txn2, messageId, DELIVERED); // Get any pending dependents oneOf(db).getMessageDependents(txn2, messageId); @@ -240,8 +236,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver the dependent oneOf(hook).incomingMessage(txn3, message2, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn3, messageId2); - will(returnValue(raw)); oneOf(db).setMessageState(txn3, messageId2, DELIVERED); // Get any pending dependents oneOf(db).getMessageDependents(txn3, messageId2); @@ -366,8 +360,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver the message oneOf(hook).incomingMessage(txn1, message, metadata); will(returnValue(true)); - oneOf(db).getRawMessage(txn1, messageId); - will(returnValue(raw)); oneOf(db).setMessageState(txn1, messageId, DELIVERED); // Get any pending dependents oneOf(db).getMessageDependents(txn1, messageId); @@ -589,8 +581,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver the message oneOf(hook).incomingMessage(txn1, message, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn1, messageId); - will(returnValue(raw)); oneOf(db).setMessageState(txn1, messageId, DELIVERED); // Get any pending dependents oneOf(db).getMessageDependents(txn1, messageId); @@ -707,8 +697,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver the message oneOf(hook).incomingMessage(txn1, message, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn1, messageId); - will(returnValue(raw)); oneOf(db).setMessageState(txn1, messageId, DELIVERED); // Get any pending dependents oneOf(db).getMessageDependents(txn1, messageId); @@ -953,8 +941,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver the message oneOf(hook).incomingMessage(txn1, message, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn1, messageId); - will(returnValue(raw)); oneOf(db).setMessageState(txn1, messageId, DELIVERED); // The message has two pending dependents: 1 and 2 oneOf(db).getMessageDependents(txn1, messageId); @@ -978,8 +964,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver message 1 oneOf(hook).incomingMessage(txn2, message1, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn2, messageId1); - will(returnValue(raw)); oneOf(db).setMessageState(txn2, messageId1, DELIVERED); // Message 1 has one pending dependent: 3 oneOf(db).getMessageDependents(txn2, messageId1); @@ -1003,8 +987,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver message 2 oneOf(hook).incomingMessage(txn3, message2, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn3, messageId2); - will(returnValue(raw)); oneOf(db).setMessageState(txn3, messageId2, DELIVERED); // Message 2 has one pending dependent: 3 (same dependent as 1) oneOf(db).getMessageDependents(txn3, messageId2); @@ -1027,8 +1009,6 @@ public class ValidationManagerImplTest extends BriarTestCase { will(returnValue(metadata)); // Deliver message 3 oneOf(hook).incomingMessage(txn4, message3, metadata); - oneOf(db).getRawMessage(txn4, messageId3); - will(returnValue(raw)); oneOf(db).setMessageState(txn4, messageId3, DELIVERED); // Message 3 has one pending dependent: 4 oneOf(db).getMessageDependents(txn4, messageId3); @@ -1059,8 +1039,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver message 4 oneOf(hook).incomingMessage(txn6, message4, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn6, messageId4); - will(returnValue(raw)); oneOf(db).setMessageState(txn6, messageId4, DELIVERED); // Message 4 has no pending dependents oneOf(db).getMessageDependents(txn6, messageId4); @@ -1111,8 +1089,6 @@ public class ValidationManagerImplTest extends BriarTestCase { // Deliver the message oneOf(hook).incomingMessage(txn1, message, metadata); will(returnValue(false)); - oneOf(db).getRawMessage(txn1, messageId); - will(returnValue(raw)); oneOf(db).setMessageState(txn1, messageId, DELIVERED); // Get any pending dependents oneOf(db).getMessageDependents(txn1, messageId);