diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/BlogProtocolEngineImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/BlogProtocolEngineImpl.java index aa279be427e7f03735917568679e7d9148dabc0c..3083387b6bf1f76f393b6dc6d2066c67f32e8e77 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/BlogProtocolEngineImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/BlogProtocolEngineImpl.java @@ -14,7 +14,6 @@ import org.briarproject.bramble.api.system.Clock; import org.briarproject.briar.api.blog.Blog; import org.briarproject.briar.api.blog.BlogInvitationResponse; import org.briarproject.briar.api.blog.BlogManager; -import org.briarproject.briar.api.blog.BlogSharingManager; import org.briarproject.briar.api.blog.event.BlogInvitationRequestReceivedEvent; import org.briarproject.briar.api.blog.event.BlogInvitationResponseReceivedEvent; import org.briarproject.briar.api.client.MessageTracker; @@ -74,8 +73,8 @@ class BlogProtocolEngineImpl extends ProtocolEngineImpl<Blog> { } @Override - protected ClientId getClientId() { - return BlogSharingManager.CLIENT_ID; + protected ClientId getShareableClientId() { + return BlogManager.CLIENT_ID; } @Override diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/ForumProtocolEngineImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/ForumProtocolEngineImpl.java index 11530af843eefe523e40fe0d9ab472f69c66eb64..9fb695e50718ca5686841a8a133184bdd0a0d7a2 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/ForumProtocolEngineImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/ForumProtocolEngineImpl.java @@ -15,7 +15,6 @@ import org.briarproject.briar.api.client.MessageTracker; import org.briarproject.briar.api.forum.Forum; import org.briarproject.briar.api.forum.ForumInvitationResponse; import org.briarproject.briar.api.forum.ForumManager; -import org.briarproject.briar.api.forum.ForumSharingManager; import org.briarproject.briar.api.forum.event.ForumInvitationRequestReceivedEvent; import org.briarproject.briar.api.forum.event.ForumInvitationResponseReceivedEvent; import org.briarproject.briar.api.sharing.InvitationRequest; @@ -74,8 +73,8 @@ class ForumProtocolEngineImpl extends ProtocolEngineImpl<Forum> { } @Override - protected ClientId getClientId() { - return ForumSharingManager.CLIENT_ID; + protected ClientId getShareableClientId() { + return ForumManager.CLIENT_ID; } @Override diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java b/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java index 4cc51ecb264fadb4b101ed41e243cbbdd5a478e1..7064696e862c8a7f73375e32f14b492c5b9b93be 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/ProtocolEngineImpl.java @@ -36,7 +36,6 @@ import static org.briarproject.briar.sharing.MessageType.DECLINE; import static org.briarproject.briar.sharing.MessageType.INVITE; import static org.briarproject.briar.sharing.MessageType.LEAVE; import static org.briarproject.briar.sharing.SharingConstants.GROUP_KEY_CONTACT_ID; -import static org.briarproject.briar.sharing.State.ERROR; import static org.briarproject.briar.sharing.State.LOCAL_INVITED; import static org.briarproject.briar.sharing.State.LOCAL_LEFT; import static org.briarproject.briar.sharing.State.REMOTE_HANGING; @@ -79,7 +78,6 @@ abstract class ProtocolEngineImpl<S extends Shareable> case SHARING: case LOCAL_LEFT: case REMOTE_HANGING: - case ERROR: throw new ProtocolStateException(); // Invalid in these states default: throw new AssertionError(); @@ -132,7 +130,6 @@ abstract class ProtocolEngineImpl<S extends Shareable> case SHARING: case LOCAL_LEFT: case REMOTE_HANGING: - case ERROR: throw new ProtocolStateException(); // Invalid in these states default: throw new AssertionError(); @@ -188,7 +185,6 @@ abstract class ProtocolEngineImpl<S extends Shareable> case SHARING: case LOCAL_LEFT: case REMOTE_HANGING: - case ERROR: throw new ProtocolStateException(); // Invalid in these states default: throw new AssertionError(); @@ -232,7 +228,6 @@ abstract class ProtocolEngineImpl<S extends Shareable> case LOCAL_INVITED: case LOCAL_LEFT: case REMOTE_HANGING: - case ERROR: return s; // Ignored in this state default: throw new AssertionError(); @@ -277,9 +272,7 @@ abstract class ProtocolEngineImpl<S extends Shareable> return onRemoteInvite(txn, s, m, false, LOCAL_LEFT); case LOCAL_INVITED: case SHARING: - return abort(txn, s); // Invalid in these states - case ERROR: - return s; // Ignored in this state + return abortWithMessage(txn, s); // Invalid in these states default: throw new AssertionError(); } @@ -289,10 +282,11 @@ abstract class ProtocolEngineImpl<S extends Shareable> InviteMessage<S> m, boolean available, State nextState) throws DbException, FormatException { // The timestamp must be higher than the last invite message, if any - if (m.getTimestamp() <= s.getInviteTimestamp()) return abort(txn, s); + if (m.getTimestamp() <= s.getInviteTimestamp()) + return abortWithMessage(txn, s); // The dependency, if any, must be the last remote message if (!isValidDependency(s, m.getPreviousMessageId())) - return abort(txn, s); + return abortWithMessage(txn, s); // Mark the invite message visible in the UI and (un)available to answer markMessageVisibleInUi(txn, m.getId(), true); markMessageAvailableToAnswer(txn, m.getId(), available); @@ -312,10 +306,11 @@ abstract class ProtocolEngineImpl<S extends Shareable> private Session onRemoteInviteWhenInvited(Transaction txn, Session s, InviteMessage<S> m) throws DbException, FormatException { // The timestamp must be higher than the last invite message, if any - if (m.getTimestamp() <= s.getInviteTimestamp()) return abort(txn, s); + if (m.getTimestamp() <= s.getInviteTimestamp()) + return abortWithMessage(txn, s); // The dependency, if any, must be the last remote message if (!isValidDependency(s, m.getPreviousMessageId())) - return abort(txn, s); + return abortWithMessage(txn, s); // Mark the invite message visible in the UI and unavailable to answer markMessageVisibleInUi(txn, m.getId(), true); markMessageAvailableToAnswer(txn, m.getId(), false); @@ -349,9 +344,7 @@ abstract class ProtocolEngineImpl<S extends Shareable> case LOCAL_INVITED: case SHARING: case LOCAL_LEFT: - return abort(txn, s); // Invalid in these states - case ERROR: - return s; // Ignored in this state + return abortWithMessage(txn, s); // Invalid in these states default: throw new AssertionError(); } @@ -360,10 +353,11 @@ abstract class ProtocolEngineImpl<S extends Shareable> private Session onRemoteAccept(Transaction txn, Session s, AcceptMessage m, State nextState) throws DbException, FormatException { // The timestamp must be higher than the last invite message - if (m.getTimestamp() <= s.getInviteTimestamp()) return abort(txn, s); + if (m.getTimestamp() <= s.getInviteTimestamp()) + return abortWithMessage(txn, s); // The dependency, if any, must be the last remote message if (!isValidDependency(s, m.getPreviousMessageId())) - return abort(txn, s); + return abortWithMessage(txn, s); // Mark the response visible in the UI markMessageVisibleInUi(txn, m.getId(), true); // Track the message @@ -382,8 +376,8 @@ abstract class ProtocolEngineImpl<S extends Shareable> AcceptMessage m) throws DbException, FormatException { // Perform normal remote accept validation and operation Session session = onRemoteAccept(txn, s, m, SHARING); - // Share the shareable with the contact - if (session.getState() != ERROR) + // Share the shareable with the contact, if session was not reset + if (session.getState() != START) setShareableVisibility(txn, s, SHARED); return session; } @@ -402,9 +396,7 @@ abstract class ProtocolEngineImpl<S extends Shareable> case LOCAL_INVITED: case SHARING: case LOCAL_LEFT: - return abort(txn, s); // Invalid in these states - case ERROR: - return s; // Ignored in this state + return abortWithMessage(txn, s); // Invalid in these states default: throw new AssertionError(); } @@ -413,10 +405,11 @@ abstract class ProtocolEngineImpl<S extends Shareable> private Session onRemoteDecline(Transaction txn, Session s, DeclineMessage m) throws DbException, FormatException { // The timestamp must be higher than the last invite message - if (m.getTimestamp() <= s.getInviteTimestamp()) return abort(txn, s); + if (m.getTimestamp() <= s.getInviteTimestamp()) + return abortWithMessage(txn, s); // The dependency, if any, must be the last remote message if (!isValidDependency(s, m.getPreviousMessageId())) - return abort(txn, s); + return abortWithMessage(txn, s); // Mark the response visible in the UI markMessageVisibleInUi(txn, m.getId(), true); // Track the message @@ -447,15 +440,13 @@ abstract class ProtocolEngineImpl<S extends Shareable> case LOCAL_INVITED: return onRemoteLeaveWhenInvited(txn, s, m); case LOCAL_LEFT: - return onRemoteLeave(txn, s, m); + return onRemoteLeaveWhenLocalLeft(txn, s, m); case SHARING: return onRemoteLeaveWhenSharing(txn, s, m); case START: case REMOTE_INVITED: case REMOTE_HANGING: - return abort(txn, s); // Invalid in these states - case ERROR: - return s; // Ignored in this state + return abortWithMessage(txn, s); // Invalid in these states default: throw new AssertionError(); } @@ -463,28 +454,22 @@ abstract class ProtocolEngineImpl<S extends Shareable> private Session onRemoteLeaveWhenInvited(Transaction txn, Session s, LeaveMessage m) throws DbException, FormatException { - // Carry out normal leave validation and operation - Session session = onRemoteLeave(txn, s, m); + // The dependency, if any, must be the last remote message + if (!isValidDependency(s, m.getPreviousMessageId())) + return abortWithMessage(txn, s); // Mark any invite messages in the session unavailable to answer - if (session.getState() != ERROR) - markInvitesUnavailableToAnswer(txn, s); + markInvitesUnavailableToAnswer(txn, s); // Move to the next state - return session; + return new Session(START, s.getContactGroupId(), s.getShareableId(), + s.getLastLocalMessageId(), m.getId(), s.getLocalTimestamp(), + s.getInviteTimestamp()); } - private Session onRemoteLeave(Transaction txn, Session s, LeaveMessage m) - throws DbException, FormatException { + private Session onRemoteLeaveWhenLocalLeft(Transaction txn, Session s, + LeaveMessage m) throws DbException, FormatException { // The dependency, if any, must be the last remote message if (!isValidDependency(s, m.getPreviousMessageId())) - return abort(txn, s); - if (s.getState() == SHARING) { - // Broadcast event informing that contact left - ContactId contactId = getContactId(txn, s.getContactGroupId()); - ContactLeftShareableEvent e = - new ContactLeftShareableEvent(s.getShareableId(), - contactId); - txn.attach(e); - } + return abortWithMessage(txn, s); // Move to the next state return new Session(START, s.getContactGroupId(), s.getShareableId(), s.getLastLocalMessageId(), m.getId(), s.getLocalTimestamp(), @@ -493,36 +478,48 @@ abstract class ProtocolEngineImpl<S extends Shareable> private Session onRemoteLeaveWhenSharing(Transaction txn, Session s, LeaveMessage m) throws DbException, FormatException { - // Carry out normal leave validation and operation - Session session = onRemoteLeave(txn, s, m); + // The dependency, if any, must be the last remote message + if (!isValidDependency(s, m.getPreviousMessageId())) + return abortWithMessage(txn, s); + // Broadcast event informing that contact left + ContactId contactId = getContactId(txn, s.getContactGroupId()); + ContactLeftShareableEvent e = + new ContactLeftShareableEvent(s.getShareableId(), + contactId); + txn.attach(e); // Stop sharing the shareable with the contact - if (session.getState() != ERROR) - setShareableVisibility(txn, s, INVISIBLE); + setShareableVisibility(txn, s, INVISIBLE); // Move to the next state - return session; + return new Session(START, s.getContactGroupId(), s.getShareableId(), + s.getLastLocalMessageId(), m.getId(), s.getLocalTimestamp(), + s.getInviteTimestamp()); } @Override public Session onAbortMessage(Transaction txn, Session s, AbortMessage m) throws DbException, FormatException { - return abort(txn, s); + abort(txn, s); + return new Session(START, s.getContactGroupId(), s.getShareableId(), + null, m.getId(), 0, 0); } - private Session abort(Transaction txn, Session s) + private void abort(Transaction txn, Session s) throws DbException, FormatException { - // If the session has already been aborted, do nothing - if (s.getState() == ERROR) return s; // Mark any invite messages in the session unavailable to answer markInvitesUnavailableToAnswer(txn, s); // If we subscribe, make the shareable invisible to the contact if (isSubscribed(txn, s.getShareableId())) setShareableVisibility(txn, s, INVISIBLE); + } + + private Session abortWithMessage(Transaction txn, Session s) + throws DbException, FormatException { + abort(txn, s); // Send an ABORT message Message sent = sendAbortMessage(txn, s); - // Move to the ERROR state - return new Session(ERROR, s.getContactGroupId(), s.getShareableId(), - sent.getId(), s.getLastRemoteMessageId(), sent.getTimestamp(), - s.getInviteTimestamp()); + // Reset the session back to initial state + return new Session(START, s.getContactGroupId(), s.getShareableId(), + sent.getId(), null, 0, 0); } private void markInvitesUnavailableToAnswer(Transaction txn, Session s) @@ -541,10 +538,10 @@ abstract class ProtocolEngineImpl<S extends Shareable> throws DbException { if (!db.containsGroup(txn, g)) return false; Group group = db.getGroup(txn, g); - return group.getClientId().equals(getClientId()); + return group.getClientId().equals(getShareableClientId()); } - protected abstract ClientId getClientId(); + protected abstract ClientId getShareableClientId(); private Message sendAbortMessage(Transaction txn, Session session) throws DbException { diff --git a/briar-core/src/main/java/org/briarproject/briar/sharing/State.java b/briar-core/src/main/java/org/briarproject/briar/sharing/State.java index 21c26c52978d4ace5b6b1d2bba9a18a9e87124a5..ade7d7a248787123e6a10bd8cfcc43cc4ba51210 100644 --- a/briar-core/src/main/java/org/briarproject/briar/sharing/State.java +++ b/briar-core/src/main/java/org/briarproject/briar/sharing/State.java @@ -10,7 +10,7 @@ import javax.annotation.concurrent.Immutable; enum State { START(0), LOCAL_INVITED(1), REMOTE_INVITED(2), SHARING(3), LOCAL_LEFT(4), - REMOTE_HANGING(5), ERROR(6); + REMOTE_HANGING(5); private final int value; diff --git a/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java index 6f351f708755124824d8bcd8e9dfe37e6f6f7289..b33e2b6a7e68735b4248360cecd8f474d7e5a3cd 100644 --- a/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/sharing/ForumSharingIntegrationTest.java @@ -3,11 +3,14 @@ package org.briarproject.briar.sharing; import net.jodah.concurrentunit.Waiter; import org.briarproject.bramble.api.contact.Contact; +import org.briarproject.bramble.api.data.BdfDictionary; import org.briarproject.bramble.api.db.DbException; import org.briarproject.bramble.api.db.Transaction; import org.briarproject.bramble.api.event.Event; import org.briarproject.bramble.api.event.EventListener; import org.briarproject.bramble.api.nullsafety.NotNullByDefault; +import org.briarproject.bramble.api.sync.Message; +import org.briarproject.bramble.api.sync.MessageId; import org.briarproject.bramble.test.TestDatabaseModule; import org.briarproject.briar.api.forum.Forum; import org.briarproject.briar.api.forum.ForumInvitationRequest; @@ -42,6 +45,7 @@ public class ForumSharingIntegrationTest extends BriarIntegrationTest<BriarIntegrationTestComponent> { private ForumManager forumManager0, forumManager1; + private MessageEncoder messageEncoder; private SharerListener listener0, listener2; private InviteeListener listener1; private Forum forum0; @@ -67,6 +71,7 @@ public class ForumSharingIntegrationTest forumSharingManager0 = c0.getForumSharingManager(); forumSharingManager1 = c1.getForumSharingManager(); forumSharingManager2 = c2.getForumSharingManager(); + messageEncoder = new MessageEncoderImpl(clientHelper, messageFactory); // initialize waiter fresh for each test eventWaiter = new Waiter(); @@ -686,6 +691,74 @@ public class ForumSharingIntegrationTest assertTrue(found); } + @Test + public void testSessionResetAfterAbort() throws Exception { + // initialize and let invitee accept all requests + listenToEvents(true); + + // send invitation + forumSharingManager0 + .sendInvitation(forum0.getId(), contactId1From0, "Hi!", + clock.currentTimeMillis()); + + // sync first request message + sync0To1(1, true); + eventWaiter.await(TIMEOUT, 1); + assertTrue(listener1.requestReceived); + + // get invitation MessageId for later + MessageId invitationId = null; + for (InvitationMessage m : forumSharingManager1 + .getInvitationMessages(contactId0From1)) { + if (m instanceof ForumInvitationRequest) { + invitationId = m.getId(); + } + } + assertNotNull(invitationId); + + // sync response back + sync1To0(1, true); + eventWaiter.await(TIMEOUT, 1); + assertTrue(listener0.responseReceived); + + // forum is shared mutually + assertTrue(forumSharingManager0.getSharedWith(forum0.getId()) + .contains(contact1From0)); + assertTrue(forumSharingManager1.getSharedWith(forum0.getId()) + .contains(contact0From1)); + + // send an accept message for the same forum + Message m = messageEncoder.encodeAcceptMessage( + forumSharingManager0.getContactGroup(contact1From0).getId(), + forum0.getId(), clock.currentTimeMillis(), invitationId); + c0.getClientHelper().addLocalMessage(m, new BdfDictionary(), true); + + // sync unexpected message and the expected abort message back + sync0To1(1, true); + sync1To0(1, true); + + // forum is no longer shared mutually + assertFalse(forumSharingManager0.getSharedWith(forum0.getId()) + .contains(contact1From0)); + assertFalse(forumSharingManager1.getSharedWith(forum0.getId()) + .contains(contact0From1)); + + // new invitation is possible now + forumSharingManager0 + .sendInvitation(forum0.getId(), contactId1From0, null, + clock.currentTimeMillis()); + sync0To1(1, true); + + // and can be answered + sync1To0(1, true); + + // forum is shared mutually again + assertTrue(forumSharingManager0.getSharedWith(forum0.getId()) + .contains(contact1From0)); + assertTrue(forumSharingManager1.getSharedWith(forum0.getId()) + .contains(contact0From1)); + } + @NotNullByDefault private class SharerListener implements EventListener { diff --git a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java index 7d216adca00b13d03793f98d37623b8453417b54..1376500ddd70840e9ee4f5f452984b2b89b4633e 100644 --- a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java +++ b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTest.java @@ -18,6 +18,7 @@ import org.briarproject.bramble.api.identity.LocalAuthor; import org.briarproject.bramble.api.lifecycle.LifecycleManager; import org.briarproject.bramble.api.nullsafety.MethodsNotNullByDefault; import org.briarproject.bramble.api.nullsafety.ParametersNotNullByDefault; +import org.briarproject.bramble.api.sync.MessageFactory; import org.briarproject.bramble.api.sync.SyncSession; import org.briarproject.bramble.api.sync.SyncSessionFactory; import org.briarproject.bramble.api.sync.event.MessageStateChangedEvent; @@ -100,6 +101,8 @@ public abstract class BriarIntegrationTest<C extends BriarIntegrationTestCompone @Inject protected AuthorFactory authorFactory; @Inject + protected MessageFactory messageFactory; + @Inject protected ContactGroupFactory contactGroupFactory; @Inject protected PrivateGroupFactory privateGroupFactory; diff --git a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java index 5f036e6ee5603b23992a751235c13d124cab6e72..c4219c9e486928c05d515f163d11f54f71baab0e 100644 --- a/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java +++ b/briar-core/src/test/java/org/briarproject/briar/test/BriarIntegrationTestComponent.java @@ -25,7 +25,6 @@ import org.briarproject.bramble.test.TestSeedProviderModule; import org.briarproject.bramble.transport.TransportModule; import org.briarproject.briar.api.blog.BlogManager; import org.briarproject.briar.api.blog.BlogSharingManager; -import org.briarproject.briar.api.client.MessageQueueManager; import org.briarproject.briar.api.client.MessageTracker; import org.briarproject.briar.api.forum.ForumManager; import org.briarproject.briar.api.forum.ForumSharingManager; @@ -133,8 +132,6 @@ public interface BriarIntegrationTestComponent { MessageTracker getMessageTracker(); - MessageQueueManager getMessageQueueManager(); - PrivateGroupManager getPrivateGroupManager(); TransportPropertyManager getTransportPropertyManager();