Guard against assert in jsoncpp, throwing errors instead. Update crude_parse to new payload_configuration schema

src/UKHASExtractor.cxx

@@ -285,6 +285,9 @@ static void extract_fields(Json::Value &data, const Json::Value &fields,
 
     while (field != fields.end() && part != parts.end())
     {
+        if (!(*field).isObject())
+            throw runtime_error("Invalid configuration (field not an object)");
+
         const string key = (*field)["name"].asString();
         const string value = (*part);
 
@@ -317,6 +320,9 @@ static void attempt_settings(Json::Value &data, const Json::Value &sentence,
                              const string &checksum_name,
                              const vector<string> &parts)
 {
+    if (!sentence.isObject())
+        throw runtime_error("Invalid configuration (sentence not an object)");
+
     const Json::Value &fields = sentence["fields"];
 
     const string callsign = sentence["payload"].asString();
@@ -343,6 +349,11 @@ Json::Value UKHASExtractor::crude_parse()
 
     const Json::Value &settings = *settings_ptr;
 
+    if (!settings.isObject())
+        /* note: Json::Value::null.isObject() == true */
+        throw runtime_error("Invalid configuration: "
+                "settings is not an object");
+
     string data, checksum;
     split_string(buffer, &data, &checksum);
 
@@ -355,17 +366,19 @@ Json::Value UKHASExtractor::crude_parse()
 
     Json::Value basic(Json::objectValue);
     cook_basic(basic, buffer, parts[0]);
-    const Json::Value &sentence = settings["sentence"];
+    const Json::Value &sentences = settings["sentences"];
 
-    /* If array: multiple sentence settings to try with.
+    if (!sentences.isNull())
-     * No settings? No problem; we can still test the checksum */
-    if (!sentence.isNull() && sentence.isArray())
     {
+        if (!sentences.isArray())
+            throw runtime_error("Invalid configuration: "
+                    "sentences is not an array");
+
         /* Silence errors, and only log them if all attempts fail */
         vector<string> errors;
 
-        for (Json::Value::iterator it = sentence.begin();
+        for (Json::Value::iterator it = sentences.begin();
-             it != sentence.end(); it++)
+             it != sentences.end(); it++)
         {
             try
             {
@@ -387,20 +400,6 @@ Json::Value UKHASExtractor::crude_parse()
             mgr->status("UKHAS Extractor: " + (*it));
         }
     }
-    else if (!sentence.isNull() && sentence.isObject())
-    {
-        try
-        {
-            Json::Value data(basic);
-            attempt_settings(data, sentence, checksum_name, parts);
-            return data;
-        }
-        catch (runtime_error e)
-        {
-            mgr->status("UKHAS Extractor: full parse failed: " +
-                        string(e.what()));
-        }
-    }
 
     basic["_basic"] = true;
     return basic;

src/Uploader.cxx

@@ -113,14 +113,15 @@ static void payload_telemetry_merge(Json::Value &doc,
                                     const string &callsign,
                                     Json::Value &receiver_info)
 {
+    if (!doc.isObject() || !doc["data"].isObject() ||
+            !doc["receivers"].isObject())
+        throw runtime_error("Server gave us an invalid payload telemetry doc");
+
     string other_b64 = doc["data"]["_raw"].asString();
 
     if (!other_b64.length() || other_b64 != data_b64)
         throw CollisionError();
 
-    if (!doc["receivers"].isObject())
-        throw runtime_error("Server gave us an invalid payload telemetry doc");
-
     doc["receivers"][callsign] = receiver_info;
 }
 
@@ -260,17 +261,31 @@ vector<Json::Value> *Uploader::flights()
     vector<Json::Value> *result = new vector<Json::Value>;
     auto_ptr< vector<Json::Value> > result_destroyer(result);
 
+    if (!response->isObject())
+        throw runtime_error("Invalid response: was not an object");
+
     const Json::Value &rows = (*response)["rows"];
     Json::Value::const_iterator it;
 
+    if (!rows.isArray())
+        throw runtime_error("Invalid response: rows was not an array");
+
     result->reserve(rows.size());
     Json::Value *current_pcfg_list = NULL;
 
     for (it = rows.begin(); it != rows.end(); it++)
     {
         const Json::Value &row = *it;
+        if (!row.isObject())
+            throw runtime_error("Invalid response: row was not an object");
+
         const Json::Value &key = row["key"], &doc = row["doc"];
-        bool is_pcfg = (key[2u].asInt() == 1);
+
+        if (!doc.isObject() || !key.isArray() || key.size() != 3 ||
+                !key[2u].isIntegral())
+            throw runtime_error("Invalid response: bad key or doc in row");
+
+        bool is_pcfg = key[2u].asBool();
 
         if (!is_pcfg)
         {
@@ -304,13 +319,21 @@ vector<Json::Value> *Uploader::payloads()
     vector<Json::Value> *result = new vector<Json::Value>;
     auto_ptr< vector<Json::Value> > result_destroyer(result);
 
+    if (!response->isObject())
+        throw runtime_error("Invalid response: was not an object");
+
     const Json::Value &rows = (*response)["rows"];
     Json::Value::const_iterator it;
 
+    if (!rows.isArray())
+        throw runtime_error("Invalid response: rows was not an array");
+
     result->reserve(rows.size());
 
     for (it = rows.begin(); it != rows.end(); it++)
     {
+        if (!(*it).isObject())
+            throw runtime_error("Invalid response: doc was not an object");
         result->push_back((*it)["doc"]);
     }
 

tests/test_extractor.py

@@ -232,7 +232,7 @@ class TestUKHASExtractor:
                             "mypayload")
 
     crude_parse_flight_doc = {
-        "sentence": {
+        "sentences": [ {
             "payload": "TESTING",
             "checksum": "crc16-ccitt",
             "fields": [
@@ -240,7 +240,7 @@ class TestUKHASExtractor:
                 {"name": "field_b"},
                 {"name": "field_c"}
             ],
-        }
+        } ]
     }
 
     def test_crude_parse_config(self):
@@ -257,26 +257,30 @@ class TestUKHASExtractor:
 
     def test_crude_checks(self):
         checks = [
-            ("$$TESTING,a,b,c*asdfg\n", "invalid checksum len"),
+            ("$$TESTING,a,b,c*asdfg\n", "invalid checksum len", False),
-            ("$$TESTING,a,b,c*45\n", "invalid checksum: expected 1A"),
+            ("$$TESTING,a,b,c*45\n", "invalid checksum: expected 1A", False),
-            ("$$TESTING,a,b,c*AAAA\n", "invalid checksum: expected BEBC"),
+            ("$$TESTING,a,b,c*AAAA\n", "invalid checksum: expected BEBC",
-            ("$$TESTING,val_a,val_b*4EB7\n", "incorrect number of fields"),
+                False),
-            ("$$TESTING,a,b,c*1A\n", "wrong checksum type"),
+            ("$$TESTING,val_a,val_b*4EB7\n", "incorrect number of fields",
-            ("$$ANOTHER,a,b,c*2355\n", "incorrect callsign"),
+                True),
+            ("$$TESTING,a,b,c*1A\n", "wrong checksum type", True),
+            ("$$ANOTHER,a,b,c*2355\n", "incorrect callsign", True),
         ]
 
         self.extr.set_current_payload(self.crude_parse_flight_doc)
 
-        for (string, error) in checks:
+        for (string, error, full_parse_line) in checks:
             self.extr.push(string)
             self.extr.check_status("start delim")
             self.extr.check_upload(string)
             self.extr.check_status("extracted")
+            if full_parse_line:
+                self.extr.check_status("full parse failed:")
             self.extr.check_status(error)
             self.extr.check_data()
 
     multi_config_flight_doc = {
-        "sentence": [
+        "sentences": [
             { "payload": "AWKWARD",
               "checksum": "crc16-ccitt",
               "fields": [ {"name": "fa"}, {"name": "fo"}, {"name": "fc"} ] },
@@ -307,7 +311,7 @@ class TestUKHASExtractor:
                               "fa": "extended", "fo": "other", "fc": "data"})
 
     ddmmmmmm_flight_doc = {
-        "sentence": {
+        "sentences": [ {
             "payload": "TESTING",
             "checksum": "crc16-ccitt",
             "fields": [
@@ -317,7 +321,7 @@ class TestUKHASExtractor:
                  "format":"ddmm.mm"},
                 {"name": "field_b"}
             ],
-        }
+        } ]
     }
 
     def test_ddmmmmmm(self):