Skip to content

Added logging, clarified code #219

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
paddybyers merged 4 commits into from
Nov 11, 2016
Merged

Added logging, clarified code #219

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
baccca8
Added channel name to bad base64/json data log messages
psolstice Nov 4, 2016
8701cb8
Fixed crypto-related tests, clarified BaseMessage code
psolstice Nov 7, 2016
a8dab9b
Throw MessageDecodeException in case of decryption error
psolstice Nov 8, 2016
86c86f8
Fixed crypto_publish_key_mismatch test
psolstice Nov 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 5 additions & 13 deletions lib/src/main/java/io/ably/lib/realtime/Channel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,7 @@
import io.ably.lib.http.Http.BodyHandler;
import io.ably.lib.transport.ConnectionManager;
import io.ably.lib.transport.ConnectionManager.QueuedMessage;
import io.ably.lib.types.AblyException;
import io.ably.lib.types.ChannelOptions;
import io.ably.lib.types.ErrorInfo;
import io.ably.lib.types.Message;
import io.ably.lib.types.MessageSerializer;
import io.ably.lib.types.PaginatedResult;
import io.ably.lib.types.Param;
import io.ably.lib.types.PresenceMessage;
import io.ably.lib.types.ProtocolMessage;
import io.ably.lib.types.*;
Copy link
Copy Markdown
Member

@mattheworiordan mattheworiordan Nov 7, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe @paddybyers is not a big fan of these broad imports, but I'll let him chime in :)

Copy link
Copy Markdown
Contributor Author

@psolstice psolstice Nov 8, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was an automatic change by IDEA.

