From 47821c85784681f699e7e364b5172929f27c5b80 Mon Sep 17 00:00:00 2001 From: bontric <benjohnwie@gmail.com> Date: Wed, 12 Sep 2018 16:57:57 +0200 Subject: [PATCH] Cleanup/refactor mailbox protocol (add "endOfStream" bool to SYNC request and add sendKeepAlive to mailboxProtocol) --- .../mailbox/protocol/MailboxMessage.java | 42 +++++++-- .../mailbox/protocol/MailboxProtocol.java | 89 ++++++++----------- .../mailbox/protocol/MailboxRequest.java | 15 ++-- .../mailbox/protocol/MailboxRequestStore.java | 6 +- .../mailbox/protocol/MailboxRequestSync.java | 15 ++-- .../mailbox/protocol/MailboxRequestTake.java | 10 +-- .../mailbox/protocol/MailboxResponse.java | 17 ++-- 7 files changed, 94 insertions(+), 100 deletions(-) diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxMessage.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxMessage.java index 39c952ebf..96396e175 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxMessage.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxMessage.java @@ -3,27 +3,52 @@ package org.briarproject.bramble.mailbox.protocol; import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.data.BdfList; +import java.net.ProtocolException; + /** * MailboxRPCMessages are serialized as BdfLists and formatted as follows: - * + * <p> * Requests (BODY depending on TYPE): * [TYPE, MESSAGE_ID, [BODY]] * [long, long [BdfList]] - * + * <p> * Error Responses: * [TYPE, MESSAGE_ID, [false, ERROR_MSG]] * [long, long, [boolean, string]] - * + * <p> * Successful Responses (BODY depending on TYPE): * [TYPE, MESSAGE_ID, [true, [BODY]]] * [long, long, [boolean, [BdfList]]] */ public interface MailboxMessage { + static MailboxMessage parse(BdfList msg) throws ProtocolException { + try { + TYPE mType = TYPE.fromLong(msg.getLong(0)); + switch (mType) { + case RESPONSE: + return new MailboxResponse(msg); + case SYNC: + return new MailboxRequestSync(msg); + case TAKE: + return new MailboxRequestTake(msg); + case END: + return new MailboxRequestEnd(msg); + case STORE: + return new MailboxRequestStore(msg); + default: + throw new ProtocolException( + "Unknown message Type received"); + } + } catch (FormatException e) { + throw new ProtocolException("Invalid MailboxMessageReceived"); + } + } + /** * @return Message ID which uniquely identifies a Request and is used to * match responses to a request. - * + * <p> * (NOTE: This identifier is only unique for "active" requests, which * wer sent and await a response. Replay protection must be provided by the * underlying protocol!) @@ -48,8 +73,6 @@ public interface MailboxMessage { BdfList toBdfList(); /** - * - * * @param list * @throws FormatException */ @@ -68,9 +91,10 @@ public interface MailboxMessage { this.value = value; } - public static TYPE fromLong(long b) throws IllegalArgumentException{ - if (b > TYPE.values().length){ - throw new IllegalArgumentException("Invalid Mailbox Message Type"); + public static TYPE fromLong(long b) throws IllegalArgumentException { + if (b > TYPE.values().length) { + throw new IllegalArgumentException( + "Invalid Mailbox Message Type"); } return TYPE.values()[(int) b]; } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java index 12fbff73d..7df043a43 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocol.java @@ -1,10 +1,8 @@ package org.briarproject.bramble.mailbox.protocol; -import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.data.BdfList; import org.briarproject.bramble.api.data.BdfReader; import org.briarproject.bramble.api.data.BdfWriter; -import org.briarproject.bramble.mailbox.protocol.MailboxMessage.TYPE; import java.io.EOFException; import java.io.IOException; @@ -37,7 +35,6 @@ public class MailboxProtocol implements Runnable { volatile BlockingQueue<MailboxRequest> inQueue = new LinkedBlockingQueue<>(); - volatile ConcurrentHashMap<Long, MailboxRequest> pendingRequests = new ConcurrentHashMap<>(); @@ -54,6 +51,13 @@ public class MailboxProtocol implements Runnable { } + public void sendKeepAlive() throws IOException { + // flush the writer without pending data to send a keepalive + synchronized (mailboxBdfWriter) { + mailboxBdfWriter.flush(); + } + } + public void writeRequest(MailboxMessage req) throws InterruptedException, IOException { if (req.hasResponse()) @@ -124,19 +128,33 @@ public class MailboxProtocol implements Runnable { } try { - mailboxMessage = parseMessage(bdfMsg); + mailboxMessage = MailboxMessage.parse(bdfMsg); } catch (ProtocolException e) { LOG.info(e.toString()); continue; } - if (mailboxMessage.getType() == RESPONSE) { + if (mailboxMessage.getType() == RESPONSE) handleResponse((MailboxResponse) mailboxMessage); - continue; - } + else + inQueue.add((MailboxRequest) mailboxMessage); + } + } + + private void handleResponse(MailboxResponse msg) { + MailboxRequest req = pendingRequests.remove(msg.getId()); - inQueue.add((MailboxRequest) mailboxMessage); + if (req == null) { + if (LOG.isLoggable(WARNING)) + LOG.warning("Received response not matching with any request"); + return; } + + if (msg.isSuccess()) + req.signalSucess(); + else + req.signalError(msg.getErrorMessage()); + } private void writeOutgoingMessages() { @@ -153,18 +171,19 @@ public class MailboxProtocol implements Runnable { return; } - try { - mailboxBdfWriter.writeList(message.toBdfList()); - mailboxBdfWriter.flush(); - } catch (IOException e) { - writingThread.interrupt(); - handleIOException(false, e); - return; + synchronized (mailboxBdfWriter) { + try { + mailboxBdfWriter.writeList(message.toBdfList()); + mailboxBdfWriter.flush(); + } catch (IOException e) { + writingThread.interrupt(); + handleIOException(false, e); + return; + } } } } - private void handleIOException(boolean isReadingThread, IOException e) { if (isReadingThread && writingThread != null) writingThread.interrupt(); @@ -191,43 +210,5 @@ public class MailboxProtocol implements Runnable { } } - private MailboxMessage parseMessage(BdfList msg) throws ProtocolException { - try { - TYPE mType = TYPE.fromLong(msg.getLong(0)); - switch (mType) { - case RESPONSE: - return new MailboxResponse(msg); - case SYNC: - return new MailboxRequestSync(msg); - case TAKE: - return new MailboxRequestTake(msg); - case END: - return new MailboxRequestEnd(msg); - case STORE: - return new MailboxRequestStore(msg); - default: - throw new ProtocolException( - "Unknown message Type received"); - } - } catch (FormatException e) { - throw new ProtocolException("Invalid MailboxMessageReceived"); - } - } - - private void handleResponse(MailboxResponse msg) { - MailboxRequest req = pendingRequests.remove(msg.getId()); - - if (req == null) { - if (LOG.isLoggable(WARNING)) - LOG.warning("Received response not matching with any request"); - return; - } - - if (msg.isSuccess()) - req.signalSucess(); - else - req.signalError(msg.getErrorMessage()); - - } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequest.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequest.java index 1a6313eab..c0b024707 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequest.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequest.java @@ -3,9 +3,8 @@ package org.briarproject.bramble.mailbox.protocol; import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.data.BdfList; -import java.util.Arrays; -import java.util.Random; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicLong; public abstract class MailboxRequest implements MailboxMessage { @@ -16,11 +15,11 @@ public abstract class MailboxRequest implements MailboxMessage { private AtomicBoolean responseReceived = new AtomicBoolean(false); private boolean wasSuccessful = false; + static AtomicLong msgIdCounter = new AtomicLong(); + public MailboxRequest(TYPE type) { this.type = type; - // msgid may as well be a counter here.. - Random random = new Random(); - msgId = random.nextLong(); + this.msgId = msgIdCounter.getAndIncrement(); } public MailboxRequest(BdfList lst) throws FormatException { @@ -31,8 +30,7 @@ public abstract class MailboxRequest implements MailboxMessage { @Override public BdfList toBdfList() { - return new BdfList( - Arrays.asList(type.getValue(), getId(), getRequestBody())); + return BdfList.of(type.getValue(), getId(), getRequestBody()); } @Override @@ -75,9 +73,8 @@ public abstract class MailboxRequest implements MailboxMessage { "Attempting to wait for a response of a request with no response"); synchronized (responseReceived) { - while (!responseReceived.get()) { + while (!responseReceived.get()) responseReceived.wait(); - } } return wasSuccessful; diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestStore.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestStore.java index f94f6e494..83cc85a86 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestStore.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestStore.java @@ -4,9 +4,6 @@ import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.data.BdfList; -import java.io.IOException; -import java.util.Arrays; - import static org.briarproject.bramble.mailbox.protocol.MailboxMessage.TYPE.STORE; public class MailboxRequestStore extends MailboxRequest { @@ -25,8 +22,7 @@ public class MailboxRequestStore extends MailboxRequest { } public BdfList getRequestBody() { - return new BdfList(Arrays.asList(contactId.getInt(), - encryptedSyncStream)); + return BdfList.of(contactId.getInt(), encryptedSyncStream); } @Override diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestSync.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestSync.java index 7a69e135a..9d5fa51c5 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestSync.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestSync.java @@ -3,15 +3,14 @@ package org.briarproject.bramble.mailbox.protocol; import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.data.BdfList; -import java.io.IOException; -import java.util.Arrays; - public class MailboxRequestSync extends MailboxRequest { private byte[] encryptedStream; + private boolean endOfStream; - public MailboxRequestSync(byte [] encryptedStream) { + public MailboxRequestSync(byte[] encryptedStream, boolean endOfStream) { super(TYPE.SYNC); this.encryptedStream = encryptedStream; + this.endOfStream = endOfStream; } public MailboxRequestSync(BdfList lst) throws FormatException { @@ -20,10 +19,9 @@ public class MailboxRequestSync extends MailboxRequest { @Override protected BdfList getRequestBody() { - return new BdfList(Arrays.asList(encryptedStream)); + return BdfList.of(encryptedStream, endOfStream); } - @Override public boolean hasResponse() { return false; @@ -32,9 +30,14 @@ public class MailboxRequestSync extends MailboxRequest { @Override public void parseBody(BdfList list) throws FormatException { this.encryptedStream = list.getRaw(0); + this.endOfStream = list.getBoolean(1); } public byte[] getSyncStream() { return encryptedStream; } + + public boolean isEndOfStream() { + return endOfStream; + } } diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestTake.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestTake.java index a1ccd2ecb..6fe0b80ac 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestTake.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestTake.java @@ -4,14 +4,11 @@ import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.contact.ContactId; import org.briarproject.bramble.api.data.BdfList; -import java.io.IOException; -import java.util.Arrays; - public class MailboxRequestTake extends MailboxRequest { private ContactId contactId; - private byte [] encryptedSyncStream = null; + private byte[] encryptedSyncStream = null; - public MailboxRequestTake(ContactId contactId,byte [] encryptedSyncStream){ + public MailboxRequestTake(ContactId contactId, byte[] encryptedSyncStream) { super(TYPE.TAKE); this.contactId = contactId; this.encryptedSyncStream = encryptedSyncStream; @@ -21,10 +18,9 @@ public class MailboxRequestTake extends MailboxRequest { super(msg); } - @Override protected BdfList getRequestBody() { - return new BdfList(Arrays.asList(contactId.getInt(), encryptedSyncStream)); + return BdfList.of(contactId.getInt(), encryptedSyncStream); } @Override diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxResponse.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxResponse.java index 077b8ee69..0febf4634 100644 --- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxResponse.java +++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxResponse.java @@ -3,8 +3,6 @@ package org.briarproject.bramble.mailbox.protocol; import org.briarproject.bramble.api.FormatException; import org.briarproject.bramble.api.data.BdfList; -import java.util.Arrays; - import javax.annotation.Nullable; import static org.briarproject.bramble.mailbox.protocol.MailboxMessage.TYPE.RESPONSE; @@ -46,21 +44,20 @@ public class MailboxResponse implements MailboxMessage { @Override public BdfList toBdfList() { BdfList body; - if (!success) { - body = new BdfList(Arrays.asList(success, errorMessage)); - } else { - body = new BdfList(Arrays.asList(success)); - } - return new BdfList(Arrays.asList(RESPONSE.getValue(), msgId, body)); + if (!success) + body = BdfList.of(success, errorMessage); + else + body = BdfList.of(success); + + return BdfList.of(RESPONSE.getValue(), msgId, body); } @Override public void parseBody(BdfList list) throws FormatException { success = list.getBoolean(0); - if (!success) { + if (!success) errorMessage = list.getString(1); - } } public String getErrorMessage() { -- GitLab