diff --git a/lib/xapi/classes/state_store.php b/lib/xapi/classes/state_store.php index 95742520538..099b9629399 100644 --- a/lib/xapi/classes/state_store.php +++ b/lib/xapi/classes/state_store.php @@ -40,6 +40,20 @@ class state_store { $this->component = $component; } + /** + * Convert the xAPI activity ID into an item ID integer. + * + * @throws xapi_exception if the activity id is not numeric. + * @param string $activityid the provided activity ID + * @return int + */ + protected function activity_id_to_item_id(string $activityid): int { + if (!is_numeric($activityid)) { + throw new xapi_exception('The state store can only store numeric activity IDs.'); + } + return intval($activityid); + } + /** * Delete any extra state data stored in the database. * @@ -55,7 +69,7 @@ class state_store { $data = [ 'component' => $this->component, 'userid' => $state->get_user()->id, - 'itemid' => $state->get_activity_id(), + 'itemid' => $this->activity_id_to_item_id($state->get_activity_id()), 'stateid' => $state->get_state_id(), 'registration' => $state->get_registration(), ]; @@ -77,7 +91,7 @@ class state_store { $data = [ 'component' => $this->component, 'userid' => $state->get_user()->id, - 'itemid' => $state->get_activity_id(), + 'itemid' => $this->activity_id_to_item_id($state->get_activity_id()), 'stateid' => $state->get_state_id(), 'registration' => $state->get_registration(), ]; @@ -109,7 +123,7 @@ class state_store { $data = [ 'component' => $this->component, 'userid' => $state->get_user()->id, - 'itemid' => $state->get_activity_id(), + 'itemid' => $this->activity_id_to_item_id($state->get_activity_id()), 'stateid' => $state->get_state_id(), 'registration' => $state->get_registration(), ]; @@ -151,7 +165,7 @@ class state_store { 'component' => $this->component, ]; if ($itemid) { - $data['itemid'] = $itemid; + $data['itemid'] = $this->activity_id_to_item_id($itemid); } if ($userid) { $data['userid'] = $userid; @@ -188,7 +202,7 @@ class state_store { 'component' => $this->component, ]; if ($itemid) { - $data['itemid'] = $itemid; + $data['itemid'] = $this->activity_id_to_item_id($itemid); } if ($userid) { $data['userid'] = $userid; @@ -226,7 +240,7 @@ class state_store { ]; if ($itemid) { $select .= ' AND itemid = :itemid'; - $params['itemid'] = $itemid; + $params['itemid'] = $this->activity_id_to_item_id($itemid); } if ($userid) { $select .= ' AND userid = :userid'; diff --git a/lib/xapi/tests/state_store_test.php b/lib/xapi/tests/state_store_test.php index 3ef74c5ac1f..817e5af7de5 100644 --- a/lib/xapi/tests/state_store_test.php +++ b/lib/xapi/tests/state_store_test.php @@ -554,4 +554,61 @@ class state_store_test extends advanced_testcase { ], ]; } + + /** + * Test delete with a non numeric activity id. + * + * The default state store only allows integer itemids. + * + * @dataProvider invalid_activityid_format_provider + * @param string $operation the method to execute + * @param bool $usestate if the param is a state or the activity id + */ + public function test_invalid_activityid_format(string $operation, bool $usestate = false): void { + $this->resetAfterTest(); + $this->setAdminUser(); + + $state = test_helper::create_state([ + 'activity' => item_activity::create_from_id('notnumeric'), + ]); + $param = ($usestate) ? $state : 'notnumeric'; + + $this->expectException(xapi_exception::class); + $store = new state_store('fake_component'); + $store->$operation($param); + } + + /** + * Data provider for test_invalid_activityid_format. + * + * @return array + */ + public function invalid_activityid_format_provider(): array { + return [ + 'delete' => [ + 'operation' => 'delete', + 'usestate' => true, + ], + 'get' => [ + 'operation' => 'get', + 'usestate' => true, + ], + 'put' => [ + 'operation' => 'put', + 'usestate' => true, + ], + 'reset' => [ + 'operation' => 'reset', + 'usestate' => false, + ], + 'wipe' => [ + 'operation' => 'wipe', + 'usestate' => false, + ], + 'get_state_ids' => [ + 'operation' => 'get_state_ids', + 'usestate' => false, + ], + ]; + } } diff --git a/lib/xapi/upgrade.txt b/lib/xapi/upgrade.txt index 493dbf5ea7f..4cc8dbd879f 100644 --- a/lib/xapi/upgrade.txt +++ b/lib/xapi/upgrade.txt @@ -1,6 +1,12 @@ This files describes API changes in core_xapi libraries and APIs, information provided here is intended especially for developers. +=== 4.3 === +* The default state store will throw and exception if the activity ID is not + a numeric value. This is to avoid problems with the database when using + the default state store. If a plugin requires a non numeric activity ID, + it must implement its own state store. + === 4.2 === * A new state store has been introduced. Now plugins can store state data