From 60fe64d78655afda4380323972432a96b676d41a Mon Sep 17 00:00:00 2001
From: Jun Pataleta <jun@moodle.com>
Date: Wed, 25 Jan 2023 11:39:23 +0800
Subject: [PATCH] MDL-75085 core_external: Validate $required param

Make sure that the $required param for external_description and its
subclasses are either VALUE_DEFAULT, VALUE_REQUIRED, or VALUE_OPTIONAL.
---
 lib/external/classes/external_description.php | 10 ++-
 lib/external/tests/external_value_test.php    | 67 +++++++++++++++++++
 lib/upgrade.txt                               |  3 +
 3 files changed, 79 insertions(+), 1 deletion(-)
 create mode 100644 lib/external/tests/external_value_test.php

diff --git a/lib/external/classes/external_description.php b/lib/external/classes/external_description.php
index 83acd898472..bcbfcd17f13 100644
--- a/lib/external/classes/external_description.php
+++ b/lib/external/classes/external_description.php
@@ -37,10 +37,18 @@ abstract class external_description {
      * Contructor.
      *
      * @param string $desc Description of element
-     * @param int $required Whethe the element value is required
+     * @param int $required Whether the element value is required. Valid values are VALUE_DEFAULT, VALUE_REQUIRED, VALUE_OPTIONAL.
      * @param mixed $default The default value
      */
     public function __construct($desc, $required, $default) {
+        if (!in_array($required, [VALUE_DEFAULT, VALUE_REQUIRED, VALUE_OPTIONAL], true)) {
+            $requiredstr = $required;
+            if (is_array($required)) {
+                $requiredstr = "Array: " . implode(" ", $required);
+            }
+            debugging("Invalid \$required parameter value: '{$requiredstr}'. 
+                It must be either VALUE_DEFAULT, VALUE_REQUIRED, or VALUE_OPTIONAL", DEBUG_DEVELOPER);
+        }
         $this->desc = $desc;
         $this->required = $required;
         $this->default = $default;
diff --git a/lib/external/tests/external_value_test.php b/lib/external/tests/external_value_test.php
new file mode 100644
index 00000000000..77bd8d5d5e7
--- /dev/null
+++ b/lib/external/tests/external_value_test.php
@@ -0,0 +1,67 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+namespace core_external;
+
+use advanced_testcase;
+
+/**
+ * Unit tests for core_external\external_description.
+ *
+ * @package    core
+ * @category   test
+ * @copyright  2023 Jun Pataleta
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ * @coversDefaultClass external_value
+ */
+class external_value_test extends advanced_testcase {
+
+    /**
+     * Data provider for the required param test.
+     *
+     * @return array[]
+     */
+    public function required_param_provider(): array {
+        return [
+            [ VALUE_DEFAULT, false ],
+            [ VALUE_REQUIRED, false ],
+            [ VALUE_OPTIONAL, false ],
+            [ 'aaa', true, 'aaa' ],
+            [ [VALUE_OPTIONAL], true, 'Array: ' . VALUE_OPTIONAL ],
+            [ -1000, true, -1000 ],
+        ];
+    }
+
+    /**
+     * Tests the constructor for the $required parameter validation.
+     *
+     * @dataProvider required_param_provider
+     * @param int $required The required param being tested.
+     * @param bool $debuggingexpected Whether debugging is expected.
+     * @param mixed $requiredstr The string value of the $required param in the debugging message.
+     * @return void
+     */
+    public function test_required_param_validation($required, $debuggingexpected, $requiredstr = '') {
+        $externalvalue = new external_value(PARAM_INT, 'Cool description', $required);
+        if ($debuggingexpected) {
+            $this->assertDebuggingCalled("Invalid \$required parameter value: '{$requiredstr}'. 
+                It must be either VALUE_DEFAULT, VALUE_REQUIRED, or VALUE_OPTIONAL", DEBUG_DEVELOPER);
+        }
+        $this->assertEquals(PARAM_INT, $externalvalue->type);
+        $this->assertEquals('Cool description', $externalvalue->desc);
+        $this->assertEquals($required, $externalvalue->required);
+    }
+}
diff --git a/lib/upgrade.txt b/lib/upgrade.txt
index 968100ab05a..408fed96db7 100644
--- a/lib/upgrade.txt
+++ b/lib/upgrade.txt
@@ -56,6 +56,9 @@ information provided here is intended especially for developers.
 
   The old class locations have been aliased for backwards compatibility and will emit a deprecation notice in a future
   release.
+* The $required parameter for \core_external\external_description is now being validated in order to prevent
+  unintentionally passing incorrect parameters to the external_description's (and its subclasses') constructors (e.g. the parameter
+  description being incorrectly passed for the $required parameter). A debugging notice will be shown when such cases occur.
 
 === 4.1 ===