Handle invalid JSON properly. Fixes #2

msiedlarek · Dec 26, 2016 · 54ddd78 · 54ddd78
1 parent 98d3ebf
commit 54ddd78
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 20 deletions.
diff --git a/README.md b/README.md
@@ -29,8 +29,17 @@ Configuration can be passed either by Java system properties from command line o
 
 ## Supported messages
 
-The payload of all messages is either empty or a valid JSON element
-(possibly a privimite, like a single integer).
+The payload of all messages is either empty or a valid JSON element (possibly
+a privimite, like a single integer). This means, specifically, that if an
+argument is supposed to be a single string, it should be sent in double quotes.
+
+Example valid message payloads:
+
+* `123`
+* `true`
+* `"notepad.exe"`
+* `[1024, 768]`
+* `["notepad.exe", "C:\file.txt", "C:\"]`
 
 ### Broadcasted status
 

diff --git a/src/main/java/com/fatico/winthing/messaging/Engine.java b/src/main/java/com/fatico/winthing/messaging/Engine.java
@@ -5,6 +5,7 @@
 import com.google.common.base.Charsets;
 import com.google.gson.Gson;
 import com.google.gson.JsonElement;
+import com.google.gson.JsonSyntaxException;
 import com.google.inject.Inject;
 import com.typesafe.config.Config;
 import org.eclipse.paho.client.mqttv3.IMqttAsyncClient;
@@ -178,7 +179,25 @@ public void connectionLost(final Throwable throwable) {
     }
 
     @Override
-    public void messageArrived(String topic, final MqttMessage mqttMessage) throws Exception {
+    public void messageArrived(final String topic, final MqttMessage mqttMessage) throws Exception {
+        try {
+            handleMessage(topic, mqttMessage);
+        } catch (final Throwable throwable) {
+            logger.error(
+                    "Error while handling message " + topic
+                            + "(" + new String(mqttMessage.getPayload(), CHARSET) + "): "
+                            + throwable.getMessage(),
+                    throwable
+            );
+        }
+    }
+
+    @Override
+    public void deliveryComplete(final IMqttDeliveryToken token) {
+        // Do nothing.
+    }
+
+    private void handleMessage(String topic, final MqttMessage mqttMessage) throws Exception {
         if (!topic.startsWith(topicPrefix)) {
             return;
         }
@@ -194,41 +213,42 @@ public void messageArrived(String topic, final MqttMessage mqttMessage) throws E
         if (payloadBytes.length == 0) {
             payload = null;
         } else {
-            payload = gson.fromJson(new String(payloadBytes, CHARSET), JsonElement.class);
+            try {
+                payload = gson.fromJson(new String(payloadBytes, CHARSET), JsonElement.class);
+            } catch (final JsonSyntaxException exception) {
+                logger.error("Invalid JSON received for: {}", topic);
+                return;
+            }
         }
 
         final Message message = new Message(
-            topic,
-            payload,
-            QualityOfService.values()[mqttMessage.getQos()],
-            mqttMessage.isRetained()
+                topic,
+                payload,
+                QualityOfService.values()[mqttMessage.getQos()],
+                mqttMessage.isRetained()
         );
 
         logger.debug(
-            "Received: {}({})",
-            message.getTopic(),
-            message.getPayload().isPresent() ? message.getPayload().get().toString() : ""
+                "Received: {}({})",
+                message.getTopic(),
+                message.getPayload().isPresent() ? message.getPayload().get().toString() : ""
         );
 
         for (final Consumer<Message> consumer : consumers) {
             try {
                 consumer.accept(message);
             } catch (final Exception exception) {
                 logger.error(
-                    "Error while processing {}({}): {}",
-                    message.getTopic(),
-                    message.getPayload().isPresent() ? message.getPayload().get().toString() : "",
-                    exception.getMessage()
+                        "Error while processing {}({}): {}",
+                        message.getTopic(),
+                        message.getPayload().isPresent()
+                                ? message.getPayload().get().toString() : "",
+                        exception.getMessage()
                 );
             }
         }
     }
 
-    @Override
-    public void deliveryComplete(final IMqttDeliveryToken token) {
-        // Do nothing.
-    }
-
     private MqttMessage serialize(final Message message) {
         final byte[] payload;
         if (message.getPayload().isPresent()) {