Merge "platform: error handling for bad JSON from glaukus"
diff --git a/jsonpoll/jsonpoll.py b/jsonpoll/jsonpoll.py
index 11f9c64..cc38875 100755
--- a/jsonpoll/jsonpoll.py
+++ b/jsonpoll/jsonpoll.py
@@ -87,9 +87,8 @@
   def WriteToStderr(self, msg, is_json=False):
     """Write a message to stderr."""
     if is_json:
-      json_data = json.loads(msg)
       flat_data = []
-      self._FlatObject('', json_data, flat_data)
+      self._FlatObject('', msg, flat_data)
       # Make the json easier to parse from the logs.
       for s in flat_data:
         sys.stderr.write('%s\n' % s)
@@ -118,7 +117,7 @@
                                os.path.dirname(output_file))
             continue
           tmpfile = fd.name
-          fd.write(response)
+          fd.write(json.dumps(response))
           fd.flush()
           os.fsync(fd.fileno())
           try:
@@ -131,11 +130,22 @@
         if os.path.exists(tmpfile):
           os.unlink(tmpfile)
 
+  def ParseJSONFromResponse(self, response):
+    try:
+      json_resp = json.loads(response)
+    except UnicodeDecodeError as ex:
+      self.WriteToStderr('Non-UTF8 character in HTTP response: %s', ex)
+      return None
+    except ValueError as ex:
+      self.WriteToStderr('Failed to parse JSON from HTTP response: %s', ex)
+      return None
+    return json_resp
+
   def GetHttpResponse(self, url):
     """Creates a request and retrieves the response from a web server."""
     try:
       handle = urllib2.urlopen(url, timeout=self._SOCKET_TIMEOUT_SECS)
-      response = handle.read()
+      response = self.ParseJSONFromResponse(handle.read())
     except socket.timeout as ex:
       self.WriteToStderr('Connection to %s timed out after %d seconds: %s\n'
                          % (url, self._SOCKET_TIMEOUT_SECS, ex))
@@ -143,9 +153,11 @@
     except urllib2.URLError as ex:
       self.WriteToStderr('Connection to %s failed: %s\n' % (url, ex.reason))
       return None
+
     # Write the response to stderr so it will be uploaded with the other system
     # log files. This will allow turbogrinder to alert on the radio subsystem.
-    self.WriteToStderr(response, is_json=True)
+    if response is not None:
+      self.WriteToStderr(response, is_json=True)
     return response
 
   def CreateDirs(self, dir_to_create):
diff --git a/jsonpoll/jsonpoll_test.py b/jsonpoll/jsonpoll_test.py
index f4f0240..1ccd764 100644
--- a/jsonpoll/jsonpoll_test.py
+++ b/jsonpoll/jsonpoll_test.py
@@ -56,7 +56,7 @@
     self.get_response_called = True
     if self.generate_empty_response:
       return None
-    return json.dumps(JSON_RESPONSE)
+    return self.ParseJSONFromResponse(self.json_response)
 
 
 class JsonPollTest(unittest.TestCase):
@@ -73,6 +73,7 @@
   def setUp(self):
     self.CreateTempFile()
     self.poller = FakeJsonPoll('fakehost.blah', 31337, 1)
+    self.poller.json_response = json.dumps(JSON_RESPONSE)
     self.poller.error_count = 0
     self.poller.generate_empty_response = False
 
@@ -90,7 +91,7 @@
     # equivalent JSON representation we wrote out from the mock.
     with open(self.output_file, 'r') as f:
       output = ''.join(line.rstrip() for line in f)
-      self.assertEqual(json.dumps(JSON_RESPONSE), output)
+      self.assertEqual(JSON_RESPONSE, json.loads(output))
 
   def testRequestStatsFailureToCreateDirOutput(self):
     self.poller.paths_to_statfiles = {'fake/url': '/root/cannotwrite'}
@@ -107,7 +108,7 @@
   def testCachedRequestStats(self):
     # Set the "last_response" as our mock output. This should mean we do not
     # write anything to the output file.
-    self.poller.last_response = json.dumps(JSON_RESPONSE)
+    self.poller.last_response = JSON_RESPONSE
 
     # Create a fake entry in the paths_to_stats map.
     self.poller.paths_to_statfiles = {'fake/url': self.output_file}
@@ -127,5 +128,39 @@
     want = ['base/key1=1', 'base/key2/key3=3', 'base/key2/key4=4']
     self.assertEqual(got.sort(), want.sort())
 
+  def testJSONParsing(self):
+    # { "key": "value" }
+    start_json = ' { "key" : "'
+    euro = u'\u20AC'
+    end_json = '" }'
+
+    # Test for empty JSON
+    self.poller.json_response = ''
+    self.assertEquals(self.poller.GetHttpResponse('fake/url'), None)
+
+    # Test for broken JSON
+    self.poller.json_response = start_json
+    self.assertEquals(self.poller.GetHttpResponse('fake/url'), None)
+    self.poller.json_response = end_json
+    self.assertEquals(self.poller.GetHttpResponse('fake/url'), None)
+    self.poller.json_response = start_json + end_json + end_json
+    self.assertEquals(self.poller.GetHttpResponse('fake/url'), None)
+
+    # The json library (dumps/loads) assumes strings as UTF-8
+    # Need to fail gracefully when wrong encoding is given
+
+    # Normal ascii
+    incoming_json = start_json + 'ascii-value' + end_json
+    self.poller.json_response = incoming_json
+    self.assertNotEquals(self.poller.GetHttpResponse('fake/url'), None)
+
+    # Unicode utf-8: '\xE2 \x82 \xAC' == euro_sign
+    self.poller.json_response = start_json + euro.encode('utf-8') + end_json
+    self.assertNotEquals(self.poller.GetHttpResponse('fake/url'), None)
+
+    # Unicode utf-16: '\x20\xAC' == euro_sign, should fail
+    self.poller.json_response = start_json + euro.encode('utf-16') + end_json
+    self.assertEquals(self.poller.GetHttpResponse('fake/url'), None)
+
 if __name__ == '__main__':
   unittest.main()