Merge pull request #263 from lsst/tickets/DM-42775

DM-42775: Fix segfault in FlagDefinitionList
lsst · Feb 5, 2024 · c19132b · c19132b
2 parents 1c40643 + bbd25ee
commit c19132b
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 1 deletion.
diff --git a/include/lsst/meas/base/FlagHandler.h b/include/lsst/meas/base/FlagHandler.h
@@ -77,10 +77,11 @@ class FlagDefinitionList {
         static FlagDefinitionList list;
         return list;
     }
+
     /**
      *  @brief get a reference to the FlagDefinition with specified index.
      */
-    FlagDefinition getDefinition(std::size_t index) const { return _vector[index]; }
+    FlagDefinition getDefinition(std::size_t index) const;
     /**
      *  @brief get a reference to the FlagDefinition with specified name.
      */

diff --git a/src/FlagHandler.cc b/src/FlagHandler.cc
@@ -30,6 +30,16 @@ namespace base {
 // so pybind11 can get the address of number_undefined
 constexpr std::size_t FlagDefinition::number_undefined;
 
+FlagDefinition FlagDefinitionList::getDefinition(std::size_t index) const {
+    if (index >= _vector.size()) {
+        throw std::out_of_range((boost::format("Cannot access index %d of length=%d flag definition list.") %
+                                 index % _vector.size())
+                                        .str());
+    } else {
+        return _vector[index];
+    }
+}
+
 FlagDefinition FlagDefinitionList::addFailureFlag(std::string const& doc) {
     return add(FlagHandler::getFailureFlagName(), doc);
 }

diff --git a/tests/test_FlagHandler.py b/tests/test_FlagHandler.py
@@ -347,6 +347,21 @@ def testSafeCentroider(self):
         self.assertTrue(source.get(self.algName + "_flag"))
         self.assertEqual(source.get("test_PythonPlugin_instFlux"), instFlux)
 
+    def testEmptyDefinitionList(self):
+        """Check that accessing a non-existent index in a flag definition list
+        raises instead of segfaulting.
+        """
+        definitions = FlagDefinitionList()
+        with self.assertRaisesRegex(IndexError, "Cannot access index 0 of length=0 flag definition list."):
+            definitions.getDefinition(0)
+
+        # Adding and accessing a flag still works
+        definitions.addFailureFlag()
+        self.assertEqual(definitions.getDefinition(0).name, "flag")
+
+        with self.assertRaisesRegex(IndexError, "Cannot access index 1 of length=1 flag definition list."):
+            definitions.getDefinition(1)
+
 
 class TestMemory(lsst.utils.tests.MemoryTestCase):
     pass