From 165fa2ff1f16ef6da1103cac1be3dd7782a9418c Mon Sep 17 00:00:00 2001
From: bontric <benjohnwie@gmail.com>
Date: Tue, 25 Sep 2018 12:39:11 +0200
Subject: [PATCH] Refactor mailbox message formatting and add format checks
---
.../bramble/mailbox/MailboxManagerImpl.java | 2 --
.../mailbox/MailboxSyncRequestWriter.java | 6 ++--
.../mailbox/protocol/MailboxMessage.java | 9 ++----
.../mailbox/protocol/MailboxProtocol.java | 4 +--
.../mailbox/protocol/MailboxRequest.java | 9 ++++--
.../mailbox/protocol/MailboxRequestEnd.java | 3 +-
.../mailbox/protocol/MailboxRequestStore.java | 3 ++
.../mailbox/protocol/MailboxRequestSync.java | 26 ++++++++++-------
.../mailbox/protocol/MailboxRequestTake.java | 2 ++
.../mailbox/protocol/MailboxResponse.java | 29 +++++--------------
.../MailboxProtocolIntegrationTest.java | 2 +-
11 files changed, 44 insertions(+), 51 deletions(-)
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java
index cc87e1ab0..77d16fbc6 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxManagerImpl.java
@@ -112,7 +112,6 @@ public class MailboxManagerImpl implements MailboxManager {
public void handleOwnerContactWithoutMailbox(MailboxInfo mailboxInfo) {
if (null == privateMailboxSession)
return;
-
ioExecutor.execute(
() -> privateMailboxSession.handleContactWithoutPrivateMailbox(mailboxInfo));
}
@@ -254,7 +253,6 @@ public class MailboxManagerImpl implements MailboxManager {
}
private void handleIncomingStream() throws IOException {
-
InputStream mailboxInputStream =
streamReaderFactory.createStreamReader(
reader.getInputStream(), incomingCtx);
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSyncRequestWriter.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSyncRequestWriter.java
index 647b03c68..5e8c57fda 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSyncRequestWriter.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/MailboxSyncRequestWriter.java
@@ -29,9 +29,7 @@ class MailboxSyncRequestWriter extends OutputStream implements StreamWriter {
if (endOfStream)
throw new IOException("End of stream was already written");
- endOfStream = true;
-
- MailboxRequestSync req = new MailboxRequestSync(new byte[] {}, true);
+ MailboxRequestSync req = new MailboxRequestSync();
try {
mailboxProtocol.writeRequest(req);
} catch (InterruptedException e) {
@@ -59,7 +57,7 @@ class MailboxSyncRequestWriter extends OutputStream implements StreamWriter {
byte[] syncStream = bufferOS.toByteArray();
bufferOS.reset();
- MailboxRequestSync req = new MailboxRequestSync(syncStream, false);
+ MailboxRequestSync req = new MailboxRequestSync(syncStream);
try {
mailboxProtocol.writeRequest(req);
} catch (InterruptedException e) {
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 96396e175..21de3d151 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
@@ -15,11 +15,11 @@ import java.net.ProtocolException;
* <p>
* Error Responses:
* [TYPE, MESSAGE_ID, [false, ERROR_MSG]]
- * [long, long, [boolean, string]]
+ * [long, long, [string]]
* <p>
* Successful Responses (BODY depending on TYPE):
* [TYPE, MESSAGE_ID, [true, [BODY]]]
- * [long, long, [boolean, [BdfList]]]
+ * [long, long, [null]]
*/
public interface MailboxMessage {
static MailboxMessage parse(BdfList msg) throws ProtocolException {
@@ -55,11 +55,6 @@ public interface MailboxMessage {
*/
long getId();
- /**
- * @return true if the message expects a response
- */
- boolean hasResponse();
-
/**
* @return Message TYPE
*/
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 940b6db9b..089526ef2 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
@@ -65,7 +65,7 @@ public class MailboxProtocol implements Runnable {
}
}
- public void writeRequest(MailboxMessage req)
+ public void writeRequest(MailboxRequest req)
throws InterruptedException, IOException {
if (req.hasResponse())
pendingRequests.put(req.getId(), (MailboxRequest) req);
@@ -259,7 +259,7 @@ public class MailboxProtocol implements Runnable {
// Signal the error to anyone waiting for a response
for (Entry<Long, MailboxRequest> entry : pendingRequests.entrySet()) {
- MailboxResponse r = new MailboxResponse(entry.getKey(), false,
+ MailboxResponse r = new MailboxResponse(entry.getKey(),
"Connection closed");
entry.getValue().signalError(r.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 c0b024707..928bd379b 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
@@ -61,6 +61,11 @@ public abstract class MailboxRequest implements MailboxMessage {
}
}
+ /**
+ * @return true if the message expects a response
+ */
+ abstract boolean hasResponse();
+
/**
* Blocks until a response for this request has been received
*
@@ -89,11 +94,11 @@ public abstract class MailboxRequest implements MailboxMessage {
}
public MailboxResponse createSuccessResponse() {
- return new MailboxResponse(msgId, true, null);
+ return new MailboxResponse(msgId, null);
}
public MailboxResponse createErrorResponse(String error) {
- return new MailboxResponse(msgId, false, error);
+ return new MailboxResponse(msgId, error);
}
protected abstract BdfList getRequestBody();
diff --git a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestEnd.java b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestEnd.java
index 3add90694..2fc1a53da 100644
--- a/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestEnd.java
+++ b/bramble-core/src/main/java/org/briarproject/bramble/mailbox/protocol/MailboxRequestEnd.java
@@ -27,6 +27,7 @@ public class MailboxRequestEnd extends MailboxRequest {
@Override
public void parseBody(BdfList list) throws FormatException {
- return;
+ if(list.size() != 0)
+ throw new FormatException();
}
}
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 362119f3d..6c7113ec3 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
@@ -43,6 +43,9 @@ public class MailboxRequestStore extends MailboxRequest {
@Override
public void parseBody(BdfList msg) throws FormatException {
+ if(msg.size() > 2)
+ throw new FormatException();
+
Long cId = msg.getOptionalLong(0);
if (cId != null) {
if (cId > Integer.MAX_VALUE || cId < Integer.MIN_VALUE)
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 9d5fa51c5..288941113 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,14 +3,19 @@ package org.briarproject.bramble.mailbox.protocol;
import org.briarproject.bramble.api.FormatException;
import org.briarproject.bramble.api.data.BdfList;
+import javax.annotation.Nullable;
+
public class MailboxRequestSync extends MailboxRequest {
- private byte[] encryptedStream;
- private boolean endOfStream;
+ @Nullable
+ private byte[] syncStream;
- public MailboxRequestSync(byte[] encryptedStream, boolean endOfStream) {
+ public MailboxRequestSync(byte[] syncStream) {
+ super(TYPE.SYNC);
+ this.syncStream = syncStream;
+ }
+ public MailboxRequestSync() {
super(TYPE.SYNC);
- this.encryptedStream = encryptedStream;
- this.endOfStream = endOfStream;
+ this.syncStream = null;
}
public MailboxRequestSync(BdfList lst) throws FormatException {
@@ -19,7 +24,7 @@ public class MailboxRequestSync extends MailboxRequest {
@Override
protected BdfList getRequestBody() {
- return BdfList.of(encryptedStream, endOfStream);
+ return BdfList.of(syncStream);
}
@Override
@@ -29,15 +34,16 @@ public class MailboxRequestSync extends MailboxRequest {
@Override
public void parseBody(BdfList list) throws FormatException {
- this.encryptedStream = list.getRaw(0);
- this.endOfStream = list.getBoolean(1);
+ if(list.size() != 1)
+ throw new FormatException();
+ this.syncStream = list.getOptionalRaw(0);
}
public byte[] getSyncStream() {
- return encryptedStream;
+ return syncStream;
}
public boolean isEndOfStream() {
- return endOfStream;
+ return syncStream == null;
}
}
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 9919f0c85..a0409538d 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
@@ -28,6 +28,8 @@ public class MailboxRequestTake extends MailboxRequest {
@Override
public void parseBody(BdfList msg) throws FormatException {
+ if(msg.size() != 1)
+ throw new FormatException();
this.encryptedSyncStream = msg.getRaw(0);
}
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 0febf4634..873c16d0f 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
@@ -8,16 +8,12 @@ import javax.annotation.Nullable;
import static org.briarproject.bramble.mailbox.protocol.MailboxMessage.TYPE.RESPONSE;
public class MailboxResponse implements MailboxMessage {
-
-
- private boolean success;
+ @Nullable
private String errorMessage;
private long msgId;
- public MailboxResponse(long msgId, boolean success,
- @Nullable String errorMessage) {
+ public MailboxResponse(long msgId, @Nullable String errorMessage) {
this.msgId = msgId;
- this.success = success;
this.errorMessage = errorMessage;
}
@@ -31,11 +27,6 @@ public class MailboxResponse implements MailboxMessage {
return msgId;
}
- @Override
- public boolean hasResponse() {
- return false;
- }
-
@Override
public TYPE getType() {
return RESPONSE;
@@ -43,21 +34,15 @@ public class MailboxResponse implements MailboxMessage {
@Override
public BdfList toBdfList() {
- BdfList body;
- if (!success)
- body = BdfList.of(success, errorMessage);
- else
- body = BdfList.of(success);
-
- return BdfList.of(RESPONSE.getValue(), msgId, body);
+ return BdfList.of(RESPONSE.getValue(), msgId, BdfList.of(errorMessage));
}
@Override
public void parseBody(BdfList list) throws FormatException {
- success = list.getBoolean(0);
- if (!success)
- errorMessage = list.getString(1);
+ if(list.size() != 1)
+ throw new FormatException();
+ errorMessage = list.getOptionalString(0);
}
public String getErrorMessage() {
@@ -65,6 +50,6 @@ public class MailboxResponse implements MailboxMessage {
}
public boolean isSuccess() {
- return success;
+ return errorMessage == null;
}
}
\ No newline at end of file
diff --git a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java
index 66e7d4b9c..f9de586ba 100644
--- a/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java
+++ b/bramble-core/src/test/java/org/briarproject/bramble/mailbox/protocol/MailboxProtocolIntegrationTest.java
@@ -297,7 +297,7 @@ public class MailboxProtocolIntegrationTest extends BrambleTestCase {
@Test
public void syncRequest() throws IOException, InterruptedException {
MailboxRequestSync req =
- new MailboxRequestSync("Test".getBytes(), false);
+ new MailboxRequestSync("Test".getBytes());
mailboxProtocol.registerRequestHandler(new MailboxRequestHandler() {
@Override
--
GitLab