From 1f20dec982120235ad83de16ad9a026b277a0542 Mon Sep 17 00:00:00 2001 From: Christian Hartmann Date: Wed, 10 Jun 2026 09:05:25 +0000 Subject: [PATCH] fix(QuestionGrid): fix validation for all subtypes and add tests Signed-off-by: Christian Hartmann --- lib/Service/SubmissionService.php | 105 ++++++++++++++++--- tests/Unit/Service/SubmissionServiceTest.php | 40 +++++++ 2 files changed, 132 insertions(+), 13 deletions(-) diff --git a/lib/Service/SubmissionService.php b/lib/Service/SubmissionService.php index 2b3ea7abc..b35605be3 100644 --- a/lib/Service/SubmissionService.php +++ b/lib/Service/SubmissionService.php @@ -551,7 +551,7 @@ public function validateSubmission(array $questions, array $answers, string $for // Check if all answers are within the possible options if (in_array($question['type'], Constants::ANSWER_TYPES_PREDEFINED) && empty($question['extraSettings']['allowOtherAnswer'])) { // Normalize option IDs once for consistent comparison (DB may return ints, request may send strings) - $optionIds = array_map('intval', array_column($question['options'] ?? [], 'id')); + $optionIds = $this->normalizeOptionIds($question['options'] ?? []); foreach ($answers[$questionId] as $answer) { // Handle linear scale questions @@ -562,21 +562,22 @@ public function validateSubmission(array $questions, array $answers, string $for throw new \InvalidArgumentException(sprintf('The answer for question "%s" must be an integer between %d and %d.', $question['text'], $optionsLowest, $optionsHighest)); } } + // Check if all grid rows, columns and values match the configured grid subtype + elseif ($question['type'] === Constants::ANSWER_TYPE_GRID) { + $this->validateGridQuestion($question, $answers[$questionId]); + } // Search corresponding option, return false if non-existent else { - $subAnswers = is_array($answer) ? $answer : [$answer]; - foreach ($subAnswers as $subAnswer) { - // Accept numeric strings like "46" from JSON payloads reliably (e.g. with hardening extensions enabled) - $answerId = is_int($subAnswer) ? $subAnswer : (is_string($subAnswer) ? intval(trim($subAnswer)) : null); - - // Reject non-numeric / malformed values early - if ($answerId === null || (string)$answerId !== (string)intval($answerId)) { - throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" is not a valid option.', is_scalar($subAnswer) ? (string)$subAnswer : gettype($subAnswer), $question['text'])); - } + // Accept numeric strings like "46" from JSON payloads reliably (e.g. with hardening extensions enabled) + $answerId = is_int($answer) ? $answer : (is_string($answer) ? intval(trim($answer)) : null); - if (!in_array($answerId, $optionIds, true)) { - throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" is not a valid option.', $subAnswer, $question['text'])); - } + // Reject non-numeric / malformed values early + if ($answerId === null || (string)$answerId !== (string)intval($answerId)) { + throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" is not a valid option.', is_scalar($answer) ? (string)$answer : gettype($answer), $question['text'])); + } + + if (!in_array($answerId, $optionIds, true)) { + throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" is not a valid option.', $answer, $question['text'])); } } } @@ -707,6 +708,84 @@ private function validateShortQuestion(array $question, string $data): bool { } } + private function validateGridQuestion(array $question, array $answers): void { + $rowIds = $this->normalizeOptionIds($question['options'] ?? [], Option::OPTION_TYPE_ROW); + $columnIds = $this->normalizeOptionIds($question['options'] ?? [], Option::OPTION_TYPE_COLUMN); + $gridType = $question['extraSettings']['questionType'] ?? null; + + foreach ($answers as $rowId => $rowAnswer) { + $normalizedRowId = $this->normalizeNumericOptionId($rowId); + if ($normalizedRowId === null || !in_array($normalizedRowId, $rowIds, true)) { + throw new \InvalidArgumentException(sprintf('Row "%s" for question "%s" is not a valid option.', $rowId, $question['text'])); + } + + if ($gridType === Constants::ANSWER_GRID_TYPE_RADIO) { + $selectedColumnIds = [$rowAnswer]; + } elseif ($gridType === Constants::ANSWER_GRID_TYPE_CHECKBOX) { + $selectedColumnIds = is_array($rowAnswer) ? $rowAnswer : []; + if (!is_array($rowAnswer)) { + throw new \InvalidArgumentException(sprintf('Invalid answer for question "%s".', $question['text'])); + } + } elseif ($gridType === Constants::ANSWER_GRID_TYPE_NUMBER) { + if (!is_array($rowAnswer)) { + throw new \InvalidArgumentException(sprintf('Invalid answer for question "%s".', $question['text'])); + } + + foreach ($rowAnswer as $columnId => $value) { + $normalizedColumnId = $this->normalizeNumericOptionId($columnId); + if ($normalizedColumnId === null || !in_array($normalizedColumnId, $columnIds, true)) { + throw new \InvalidArgumentException(sprintf('Column "%s" for question "%s" is not a valid option.', $columnId, $question['text'])); + } + if ($value !== '' && !is_numeric($value)) { + throw new \InvalidArgumentException(sprintf('Answer "%s" for question "%s" must be a number.', is_scalar($value) ? (string)$value : gettype($value), $question['text'])); + } + } + continue; + } else { + throw new \InvalidArgumentException(sprintf('Invalid grid type for question "%s".', $question['text'])); + } + + foreach ($selectedColumnIds as $columnId) { + if ((!is_int($columnId) && (!is_string($columnId) || !ctype_digit($columnId))) || !in_array((int)$columnId, $columnIds, true)) { + throw new \InvalidArgumentException(sprintf('Column "%s" for question "%s" is not a valid option.', is_scalar($columnId) ? (string)$columnId : gettype($columnId), $question['text'])); + } + } + } + } + + private function normalizeNumericOptionId(mixed $value): ?int { + if (is_int($value)) { + return $value; + } + + if (is_string($value) && ctype_digit($value)) { + return (int)$value; + } + + return null; + } + + /** + * @param array $options + * @return list + */ + private function normalizeOptionIds(array $options, ?string $optionType = null): array { + $normalizedIds = []; + + foreach ($options as $option) { + if ($optionType !== null && (($option['optionType'] ?? null) !== $optionType)) { + continue; + } + + $normalizedId = $this->normalizeNumericOptionId($option['id'] ?? null); + if ($normalizedId !== null) { + $normalizedIds[] = $normalizedId; + } + } + + return $normalizedIds; + } + private function setCellValue(Worksheet $activeWorksheet, int $column, int $row, mixed $value, string $fileFormat): void { if (!is_string($value)) { $activeWorksheet->setCellValue([$column, $row], $value); diff --git a/tests/Unit/Service/SubmissionServiceTest.php b/tests/Unit/Service/SubmissionServiceTest.php index f71f54448..fc6c89d6b 100644 --- a/tests/Unit/Service/SubmissionServiceTest.php +++ b/tests/Unit/Service/SubmissionServiceTest.php @@ -1237,6 +1237,46 @@ public static function dataValidateSubmission() { // Expected Result null, ], + 'valid-number-grid' => [ + [ + ['id' => 458, 'type' => 'grid', 'text' => 'Number grid', 'isRequired' => false, 'extraSettings' => ['questionType' => Constants::ANSWER_GRID_TYPE_NUMBER], 'options' => [ + ['id' => 2433, 'optionType' => 'column'], + ['id' => 2434, 'optionType' => 'column'], + ['id' => 2435, 'optionType' => 'column'], + ['id' => 2436, 'optionType' => 'row'], + ['id' => 2437, 'optionType' => 'row'], + ['id' => 2438, 'optionType' => 'row'], + ]], + ], + [ + '458' => [ + '2436' => ['2433' => '1', '2434' => '', '2435' => ''], + '2437' => ['2433' => '', '2434' => '1', '2435' => ''], + '2438' => ['2433' => '', '2434' => '', '2435' => '1'], + ], + ], + null, + ], + 'invalid-number-grid-value' => [ + [ + ['id' => 1, 'type' => 'grid', 'text' => 'Number grid', 'isRequired' => false, 'extraSettings' => ['questionType' => Constants::ANSWER_GRID_TYPE_NUMBER], 'options' => [ + ['id' => 10, 'optionType' => 'row'], + ['id' => 20, 'optionType' => 'column'], + ]], + ], + ['1' => ['10' => ['20' => 'not a number']]], + 'Answer "not a number" for question "Number grid" must be a number.', + ], + 'invalid-number-grid-column' => [ + [ + ['id' => 1, 'type' => 'grid', 'text' => 'Number grid', 'isRequired' => false, 'extraSettings' => ['questionType' => Constants::ANSWER_GRID_TYPE_NUMBER], 'options' => [ + ['id' => 10, 'optionType' => 'row'], + ['id' => 20, 'optionType' => 'column'], + ]], + ], + ['1' => ['10' => ['21' => '1']]], + 'Column "21" for question "Number grid" is not a valid option.', + ], 'invalid-ranking-missing-option' => [ // Questions [