From 83bbe4ec37cf2a82c4c3c3694cd78e02a2fd3dfe Mon Sep 17 00:00:00 2001
From: "Dustin J. Mitchell" <dustin@v.igoro.us>
Date: Thu, 25 Jan 2024 08:01:20 -0500
Subject: [PATCH] Fix invalid imports in `[{..}]` form (#3241)

* Fix invalid imports in `[{..}]` form

Before this change, if an import of data that takes the form of a JSON
array of JSON objects results in an error, the import would be re-tried
assuming that each line of the file is a JSON object (the old format).
However, no check was made that the value actually was an object before
casting it to `json::object`, resulting in a segfault.

This adds the check, and handles the failure with a useful error message
(from the first attempt to parse the file).
---
 src/commands/CmdImport.cpp | 25 ++++++++++++++++++-------
 test/import.t              |  6 ++++++
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/src/commands/CmdImport.cpp b/src/commands/CmdImport.cpp
index a663324c8..4bfe3072b 100644
--- a/src/commands/CmdImport.cpp
+++ b/src/commands/CmdImport.cpp
@@ -133,7 +133,7 @@ int CmdImport::import (const std::string& input)
   }
 
   // If an exception is caught, then it is because the free-form JSON
-  // objects/array above failed to parse. This is an indication that the input
+  // objects/array above failed to parse. This may be an indication that the input
   // is an old-style line-by-line set of JSON objects, because both an array of
   // objects, and a single object have failed to parse..
   //
@@ -142,19 +142,30 @@ int CmdImport::import (const std::string& input)
   //   { ... }
   catch (std::string& e)
   {
+    // Make a very cursory check for the old-style format.
+    if (input[0] != '{') {
+      throw e;
+    }
+
+    // Read the entire file, so that errors do not result in a partial import.
+    std::vector<std::unique_ptr<json::object>> objects;
     for (auto& line : split (input, '\n'))
     {
       if (line.length ())
       {
         json::value* root = json::parse (line);
-        if (root)
-        {
-          importSingleTask ((json::object*) root);
-          ++count;
-          delete root;
-        }
+        if (root && root->type () == json::j_object)
+          objects.push_back (std::unique_ptr<json::object> ((json::object *)root));
+        else
+          throw format ("Invalid JSON: {1}", line);
       }
     }
+
+    // Import the tasks.
+    for (auto& root : objects) {
+      importSingleTask (root.get());
+      ++count;
+    }
   }
 
   return count;
diff --git a/test/import.t b/test/import.t
index 44bade529..83b17b2ec 100755
--- a/test/import.t
+++ b/test/import.t
@@ -279,6 +279,12 @@ class TestImportValidate(TestCase):
         code, out, err = self.t.runError("import", input=j)
         self.assertIn("Not a valid UUID", err)
 
+    def test_import_invalid_uuid_array(self):
+        """Verify invalid UUID is caught in array form"""
+        j = '[{"uuid":"1", "description":"bad"}]'
+        code, out, err = self.t.runError("import", input=j)
+        self.assertIn("Not a valid UUID", err)
+
     def test_import_invalid_uuid2(self):
         """Verify invalid UUID is caught, part two"""
         # UUID is the right length, but with s/-/0/.