Skip to content

Commit

Permalink
fix: ensure the payload format is valid
Browse files Browse the repository at this point in the history
This commit should prevent some NPE issues encountered after the
parsing of the packet.

Related:

- #642
- #609
- #505

Backported from e8ffe9d
  • Loading branch information
darrachequesne committed Jul 10, 2022
1 parent 8bd35da commit 8664499
Show file tree
Hide file tree
Showing 6 changed files with 72 additions and 42 deletions.
21 changes: 9 additions & 12 deletions src/main/java/io/socket/client/Manager.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import io.socket.backo.Backoff;
import io.socket.emitter.Emitter;
import io.socket.parser.DecodingException;
import io.socket.parser.IOParser;
import io.socket.parser.Packet;
import io.socket.parser.Parser;
Expand Down Expand Up @@ -370,10 +371,14 @@ private void onopen() {
@Override
public void call(Object... objects) {
Object data = objects[0];
if (data instanceof String) {
Manager.this.ondata((String)data);
} else if (data instanceof byte[]) {
Manager.this.ondata((byte[])data);
try {
if (data instanceof String) {
Manager.this.decoder.add((String) data);
} else if (data instanceof byte[]) {
Manager.this.decoder.add((byte[]) data);
}
} catch (DecodingException e) {
logger.fine("error while decoding the packet: " + e.getMessage());
}
}
}));
Expand Down Expand Up @@ -419,14 +424,6 @@ private void onpong() {
null != this.lastPing ? new Date().getTime() - this.lastPing.getTime() : 0);
}

private void ondata(String data) {
this.decoder.add(data);
}

private void ondata(byte[] data) {
this.decoder.add(data);
}

private void ondecoded(Packet packet) {
this.emit(EVENT_PACKET, packet);
}
Expand Down
7 changes: 7 additions & 0 deletions src/main/java/io/socket/parser/DecodingException.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
package io.socket.parser;

public class DecodingException extends RuntimeException {
public DecodingException(String message) {
super(message);
}
}
44 changes: 35 additions & 9 deletions src/main/java/io/socket/parser/IOParser.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package io.socket.parser;

import io.socket.hasbinary.HasBinary;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.json.JSONTokener;

