diff --git a/lib/Constants.php b/lib/Constants.php index 7b09bda65..3e601a2f1 100644 --- a/lib/Constants.php +++ b/lib/Constants.php @@ -234,6 +234,11 @@ class Constants { */ public const QUESTION_EXTRASETTINGS_OTHER_PREFIX = 'system-other-answer:'; + /** + * Maximum number of submissions returned per paginated API request. + */ + public const SUBMISSIONS_LIMIT_MAX = 1000; + public const SUPPORTED_EXPORT_FORMATS = [ 'csv' => 'text/csv', 'ods' => 'application/vnd.oasis.opendocument.spreadsheet', diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index 8a5797564..09bc3cb9c 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -1180,6 +1180,11 @@ public function getSubmissions(int $formId, ?string $query = null, ?int $limit = return new DataDownloadResponse($submissionsData, $fileName, Constants::SUPPORTED_EXPORT_FORMATS[$fileFormat]); } + // Cap user-supplied page size to avoid unbounded resource use + if ($limit !== null) { + $limit = max(1, min($limit, Constants::SUBMISSIONS_LIMIT_MAX)); + } + // Load submissions and currently active questions if ($canSeeAllSubmissions) { $submissions = $this->submissionService->getSubmissions($formId, null, $query, $limit, $offset); diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index ca63507ce..453e81adc 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -297,6 +297,108 @@ public function testGetSubmissions(array $submissions, array $questions, array $ $this->assertEquals(new DataResponse($expected), $this->apiController->getSubmissions(1)); } + public function testGetSubmissions_limitIsCapped(): void { + $form = new Form(); + $form->setId(1); + $form->setOwnerId('otherUser'); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(1, Constants::PERMISSION_RESULTS) + ->willReturn($form); + + $this->formsService->expects($this->once()) + ->method('getPermissions') + ->with($form) + ->willReturn([Constants::PERMISSION_RESULTS]); + + // An overly large limit must be clamped to SUBMISSIONS_LIMIT_MAX + $this->submissionService->expects($this->once()) + ->method('getSubmissions') + ->with(1, null, null, Constants::SUBMISSIONS_LIMIT_MAX, 0) + ->willReturn([]); + + $this->submissionMapper->expects($this->once()) + ->method('countSubmissions') + ->with(1) + ->willReturn(0); + + $this->formsService->expects($this->once()) + ->method('getQuestions') + ->with(1) + ->willReturn([]); + + $this->apiController->getSubmissions(1, limit: 100000); + } + + public function testGetSubmissions_limitFloorIsOne(): void { + $form = new Form(); + $form->setId(1); + $form->setOwnerId('otherUser'); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(1, Constants::PERMISSION_RESULTS) + ->willReturn($form); + + $this->formsService->expects($this->once()) + ->method('getPermissions') + ->with($form) + ->willReturn([Constants::PERMISSION_RESULTS]); + + // A non-positive limit must be floored to 1 + $this->submissionService->expects($this->once()) + ->method('getSubmissions') + ->with(1, null, null, 1, 0) + ->willReturn([]); + + $this->submissionMapper->expects($this->once()) + ->method('countSubmissions') + ->with(1) + ->willReturn(0); + + $this->formsService->expects($this->once()) + ->method('getQuestions') + ->with(1) + ->willReturn([]); + + $this->apiController->getSubmissions(1, limit: 0); + } + + public function testGetSubmissions_nullLimitStaysNull(): void { + $form = new Form(); + $form->setId(1); + $form->setOwnerId('otherUser'); + + $this->formsService->expects($this->once()) + ->method('getFormIfAllowed') + ->with(1, Constants::PERMISSION_RESULTS) + ->willReturn($form); + + $this->formsService->expects($this->once()) + ->method('getPermissions') + ->with($form) + ->willReturn([Constants::PERMISSION_RESULTS]); + + // null limit is preserved so the summary view can load all submissions + $this->submissionService->expects($this->once()) + ->method('getSubmissions') + ->with(1, null, null, null, 0) + ->willReturn([]); + + $this->submissionMapper->expects($this->once()) + ->method('countSubmissions') + ->with(1) + ->willReturn(0); + + $this->formsService->expects($this->once()) + ->method('getQuestions') + ->with(1) + ->willReturn([]); + + $this->apiController->getSubmissions(1); + } + public function testExportSubmissions_invalidForm() { $this->formsService->expects($this->once()) ->method('getFormIfAllowed')