import io.ably.lib.types.ProtocolMessage.Action;
import io.ably.lib.types.ProtocolMessage.Flag;
import io.ably.lib.util.EventEmitter;
Expand Down Expand Up @@ -383,8 +375,8 @@ private void onMessage(ProtocolMessage message) {
Message msg = messages[i];
try {
msg.decode(options);
} catch(AblyException e) {
Log.e(TAG, "Unexpected exception decrypting message", e);
} catch (MessageDecodeException e) {
Log.e(TAG, String.format("%s on channel %s", e.errorInfo.message, name));
}
/* populate fields derived from protocol message */
if(msg.connectionId == null) msg.connectionId = message.connectionId;
Expand All @@ -408,8 +400,8 @@ private void onPresence(ProtocolMessage message, String syncChannelSerial) {
PresenceMessage msg = messages[i];
try {
msg.decode(options);
} catch(AblyException e) {
Log.e(TAG, "Unexpected exception decrypting message", e);
} catch (MessageDecodeException e) {
Log.e(TAG, String.format("%s on channel %s", e.errorInfo.message, name));
}
/* populate fields derived from protocol message */
if(msg.connectionId == null) msg.connectionId = message.connectionId;
Expand Down
111 changes: 57 additions & 54 deletions lib/src/main/java/io/ably/lib/types/BaseMessage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,48 +68,48 @@ public void getDetails(StringBuilder builder) {
builder.append(" id=").append(id);
}

public void decode(ChannelOptions opts) throws AblyException {
public void decode(ChannelOptions opts) throws MessageDecodeException {
if(encoding != null) {
String[] xforms = encoding.split("\\/");
int i = 0, j = xforms.length;
try {
while((i = j) > 0) {
Matcher match = xformPattern.matcher(xforms[--j]);
if(!match.matches()) break;
String xform = match.group(1).intern();
if(xform == "base64") {
try {
data = Base64Coder.decode((String) data);
}
catch (IllegalArgumentException e) {
Log.e(TAG, "Invalid base64 data received");
break;
}
continue;
}
if(xform == "utf-8") {
try { data = new String((byte[])data, "UTF-8"); } catch(UnsupportedEncodingException e) {}
continue;
}
if(xform == "json") {
try {
String jsonText = ((String)data).trim();
data = Serialisation.gsonParser.parse(jsonText);
}
catch(JsonParseException e) {
Log.e(TAG, "Invalid JSON data received");
break;
}
continue;
}
if(xform == "cipher") {
if(opts != null && opts.encrypted) {
data = opts.getCipher().decrypt((byte[]) data);
switch(match.group(1)) {
case "base64":
try {
data = Base64Coder.decode((String) data);
} catch (IllegalArgumentException e) {
throw MessageDecodeException.fromDescription("Invalid base64 data received");
}
continue;

case "utf-8":
try { data = new String((byte[])data, "UTF-8"); } catch(UnsupportedEncodingException e) {}
continue;

case "json":
try {
String jsonText = ((String)data).trim();
data = Serialisation.gsonParser.parse(jsonText);
} catch(JsonParseException e) {
throw MessageDecodeException.fromDescription("Invalid JSON data received");
}
continue;
}
else {
Log.i(TAG, "Encrypted message received but encryption is not set up");
}

case "cipher":
if(opts != null && opts.encrypted) {
try {
data = opts.getCipher().decrypt((byte[]) data);
} catch(AblyException e) {
throw MessageDecodeException.fromDescription(e.errorInfo.message);
}
continue;
}
else {
throw MessageDecodeException.fromDescription("Encrypted message received but encryption is not set up");
}
}
break;
}
Expand Down Expand Up @@ -179,26 +179,29 @@ public JsonElement serialize(BaseMessage message, Type typeOfMessage, JsonSerial
/* Msgpack processing */
boolean readField(MessageUnpacker unpacker, String fieldName, MessageFormat fieldType) throws IOException {
boolean result = true;
if(fieldName == "timestamp") {
timestamp = unpacker.unpackLong();
} else if(fieldName == "id") {
id = unpacker.unpackString();
} else if(fieldName == "clientId") {
clientId = unpacker.unpackString();
} else if(fieldName == "connectionId") {
connectionId = unpacker.unpackString();
} else if(fieldName == "encoding") {
encoding = unpacker.unpackString();
} else if(fieldName == "data") {
if(fieldType.getValueType().isBinaryType()) {
byte[] byteData = new byte[unpacker.unpackBinaryHeader()];
unpacker.readPayload(byteData);
data = byteData;
} else {
data = unpacker.unpackString();
}
} else {
result = false;
switch (fieldName) {
case "timestamp":
timestamp = unpacker.unpackLong(); break;
case "id":
id = unpacker.unpackString(); break;
case "clientId":
clientId = unpacker.unpackString(); break;
case "connectionId":
connectionId = unpacker.unpackString(); break;
case "encoding":
encoding = unpacker.unpackString(); break;
case "data":
if(fieldType.getValueType().isBinaryType()) {
byte[] byteData = new byte[unpacker.unpackBinaryHeader()];
unpacker.readPayload(byteData);
data = byteData;
} else {
data = unpacker.unpackString();
}
break;
default:
result = false;
break;
}
return result;
}
Expand Down
17 changes: 17 additions & 0 deletions lib/src/main/java/io/ably/lib/types/MessageDecodeException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package io.ably.lib.types;

/**
* Special AblyException for message decoding problems
*/
public class MessageDecodeException extends AblyException {

private MessageDecodeException(Throwable e, String description) {
super(e, new ErrorInfo(description, 91200));
}

public static MessageDecodeException fromDescription(String description) {
return new MessageDecodeException(
new Exception(description),
description);
}
}
15 changes: 12 additions & 3 deletions lib/src/main/java/io/ably/lib/types/MessageSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;

import io.ably.lib.util.Log;
import org.msgpack.core.MessagePack;
import org.msgpack.core.MessagePacker;
import org.msgpack.core.MessageUnpacker;
Expand Down Expand Up @@ -112,9 +113,15 @@ public Message[] handleResponseBody(String contentType, byte[] body) throws Ably
messages = readJSON(body);
else if("application/x-msgpack".equals(contentType))
messages = readMsgpack(body);
if(messages != null)
for(Message message : messages)
message.decode(opts);
if(messages != null) {
for (Message message : messages) {
try {
message.decode(opts);
} catch (MessageDecodeException e) {
Log.e(TAG, e.errorInfo.message);
}
}
}
return messages;
} catch(IOException e) {
throw AblyException.fromThrowable(e);
Expand All @@ -125,4 +132,6 @@ else if("application/x-msgpack".equals(contentType))
}

private static BodyHandler<Message> messageResponseHandler = new MessageBodyHandler(null);

private static final String TAG = MessageSerializer.class.getName();
}
15 changes: 12 additions & 3 deletions lib/src/main/java/io/ably/lib/types/PresenceSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import java.io.ByteArrayOutputStream;
import java.io.IOException;

import io.ably.lib.util.Log;
import org.msgpack.core.MessagePack;
import org.msgpack.core.MessagePacker;
import org.msgpack.core.MessageUnpacker;
Expand Down Expand Up @@ -103,9 +104,15 @@ public PresenceMessage[] handleResponseBody(String contentType, byte[] body) thr
messages = readJSON(body);
else if("application/x-msgpack".equals(contentType))
messages = readMsgpack(body);
if(messages != null)
for(PresenceMessage message : messages)
message.decode(opts);
if(messages != null) {
for (PresenceMessage message : messages) {
try {
message.decode(opts);
} catch (MessageDecodeException e) {
Log.e(TAG, e.errorInfo.message);
}
}
}
return messages;
} catch(IOException e) {
throw AblyException.fromThrowable(e);
Expand All @@ -116,4 +123,6 @@ else if("application/x-msgpack".equals(contentType))
}

private static BodyHandler<PresenceMessage> presenceResponseHandler = new PresenceBodyHandler(null);

private static final String TAG = PresenceSerializer.class.getName();
}
21 changes: 13 additions & 8 deletions lib/src/test/java/io/ably/lib/test/realtime/RealtimeCryptoMessageTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import javax.crypto.spec.IvParameterSpec;

import io.ably.lib.types.*;
import org.junit.AfterClass;
import org.junit.BeforeClass;
import org.junit.Test;
Expand All @@ -15,10 +16,6 @@
import io.ably.lib.test.common.Helpers;
import io.ably.lib.test.common.Setup;
import io.ably.lib.test.common.Setup.TestVars;
import io.ably.lib.types.AblyException;
import io.ably.lib.types.ChannelOptions;
import io.ably.lib.types.ClientOptions;
import io.ably.lib.types.Message;
import io.ably.lib.util.Base64Coder;
import io.ably.lib.util.Crypto;
import io.ably.lib.util.Crypto.CipherParams;
Expand Down Expand Up @@ -71,7 +68,9 @@ public void encrypt_message_128() {

/* decode (ie remove any base64 encoding) */
testMessage.decode(null);
encryptedMessage.decode(null);
try {
encryptedMessage.decode(null);
} catch (MessageDecodeException e) {}

/* reset channel cipher, to ensure it uses the given iv */
ChannelOptions options = new ChannelOptions() {{encrypted = true; cipherParams = params;}};
Expand Down Expand Up @@ -125,7 +124,9 @@ public void encrypt_message_256() {

/* decode (ie remove any base64 encoding) */
testMessage.decode(null);
encryptedMessage.decode(null);
try {
encryptedMessage.decode(null);
} catch (MessageDecodeException e) {}

/* reset channel cipher, to ensure it uses the given iv */
ChannelOptions options = new ChannelOptions() {{encrypted = true; cipherParams = params;}};
Expand Down Expand Up @@ -179,7 +180,9 @@ public void decrypt_message_128() {

/* decode (ie remove any base64 encoding) */
testMessage.decode(null);
encryptedMessage.decode(null);
try {
encryptedMessage.decode(null);
} catch (MessageDecodeException e) {}

/* reset channel cipher, to ensure it uses the given iv */
ChannelOptions options = new ChannelOptions() {{encrypted = true; cipherParams = params;}};
Expand Down Expand Up @@ -233,7 +236,9 @@ public void decrypt_message_256() {

/* decode (ie remove any base64 encoding) */
testMessage.decode(null);
encryptedMessage.decode(null);
try {
encryptedMessage.decode(null);
} catch (MessageDecodeException e) {}

/* reset channel cipher, to ensure it uses the given iv */
ChannelOptions options = new ChannelOptions() {{encrypted = true; cipherParams = params;}};
Expand Down
14 changes: 6 additions & 8 deletions lib/src/test/java/io/ably/lib/test/rest/RestCryptoTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,7 @@
import io.ably.lib.rest.Channel;
import io.ably.lib.test.common.Setup;
import io.ably.lib.test.common.Setup.TestVars;
import io.ably.lib.types.AblyException;
import io.ably.lib.types.ChannelOptions;
import io.ably.lib.types.ClientOptions;
import io.ably.lib.types.Message;
import io.ably.lib.types.PaginatedResult;
import io.ably.lib.types.*;
import io.ably.lib.util.Crypto;
import io.ably.lib.util.Crypto.CipherParams;

Expand Down Expand Up @@ -340,10 +336,12 @@ public void crypto_publish_key_mismatch() {
try {
ChannelOptions rx_channelOpts = new ChannelOptions() {{ encrypted = true; }};
Channel rx_publish = ably_text.channels.get("persisted:crypto_publish_key_mismatch", rx_channelOpts);
rx_publish.history(null);
fail("crypto_publish_key_mismatch: Expected exception");

PaginatedResult<Message> messages = rx_publish.history(new Param[] { new Param("direction", "backwards"), new Param("limit", "2") });
for (Message failedMessage: messages.items())
assertTrue("Check decrypt failure", failedMessage.encoding.contains("cipher"));
} catch (AblyException e) {
assertEquals("Expect decrypt padding failure", e.getCause().getClass(), javax.crypto.BadPaddingException.class);
fail("Didn't expect exception");
}
}

Expand Down