diff --git a/briar-api/src/org/briarproject/api/clients/MessageQueueManager.java b/briar-api/src/org/briarproject/api/clients/MessageQueueManager.java
index 1d989d75fbee8feef3e6f2f9619b5341f261db84..259e900fbb59866f04a50843eb31864ee3e030bf 100644
--- a/briar-api/src/org/briarproject/api/clients/MessageQueueManager.java
+++ b/briar-api/src/org/briarproject/api/clients/MessageQueueManager.java
@@ -48,8 +48,17 @@ 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
+ * 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
*/
void incomingMessage(Transaction txn, QueueMessage q, Metadata meta)
- throws DbException;
+ throws DbException, InvalidMessageException;
}
}
diff --git a/briar-api/src/org/briarproject/api/sync/ValidationManager.java b/briar-api/src/org/briarproject/api/sync/ValidationManager.java
index 8fe76152211a742a413ec02dc97e18dceac90b16..6d1960b25d4aa73b07c98611d016dbf59cbd228e 100644
--- a/briar-api/src/org/briarproject/api/sync/ValidationManager.java
+++ b/briar-api/src/org/briarproject/api/sync/ValidationManager.java
@@ -55,9 +55,20 @@ 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 InvalidMessageException 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).
+ * Throwing this will delete the incoming message and its metadata
+ * marking it as invalid in the database.
+ * Never rethrow DbException as InvalidMessageException
*/
boolean incomingMessage(Transaction txn, Message m, Metadata meta)
- throws DbException;
+ throws DbException, InvalidMessageException;
}
}
diff --git a/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java b/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java
index ce34fd58fabdf4907bb35b812aa0af2c90849d4b..455e17c2ff892bf44dcdc8386cfe5db36aa8cc09 100644
--- a/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java
+++ b/briar-core/src/org/briarproject/blogs/BlogManagerImpl.java
@@ -48,7 +48,6 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.logging.Logger;
import javax.inject.Inject;
@@ -75,9 +74,6 @@ import static org.briarproject.blogs.BlogPostValidator.authorToBdfDictionary;
class BlogManagerImpl extends BdfIncomingMessageHook implements BlogManager,
AddContactHook, RemoveContactHook, Client {
- private static final Logger LOG =
- Logger.getLogger(BlogManagerImpl.class.getName());
-
static final ClientId CLIENT_ID = new ClientId(StringUtils.fromHexString(
"dafbe56f0c8971365cea4bb5f08ec9a6" +
"1d686e058b943997b6ff259ba423f613"));
diff --git a/briar-core/src/org/briarproject/clients/BdfIncomingMessageHook.java b/briar-core/src/org/briarproject/clients/BdfIncomingMessageHook.java
index 394b031bc02b46dfc0209c41be8c6945545b18d5..e09a305d8b487cd27e475290a6bc50aa23938926 100644
--- a/briar-core/src/org/briarproject/clients/BdfIncomingMessageHook.java
+++ b/briar-core/src/org/briarproject/clients/BdfIncomingMessageHook.java
@@ -14,6 +14,7 @@ import org.briarproject.api.db.DbException;
import org.briarproject.api.db.Metadata;
import org.briarproject.api.db.Transaction;
import org.briarproject.api.sync.GroupId;
+import org.briarproject.api.sync.InvalidMessageException;
import org.briarproject.api.sync.Message;
import org.briarproject.api.sync.MessageId;
import org.briarproject.api.sync.ValidationManager.IncomingMessageHook;
@@ -39,24 +40,39 @@ public abstract class BdfIncomingMessageHook implements IncomingMessageHook,
this.metadataParser = metadataParser;
}
+ /**
+ * Called once for each incoming message that passes validation.
+ *
+ * @throws DbException Should only be used for real database errors.
+ * Do not rethrow
+ * @throws FormatException Use this 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).
+ * Throwing this will delete the incoming message and its metadata
+ * marking it as invalid in the database.
+ * Never rethrow DbException as FormatException
+ */
protected abstract boolean incomingMessage(Transaction txn, Message m,
BdfList body, BdfDictionary meta) throws DbException,
FormatException;
@Override
public boolean incomingMessage(Transaction txn, Message m, Metadata meta)
- throws DbException {
+ throws DbException, InvalidMessageException {
return incomingMessage(txn, m, meta, MESSAGE_HEADER_LENGTH);
}
@Override
public void incomingMessage(Transaction txn, QueueMessage q, Metadata meta)
- throws DbException {
+ throws DbException, InvalidMessageException {
incomingMessage(txn, q, meta, QUEUE_MESSAGE_HEADER_LENGTH);
}
private boolean incomingMessage(Transaction txn, Message m, Metadata meta,
- int headerLength) throws DbException {
+ int headerLength) throws DbException, InvalidMessageException {
try {
byte[] raw = m.getRaw();
BdfList body = clientHelper.toList(raw, headerLength,
@@ -64,7 +80,7 @@ public abstract class BdfIncomingMessageHook implements IncomingMessageHook,
BdfDictionary metaDictionary = metadataParser.parse(meta);
return incomingMessage(txn, m, body, metaDictionary);
} catch (FormatException e) {
- throw new DbException(e);
+ throw new InvalidMessageException(e);
}
}
diff --git a/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java b/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java
index ad1fffe3b50a7151c6f462e930bb590994762d01..c1ceef543f5dd654c94fd9d154a1d9b0d8478f41 100644
--- a/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java
+++ b/briar-core/src/org/briarproject/clients/MessageQueueManagerImpl.java
@@ -132,7 +132,7 @@ class MessageQueueManagerImpl implements MessageQueueManager {
private long outgoingPosition, incomingPosition;
private final TreeMap<Long, MessageId> pending;
- QueueState(long outgoingPosition, long incomingPosition,
+ private QueueState(long outgoingPosition, long incomingPosition,
TreeMap<Long, MessageId> pending) {
this.outgoingPosition = outgoingPosition;
this.incomingPosition = incomingPosition;
@@ -166,7 +166,7 @@ class MessageQueueManagerImpl implements MessageQueueManager {
private final QueueMessageValidator delegate;
- DelegatingMessageValidator(QueueMessageValidator delegate) {
+ private DelegatingMessageValidator(QueueMessageValidator delegate) {
this.delegate = delegate;
}
@@ -193,8 +193,8 @@ class MessageQueueManagerImpl implements MessageQueueManager {
}
@Override
- public boolean incomingMessage(Transaction txn, Message m, Metadata meta)
- throws DbException {
+ public boolean incomingMessage(Transaction txn, Message m,
+ Metadata meta) throws DbException, InvalidMessageException {
long queuePosition = ByteUtils.readUint64(m.getRaw(),
MESSAGE_HEADER_LENGTH);
QueueState queueState = loadQueueState(txn, m.getGroupId());
diff --git a/briar-core/src/org/briarproject/forum/ForumManagerImpl.java b/briar-core/src/org/briarproject/forum/ForumManagerImpl.java
index f173fafc62a22048bc6debc4c30841302eb3a707..8cd1d9f0eee072d0bb8c59a65d02eee21609e6a5 100644
--- a/briar-core/src/org/briarproject/forum/ForumManagerImpl.java
+++ b/briar-core/src/org/briarproject/forum/ForumManagerImpl.java
@@ -148,14 +148,12 @@ class ForumManagerImpl extends BdfIncomingMessageHook implements ForumManager {
BdfDictionary meta = new BdfDictionary();
meta.put(KEY_TIMESTAMP, p.getMessage().getTimestamp());
if (p.getParent() != null) meta.put(KEY_PARENT, p.getParent());
- if (p.getAuthor() != null) {
- Author a = p.getAuthor();
- BdfDictionary authorMeta = new BdfDictionary();
- authorMeta.put(KEY_ID, a.getId());
- authorMeta.put(KEY_NAME, a.getName());
- authorMeta.put(KEY_PUBLIC_NAME, a.getPublicKey());
- meta.put(KEY_AUTHOR, authorMeta);
- }
+ Author a = p.getAuthor();
+ BdfDictionary authorMeta = new BdfDictionary();
+ authorMeta.put(KEY_ID, a.getId());
+ authorMeta.put(KEY_NAME, a.getName());
+ authorMeta.put(KEY_PUBLIC_NAME, a.getPublicKey());
+ meta.put(KEY_AUTHOR, authorMeta);
meta.put(KEY_LOCAL, true);
meta.put(MSG_KEY_READ, true);
clientHelper.addLocalMessage(txn, p.getMessage(), meta, true);
diff --git a/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java b/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java
index 95a72aed8eaccf2dfa876f7aa0f59f7b373b0bd0..78a6e861e7af7a733523bc5bcddf33d73b886d53 100644
--- a/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java
+++ b/briar-core/src/org/briarproject/introduction/IntroductionManagerImpl.java
@@ -32,7 +32,6 @@ import org.briarproject.api.sync.MessageStatus;
import org.briarproject.clients.ConversationClientImpl;
import org.briarproject.util.StringUtils;
-import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
@@ -206,7 +205,7 @@ class IntroductionManagerImpl extends ConversationClientImpl
*/
@Override
protected boolean incomingMessage(Transaction txn, Message m, BdfList body,
- BdfDictionary message) throws DbException {
+ BdfDictionary message) throws DbException, FormatException {
// Get message data and type
GroupId groupId = m.getGroupId();
@@ -220,19 +219,9 @@ class IntroductionManagerImpl extends ConversationClientImpl
} catch (FormatException e) {
stateExists = false;
}
- BdfDictionary state;
- try {
- if (stateExists) throw new FormatException();
- state = introduceeManager.initialize(txn, groupId, message);
- } catch (FormatException e) {
- if (LOG.isLoggable(WARNING)) {
- LOG.warning(
- "Could not initialize introducee state, deleting...");
- LOG.log(WARNING, e.toString(), e);
- }
- deleteMessage(txn, m.getId());
- return false;
- }
+ if (stateExists) throw new FormatException();
+ BdfDictionary state =
+ introduceeManager.initialize(txn, groupId, message);
try {
introduceeManager.incomingMessage(txn, state, message);
trackIncomingMessage(txn, m);
@@ -240,21 +229,15 @@ class IntroductionManagerImpl extends ConversationClientImpl
if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
introduceeManager.abort(txn, state);
} catch (FormatException e) {
+ // FIXME necessary?
if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
introduceeManager.abort(txn, state);
}
}
// our role can be anything
else if (type == TYPE_RESPONSE || type == TYPE_ACK || type == TYPE_ABORT) {
- BdfDictionary state;
- try {
- state = getSessionState(txn, groupId,
- message.getRaw(SESSION_ID));
- } catch (FormatException e) {
- LOG.warning("Could not find state for message, deleting...");
- deleteMessage(txn, m.getId());
- return false;
- }
+ BdfDictionary state =
+ getSessionState(txn, groupId, message.getRaw(SESSION_ID));
long role = state.getLong(ROLE, -1L);
try {
@@ -263,18 +246,15 @@ class IntroductionManagerImpl extends ConversationClientImpl
} else if (role == ROLE_INTRODUCEE) {
introduceeManager.incomingMessage(txn, state, message);
} else {
- if(LOG.isLoggable(WARNING)) {
- LOG.warning("Unknown role '" + role +
- "'. Deleting message...");
- deleteMessage(txn, m.getId());
- }
+ throw new AssertionError("Unknown role '" + role + "'");
}
if (type == TYPE_RESPONSE) trackIncomingMessage(txn, m);
} catch (DbException e) {
if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
if (role == ROLE_INTRODUCER) introducerManager.abort(txn, state);
else introduceeManager.abort(txn, state);
- } catch (IOException e) {
+ } catch (FormatException e) {
+ // FIXME necessary?
if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
if (role == ROLE_INTRODUCER) introducerManager.abort(txn, state);
else introduceeManager.abort(txn, state);
diff --git a/briar-core/src/org/briarproject/privategroup/PrivateGroupManagerImpl.java b/briar-core/src/org/briarproject/privategroup/PrivateGroupManagerImpl.java
index db4a73dc3786edd8e1ebc256c5fbf0307faa87ce..4b2ab02666881033dd666eb1d6b491d39f204277 100644
--- a/briar-core/src/org/briarproject/privategroup/PrivateGroupManagerImpl.java
+++ b/briar-core/src/org/briarproject/privategroup/PrivateGroupManagerImpl.java
@@ -41,7 +41,6 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.logging.Logger;
import javax.inject.Inject;
@@ -65,8 +64,6 @@ import static org.briarproject.privategroup.Constants.KEY_TYPE;
public class PrivateGroupManagerImpl extends BdfIncomingMessageHook implements
PrivateGroupManager {
- private static final Logger LOG =
- Logger.getLogger(PrivateGroupManagerImpl.class.getName());
static final ClientId CLIENT_ID = new ClientId(
StringUtils.fromHexString("5072697661746547726f75704d616e61"
+ "67657220627920546f727374656e2047"));
@@ -198,7 +195,8 @@ public class PrivateGroupManagerImpl extends BdfIncomingMessageHook implements
try {
BdfDictionary meta = new BdfDictionary();
meta.put(KEY_TYPE, POST.getInt());
- if (m.getParent() != null) meta.put(KEY_PARENT_MSG_ID, m.getParent());
+ if (m.getParent() != null)
+ meta.put(KEY_PARENT_MSG_ID, m.getParent());
addMessageMetadata(meta, m, true);
clientHelper.addLocalMessage(txn, m.getMessage(), meta, true);
setPreviousMsgId(txn, m.getMessage().getGroupId(),
@@ -429,44 +427,29 @@ public class PrivateGroupManagerImpl extends BdfIncomingMessageHook implements
MessageId parentId = new MessageId(parentIdBytes);
BdfDictionary parentMeta = clientHelper
.getMessageMetadataAsDictionary(txn, parentId);
- if (timestamp <= parentMeta.getLong(KEY_TIMESTAMP)) {
- // FIXME throw new InvalidMessageException() (#643)
- db.deleteMessage(txn, m.getId());
- return false;
- }
+ if (timestamp <= parentMeta.getLong(KEY_TIMESTAMP))
+ throw new FormatException();
MessageType parentType = MessageType
.valueOf(parentMeta.getLong(KEY_TYPE).intValue());
- if (parentType != POST) {
- // FIXME throw new InvalidMessageException() (#643)
- db.deleteMessage(txn, m.getId());
- return false;
- }
+ if (parentType != POST)
+ throw new FormatException();
}
// and the member's previous message
byte[] previousMsgIdBytes = meta.getRaw(KEY_PREVIOUS_MSG_ID);
MessageId previousMsgId = new MessageId(previousMsgIdBytes);
BdfDictionary previousMeta = clientHelper
.getMessageMetadataAsDictionary(txn, previousMsgId);
- if (timestamp <= previousMeta.getLong(KEY_TIMESTAMP)) {
- // FIXME throw new InvalidMessageException() (#643)
- db.deleteMessage(txn, m.getId());
- return false;
- }
+ if (timestamp <= previousMeta.getLong(KEY_TIMESTAMP))
+ throw new FormatException();
// previous message must be from same member
if (!Arrays.equals(meta.getRaw(KEY_MEMBER_ID),
- previousMeta.getRaw(KEY_MEMBER_ID))) {
- // FIXME throw new InvalidMessageException() (#643)
- db.deleteMessage(txn, m.getId());
- return false;
- }
+ previousMeta.getRaw(KEY_MEMBER_ID)))
+ throw new FormatException();
// previous message must be a POST or JOIN
MessageType previousType = MessageType
.valueOf(previousMeta.getLong(KEY_TYPE).intValue());
- if (previousType != JOIN && previousType != POST) {
- // FIXME throw new InvalidMessageException() (#643)
- db.deleteMessage(txn, m.getId());
- return false;
- }
+ if (previousType != JOIN && previousType != POST)
+ throw new FormatException();
trackIncomingMessage(txn, m);
return true;
default:
diff --git a/briar-core/src/org/briarproject/sharing/SharingManagerImpl.java b/briar-core/src/org/briarproject/sharing/SharingManagerImpl.java
index 9699f3a34fc6bab65dfba3a2ff75a37e03316e15..32591209f4531e125fdda0bfc71b31e7bfcafdd1 100644
--- a/briar-core/src/org/briarproject/sharing/SharingManagerImpl.java
+++ b/briar-core/src/org/briarproject/sharing/SharingManagerImpl.java
@@ -205,30 +205,25 @@ abstract class SharingManagerImpl<S extends Shareable, I extends Invitation, IS
// this is what we would expect under normal circumstances
stateExists = false;
}
- try {
- // check if we already have a state with that sessionId
- if (stateExists) throw new FormatException();
-
- // check if shareable can be shared
- I invitation = (I) msg;
- S f = getSFactory().parse(invitation);
- ContactId contactId = getContactId(txn, m.getGroupId());
- Contact contact = db.getContact(txn, contactId);
- if (!canBeShared(txn, f.getId(), contact))
- checkForRaceCondition(txn, f, contact);
-
- // initialize state and process invitation
- IS state = initializeInviteeState(txn, contactId, invitation,
- m.getId());
- InviteeEngine<IS, IR> engine =
- new InviteeEngine<IS, IR>(getIRFactory(), clock);
- processInviteeStateUpdate(txn, m.getId(),
- engine.onMessageReceived(state, msg));
- trackIncomingMessage(txn, m);
- } catch (FormatException e) {
- if (LOG.isLoggable(WARNING)) LOG.log(WARNING, e.toString(), e);
- deleteMessage(txn, m.getId());
- }
+ // check if we already have a state with that sessionId
+ if (stateExists) throw new FormatException();
+
+ // check if shareable can be shared
+ I invitation = (I) msg;
+ S f = getSFactory().parse(invitation);
+ ContactId contactId = getContactId(txn, m.getGroupId());
+ Contact contact = db.getContact(txn, contactId);
+ if (!canBeShared(txn, f.getId(), contact))
+ checkForRaceCondition(txn, f, contact);
+
+ // initialize state and process invitation
+ IS state = initializeInviteeState(txn, contactId, invitation,
+ m.getId());
+ InviteeEngine<IS, IR> engine =
+ new InviteeEngine<IS, IR>(getIRFactory(), clock);
+ processInviteeStateUpdate(txn, m.getId(),
+ engine.onMessageReceived(state, msg));
+ trackIncomingMessage(txn, m);
} else if (msg.getType() == SHARE_MSG_TYPE_ACCEPT ||
msg.getType() == SHARE_MSG_TYPE_DECLINE) {
// we are a sharer who just received a response
@@ -262,7 +257,7 @@ abstract class SharingManagerImpl<S extends Shareable, I extends Invitation, IS
}
} else {
// message has passed validator, so that should never happen
- throw new RuntimeException("Illegal Sharing Message");
+ throw new AssertionError("Illegal Sharing Message");
}
// don't share message as other party already has it
return false;
diff --git a/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java b/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java
index 5bebd3009f618c7687025dd4103db97217478a69..babfac757678672bf6715d6dd2d7384298e0124c 100644
--- a/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java
+++ b/briar-core/src/org/briarproject/sync/ValidationManagerImpl.java
@@ -360,16 +360,18 @@ class ValidationManagerImpl implements ValidationManager, Service,
boolean shareMsg = false;
IncomingMessageHook hook = hooks.get(c);
if (hook != null) {
- shareMsg = hook.incomingMessage(txn, m, meta);
- }
- // TODO: Find a better way for clients to signal validity, #643
- if (db.getRawMessage(txn, m.getId()) == null) {
- db.setMessageState(txn, m.getId(), INVALID);
- return new DeliveryResult(false, false);
- } else {
- db.setMessageState(txn, m.getId(), DELIVERED);
- return new DeliveryResult(true, shareMsg);
+ try {
+ shareMsg = hook.incomingMessage(txn, m, meta);
+ } catch (InvalidMessageException e) {
+ // message is invalid, mark it as such and delete it
+ db.setMessageState(txn, m.getId(), INVALID);
+ db.deleteMessageMetadata(txn, m.getId());
+ db.deleteMessage(txn, m.getId());
+ return new DeliveryResult(false, false);
+ }
}
+ db.setMessageState(txn, m.getId(), DELIVERED);
+ return new DeliveryResult(true, shareMsg);
}
private Queue<MessageId> getPendingDependents(Transaction txn, MessageId m)