From 296aef87999eb255c953ca28d034bb2afba563f5 Mon Sep 17 00:00:00 2001 From: Nicolas Milliard Date: Mon, 16 Mar 2015 15:13:40 -0700 Subject: [PATCH] Enhance Parser decode * Avoid using expected IndexOutOfBound exception * Add few more error checks * Add few malformated message error test cases --- .../github/nkzawa/socketio/parser/Parser.java | 59 ++++++++++--------- .../nkzawa/socketio/parser/Helpers.java | 13 ++++ .../nkzawa/socketio/parser/ParserTest.java | 19 +++++- 3 files changed, 62 insertions(+), 29 deletions(-) diff --git a/src/main/java/com/github/nkzawa/socketio/parser/Parser.java b/src/main/java/com/github/nkzawa/socketio/parser/Parser.java index e6f15fa..95c69f4 100644 --- a/src/main/java/com/github/nkzawa/socketio/parser/Parser.java +++ b/src/main/java/com/github/nkzawa/socketio/parser/Parser.java @@ -57,10 +57,10 @@ public class Parser { "CONNECT", "DISCONNECT", "EVENT", - "BINARY_EVENT", "ACK", - "BINARY_ACK", "ERROR", + "BINARY_EVENT", + "BINARY_ACK" }; @@ -172,11 +172,13 @@ public class Parser { private static Packet decodeString(String str) { Packet p = new Packet(); int i = 0; + int length = str.length(); p.type = Character.getNumericValue(str.charAt(0)); if (p.type < 0 || p.type > types.length - 1) return error(); if (BINARY_EVENT == p.type || BINARY_ACK == p.type) { + if (!str.contains("-") || length <= i + 1) return error(); StringBuilder attachments = new StringBuilder(); while (str.charAt(++i) != '-') { attachments.append(str.charAt(i)); @@ -184,48 +186,49 @@ public class Parser { p.attachments = Integer.parseInt(attachments.toString()); } - if (str.length() > i + 1 && '/' == str.charAt(i + 1)) { + if (length > i + 1 && '/' == str.charAt(i + 1)) { StringBuilder nsp = new StringBuilder(); while (true) { ++i; char c = str.charAt(i); if (',' == c) break; nsp.append(c); - if (i + 1 == str.length()) break; + if (i + 1 == length) break; } p.nsp = nsp.toString(); } else { p.nsp = "/"; } - Character next; - try { - next = str.charAt(i + 1); - } catch (IndexOutOfBoundsException e) { - next = Character.UNASSIGNED; - } - if (Character.UNASSIGNED != next && Character.getNumericValue(next) > -1) { - StringBuilder id = new StringBuilder(); - while (true) { - ++i; - char c = str.charAt(i); - if (Character.getNumericValue(c) < 0) { - --i; - break; + if (length > i + 1){ + Character next = str.charAt(i + 1); + if (Character.getNumericValue(next) > -1) { + StringBuilder id = new StringBuilder(); + while (true) { + ++i; + char c = str.charAt(i); + if (Character.getNumericValue(c) < 0) { + --i; + break; + } + id.append(c); + if (i + 1 == length) break; + } + try { + p.id = Integer.parseInt(id.toString()); + } catch (NumberFormatException e){ + return error(); } - id.append(c); - if (i + 1 == str.length()) break; } - p.id = Integer.parseInt(id.toString()); } - try { - str.charAt(++i); - p.data = new JSONTokener(str.substring(i)).nextValue(); - } catch (IndexOutOfBoundsException e) { - // do nothing - } catch (JSONException e) { - return error(); + if (length > i + 1){ + try { + str.charAt(++i); + p.data = new JSONTokener(str.substring(i)).nextValue(); + } catch (JSONException e) { + return error(); + } } logger.fine(String.format("decoded %s as %s", str, p)); diff --git a/src/test/java/com/github/nkzawa/socketio/parser/Helpers.java b/src/test/java/com/github/nkzawa/socketio/parser/Helpers.java index d3fb3f7..505b0d2 100644 --- a/src/test/java/com/github/nkzawa/socketio/parser/Helpers.java +++ b/src/test/java/com/github/nkzawa/socketio/parser/Helpers.java @@ -15,6 +15,7 @@ import static org.junit.Assert.assertThat; public class Helpers { private static Parser.Encoder encoder = new Parser.Encoder(); + private static Packet errorPacket = new Packet(Parser.ERROR, "parser error"); public static void test(final Packet obj) { encoder.encode(obj, new Parser.Encoder.Callback() { @@ -33,6 +34,18 @@ public class Helpers { }); } + public static void testDecodeError(final String errorMessage) { + Parser.Decoder decoder = new Parser.Decoder(); + decoder.on(Parser.Decoder.EVENT_DECODED, new Emitter.Listener() { + @Override + public void call(Object... args) { + Packet packet = (Packet)args[0]; + assertPacket(errorPacket, packet); + } + }); + decoder.add(errorMessage); + } + public static void testBin(final Packet obj) { final Object originalData = obj.data; encoder.encode(obj, new Parser.Encoder.Callback() { diff --git a/src/test/java/com/github/nkzawa/socketio/parser/ParserTest.java b/src/test/java/com/github/nkzawa/socketio/parser/ParserTest.java index 372dd32..9f355e6 100644 --- a/src/test/java/com/github/nkzawa/socketio/parser/ParserTest.java +++ b/src/test/java/com/github/nkzawa/socketio/parser/ParserTest.java @@ -11,7 +11,6 @@ public class ParserTest { private static Parser.Encoder encoder = new Parser.Encoder(); - @Test public void encodeConnection() { Packet packet = new Packet(Parser.CONNECT); @@ -47,4 +46,22 @@ public class ParserTest { packet.nsp = "/"; Helpers.test(packet); } + + @Test + public void decodeInError() throws JSONException { + // Random string + Helpers.testDecodeError("asdf"); + // Unknown type + Helpers.testDecodeError(Parser.types.length + "asdf"); + // Binary event with no `-` + Helpers.testDecodeError(Parser.BINARY_EVENT + "asdf"); + // Binary ack with no `-` + Helpers.testDecodeError(Parser.BINARY_ACK + "asdf"); + // Binary event with no attachment + Helpers.testDecodeError(String.valueOf(Parser.BINARY_EVENT)); + // event non numeric id + Helpers.testDecodeError(Parser.EVENT + "2sd"); + // event with invalid json data + Helpers.testDecodeError(Parser.EVENT + "2[\"a\",1,{asdf}]"); + } }