import java.util.ArrayList;
Expand All @@ -14,10 +16,6 @@ final public class IOParser implements Parser {

private static final Logger logger = Logger.getLogger(IOParser.class.getName());

private static Packet<String> error() {
return new Packet<String>(ERROR, "parser error");
}

private IOParser() {}

final public static class Encoder implements Parser.Encoder {
Expand Down Expand Up @@ -126,12 +124,16 @@ private static Packet decodeString(String str) {
int i = 0;
int length = str.length();

Packet<Object> p = new Packet<Object>(Character.getNumericValue(str.charAt(0)));
Packet<Object> p = new Packet<>(Character.getNumericValue(str.charAt(0)));

if (p.type < 0 || p.type > types.length - 1) return error();
if (p.type < 0 || p.type > types.length - 1) {
throw new DecodingException("unknown packet type " + p.type);
}

if (BINARY_EVENT == p.type || BINARY_ACK == p.type) {
if (!str.contains("-") || length <= i + 1) return error();
if (!str.contains("-") || length <= i + 1) {
throw new DecodingException("illegal attachments");
}
StringBuilder attachments = new StringBuilder();
while (str.charAt(++i) != '-') {
attachments.append(str.charAt(i));
Expand Down Expand Up @@ -170,7 +172,7 @@ private static Packet decodeString(String str) {
try {
p.id = Integer.parseInt(id.toString());
} catch (NumberFormatException e){
return error();
throw new DecodingException("invalid payload");
}
}
}
Expand All @@ -181,7 +183,10 @@ private static Packet decodeString(String str) {
p.data = new JSONTokener(str.substring(i)).nextValue();
} catch (JSONException e) {
logger.log(Level.WARNING, "An error occured while retrieving data from JSONTokener", e);
return error();
throw new DecodingException("invalid payload");
}
if (!isPayloadValid(p.type, p.data)) {
throw new DecodingException("invalid payload");
}
}

Expand All @@ -191,6 +196,27 @@ private static Packet decodeString(String str) {
return p;
}

private static boolean isPayloadValid(int type, Object payload) {
switch (type) {
case Parser.CONNECT:
return payload instanceof JSONObject;
case Parser.ERROR:
return payload instanceof String;
case Parser.DISCONNECT:
return payload == null;
case Parser.EVENT:
case Parser.BINARY_EVENT:
return payload instanceof JSONArray
&& ((JSONArray) payload).length() > 0
&& !((JSONArray) payload).isNull(0);
case Parser.ACK:
case Parser.BINARY_ACK:
return payload instanceof JSONArray;
default:
return false;
}
}

@Override
public void destroy() {
if (this.reconstructor != null) {
Expand Down
26 changes: 13 additions & 13 deletions src/test/java/io/socket/parser/ByteArrayTest.java
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
package io.socket.parser;

import io.socket.emitter.Emitter;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

import java.nio.charset.Charset;
import java.nio.charset.StandardCharsets;

import static java.util.Arrays.asList;
import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;

Expand All @@ -20,29 +20,29 @@ public class ByteArrayTest {

@Test
public void encodeByteArray() {
Packet<byte[]> packet = new Packet<byte[]>(Parser.BINARY_EVENT);
packet.data = "abc".getBytes(Charset.forName("UTF-8"));
Packet<JSONArray> packet = new Packet<>(Parser.BINARY_EVENT);
packet.data = new JSONArray(asList("abc", "abc".getBytes(StandardCharsets.UTF_8)));
packet.id = 23;
packet.nsp = "/cool";
Helpers.testBin(packet);
}

@Test
public void encodeByteArray2() {
Packet<byte[]> packet = new Packet<byte[]>(Parser.BINARY_EVENT);
packet.data = new byte[2];
Packet<JSONArray> packet = new Packet<>(Parser.BINARY_EVENT);
packet.data = new JSONArray(asList("2", new byte[] { 0, 1 }));
packet.id = 0;
packet.nsp = "/";
Helpers.testBin(packet);
}

@Test
public void encodeByteArrayDeepInJson() throws JSONException {
JSONObject data = new JSONObject("{a: \"hi\", b: {}, c: {a: \"bye\", b: {}}}");
data.getJSONObject("b").put("why", new byte[3]);
data.getJSONObject("c").getJSONObject("b").put("a", new byte[6]);
JSONArray data = new JSONArray("[{a: \"hi\", b: {}, c: {a: \"bye\", b: {}}}]");
data.getJSONObject(0).getJSONObject("b").put("why", new byte[3]);
data.getJSONObject(0).getJSONObject("c").getJSONObject("b").put("a", new byte[6]);

Packet<JSONObject> packet = new Packet<JSONObject>(Parser.BINARY_EVENT);
Packet<JSONArray> packet = new Packet<>(Parser.BINARY_EVENT);
packet.data = data;
packet.id = 999;
packet.nsp = "/deep";
Expand All @@ -51,10 +51,10 @@ public void encodeByteArrayDeepInJson() throws JSONException {

@Test
public void encodeDeepBinaryJSONWithNullValue() throws JSONException {
JSONObject data = new JSONObject("{a: \"b\", c: 4, e: {g: null}, h: null}");
data.put("h", new byte[9]);
JSONArray data = new JSONArray("[{a: \"b\", c: 4, e: {g: null}, h: null}]");
data.getJSONObject(0).put("h", new byte[9]);

Packet<JSONObject> packet = new Packet<JSONObject>(Parser.BINARY_EVENT);
Packet<JSONArray> packet = new Packet<>(Parser.BINARY_EVENT);
packet.data = data;
packet.nsp = "/";
packet.id = 600;
Expand Down
13 changes: 5 additions & 8 deletions src/test/java/io/socket/parser/Helpers.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package io.socket.parser;

import io.socket.emitter.Emitter;
import org.json.JSONArray;
import org.json.JSONException;
import org.json.JSONObject;
Expand All @@ -10,6 +9,7 @@

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;

@RunWith(JUnit4.class)
public class Helpers {
Expand All @@ -35,13 +35,10 @@ public void call(Packet packet) {

public static void testDecodeError(final String errorMessage) {
Parser.Decoder decoder = new IOParser.Decoder();
decoder.onDecoded(new IOParser.Decoder.Callback() {
@Override
public void call(Packet packet) {
assertPacket(errorPacket, packet);
}
});
decoder.add(errorMessage);
try {
decoder.add(errorMessage);
fail();
} catch (DecodingException e) {}
}

@SuppressWarnings("unchecked")
Expand Down
3 changes: 3 additions & 0 deletions src/test/java/io/socket/parser/ParserTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,5 +63,8 @@ public void decodeInError() throws JSONException {
Helpers.testDecodeError(Parser.EVENT + "2sd");
// event with invalid json data
Helpers.testDecodeError(Parser.EVENT + "2[\"a\",1,{asdf}]");
Helpers.testDecodeError(Parser.EVENT + "2{}");
Helpers.testDecodeError(Parser.EVENT + "2[]");
Helpers.testDecodeError(Parser.EVENT + "2[null]");
}
}

0 comments on commit 8664499

Please sign in to comment.