From e4acb387bb0dbd2cdda77ef4ce829f7a21328c59 Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Wed, 15 Nov 2023 13:58:46 +0000
Subject: [PATCH] MDL-80127 question engine: prevent fatal errors from old bad
 data

---
 question/engine/states.php                   | 17 +++++++++++++++--
 question/engine/tests/questionstate_test.php | 11 +++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/question/engine/states.php b/question/engine/states.php
index e48f0d553b0..f13d700175a 100644
--- a/question/engine/states.php
+++ b/question/engine/states.php
@@ -111,10 +111,23 @@ abstract class question_state {
     }
 
     /**
+     * Get the instance of this class for a given state name.
+     *
      * @param string $name a state name.
-     * @return question_state the state with that name.
+     * @return question_state|null the state with that name. (Null only in an exceptional case.)
      */
-    public static function get($name) {
+    public static function get(string $name): ?question_state {
+        // In the past, there was a bug where null states got stored
+        // in the database as an empty string, which was wrong because
+        // the state column should be NOT NULL.
+        // That is no longer possible, but we need to avoid exceptions
+        // for people with old bad data in their database.
+        if ($name === '') {
+            debugging('Attempt to create a state from an empty string. ' .
+                'This is probably a sign of bad data in your database. See MDL-80127.');
+            return null;
+        }
+
         return self::$$name;
     }
 
diff --git a/question/engine/tests/questionstate_test.php b/question/engine/tests/questionstate_test.php
index 85178ac8043..332fac84fa0 100644
--- a/question/engine/tests/questionstate_test.php
+++ b/question/engine/tests/questionstate_test.php
@@ -31,6 +31,7 @@ require_once($CFG->libdir . '/questionlib.php');
  * @category   test
  * @copyright  2009 The Open University
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @covers \question_state
  */
 class questionstate_test extends \advanced_testcase {
     public function test_is_active() {
@@ -154,4 +155,14 @@ class questionstate_test extends \advanced_testcase {
         $this->assertEquals(question_state::$mangrright,
                 question_state::$gradedpartial->corresponding_commented_state(1));
     }
+
+    public function test_get(): void {
+        $this->assertEquals(question_state::$todo, question_state::get('todo'));
+    }
+
+    public function test_get_bad_data(): void {
+        question_state::get('');
+        $this->assertDebuggingCalled('Attempt to create a state from an empty string. ' .
+                'This is probably a sign of bad data in your database. See MDL-80127.');
+    }
 }