From b0e0762ca0715b70915e5b0908ba9d98346fe14d Mon Sep 17 00:00:00 2001 From: Tim Hunt Date: Tue, 17 Jul 2012 17:32:56 +0100 Subject: [PATCH] MDL-34251 question engine: possible infinite loop loading usages In the case where either a question_attempt had not steps, or a question_usage had not question_attempts, the load_from_records methods could get stuck in an infinite loop. --- question/engine/questionattempt.php | 9 +++++++++ question/engine/questionusage.php | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/question/engine/questionattempt.php b/question/engine/questionattempt.php index 6ead6884609..a6832841225 100644 --- a/question/engine/questionattempt.php +++ b/question/engine/questionattempt.php @@ -1213,6 +1213,15 @@ class question_attempt { $qa->behaviour = question_engine::make_behaviour( $record->behaviour, $qa, $preferredbehaviour); + // If attemptstepid is null (which should not happen, but has happened + // due to corrupt data, see MDL-34251) then the current pointer in $records + // will not be advanced in the while loop below, and we get stuck in an + // infinite loop, since this method is supposed to always consume at + // least one record. Therefore, in this case, advance the record here. + if (is_null($record->attemptstepid)) { + $records->next(); + } + $i = 0; while ($record && $record->questionattemptid == $questionattemptid && !is_null($record->attemptstepid)) { $qa->steps[$i] = question_attempt_step::load_from_records($records, $record->attemptstepid); diff --git a/question/engine/questionusage.php b/question/engine/questionusage.php index 46ea5cf3912..8ca56e85aae 100644 --- a/question/engine/questionusage.php +++ b/question/engine/questionusage.php @@ -706,6 +706,14 @@ class question_usage_by_activity { $quba->observer = new question_engine_unit_of_work($quba); + // If slot is null then the current pointer in $records will not be + // advanced in the while loop below, and we get stuck in an infinite loop, + // since this method is supposed to always consume at least one record. + // Therefore, in this case, advance the record here. + if (is_null($record->slot)) { + $records->next(); + } + while ($record && $record->qubaid == $qubaid && !is_null($record->slot)) { $quba->questionattempts[$record->slot] = question_attempt::load_from_records($records,