From b048871368b0d33681f67eb0fde008c0e9507fea Mon Sep 17 00:00:00 2001 From: William Allen Date: Mon, 17 Aug 2026 09:18:58 -0400 Subject: [PATCH] Use Laravel `request()` helper instead of superglobals This PR refactors away all superglobal (i.e., `$_GET` and `$_POST`) usage in the submission handling process in favor of Laravel's request handling methods. Doing so will allow us to use Laravel's request testing methods instead of creating full HTTP requests to test submissions in a future PR. --- app/Http/Controllers/SubmissionController.php | 21 +++--- app/Utils/UnparsedSubmissionProcessor.php | 68 ++++++++----------- phpstan-baseline.neon | 32 +++++---- 3 files changed, 60 insertions(+), 61 deletions(-) diff --git a/app/Http/Controllers/SubmissionController.php b/app/Http/Controllers/SubmissionController.php index 1e11c5dabd..2ffc192b80 100644 --- a/app/Http/Controllers/SubmissionController.php +++ b/app/Http/Controllers/SubmissionController.php @@ -34,9 +34,9 @@ public function __invoke(): Response|JsonResponse { // If we have a POST or PUT we defer to the unparsed submission processor. try { - if (isset($_POST['project'])) { + if (request()->isMethod('POST') && request()->has('project')) { return (new UnparsedSubmissionProcessor())->postSubmit(); - } elseif (isset($_GET['buildid'])) { + } elseif (request()->isMethod('PUT') && request()->has('buildid')) { return (new UnparsedSubmissionProcessor())->putSubmitFile(); } } catch (Exception $e) { @@ -99,8 +99,7 @@ private function submitProcess(): Response $this->failProcessing(null, Response::HTTP_BAD_REQUEST, "Invalid project name: $projectname"); } - $expected_md5 = isset($_GET['MD5']) ? htmlspecialchars($_GET['MD5']) : ''; - + $expected_md5 = request()->query('MD5', ''); if ($expected_md5 !== '' && !preg_match('/^[a-f0-9]{32}$/i', $expected_md5)) { Log::info("Rejected submission with invalid hash '$expected_md5' for project $projectname"); $this->failProcessing(null, Response::HTTP_BAD_REQUEST, "Provided md5 hash '{$expected_md5}' is improperly formatted."); @@ -199,19 +198,21 @@ private function submitProcess(): Response // Check if CTest provided us enough info to assign a buildid. $buildid = null; - if (isset($_GET['build']) && isset($_GET['site']) && isset($_GET['stamp'])) { + if (request()->has('build') && request()->has('site') && request()->has('stamp')) { $build = new Build(); - $build->Name = pdo_real_escape_string($_GET['build']); + $build->Name = pdo_real_escape_string(request()->query('build')); $build->ProjectId = $this->project->Id; - $build->SetStamp(pdo_real_escape_string($_GET['stamp'])); + $build->SetStamp(pdo_real_escape_string(request()->query('stamp'))); $build->StartTime = gmdate(FMT_DATETIME); $build->SubmitTime = $build->StartTime; - if (isset($_GET['subproject'])) { - $build->SetSubProject(pdo_real_escape_string($_GET['subproject'])); + if (request()->has('subproject')) { + $build->SetSubProject(pdo_real_escape_string(request()->query('subproject'))); } - $build->SiteId = Site::firstOrCreate(['name' => $_GET['site']], ['name' => $_GET['site']])->id; + $build->SiteId = Site::firstOrCreate([ + 'name' => request()->query('site'), + ])->id; if ($build->AddBuild()) { // Insert row to keep track of how many submissions are waiting to be diff --git a/app/Utils/UnparsedSubmissionProcessor.php b/app/Utils/UnparsedSubmissionProcessor.php index c3f9c3c221..7d5a07966e 100644 --- a/app/Utils/UnparsedSubmissionProcessor.php +++ b/app/Utils/UnparsedSubmissionProcessor.php @@ -112,40 +112,29 @@ public function postSubmit(): JsonResponse /** Parse build metadata from POST request. */ public function parseBuildMetadata(): void { - // We require POST to contain the following values. - $vars = ['project', 'build', 'stamp', 'site', 'starttime', 'endtime', 'datafilesmd5']; - foreach ($vars as $var) { - if (empty($_POST[$var])) { - abort(Response::HTTP_BAD_REQUEST, 'Variable \'' . $var . '\' not set but required.'); - } - } - - $this->projectname = htmlspecialchars($_POST['project']); - $this->buildname = htmlspecialchars($_POST['build']); - $this->buildstamp = htmlspecialchars($_POST['stamp']); - $this->sitename = htmlspecialchars($_POST['site']); - $this->starttime = htmlspecialchars($_POST['starttime']); - $this->endtime = htmlspecialchars($_POST['endtime']); - $this->generator = ''; - if (isset($_POST['generator'])) { - $this->generator = htmlspecialchars($_POST['generator']); - } - - $this->subprojectname = ''; - if (isset($_POST['subproject'])) { - $this->subprojectname = htmlspecialchars($_POST['subproject']); - } - - $validator = Validator::make([ - 'name' => $this->projectname, - ], [ - 'name' => new ProjectNameRule(), + $validator = Validator::make(request()->post(), [ + 'project' => ['required', new ProjectNameRule()], + 'build' => 'required', + 'stamp' => 'required', + 'site' => 'required', + 'starttime' => 'required', + 'endtime' => 'required', + 'datafilesmd5' => 'required', ]); if ($validator->fails()) { abort(Response::HTTP_BAD_REQUEST, $validator->errors()->first()); } + $this->projectname = request()->post('project'); + $this->buildname = request()->post('build'); + $this->buildstamp = request()->post('stamp'); + $this->sitename = request()->post('site'); + $this->starttime = request()->post('starttime'); + $this->endtime = request()->post('endtime'); + $this->generator = request()->post('generator', ''); + $this->subprojectname = request()->post('subproject', ''); + $this->getAuthTokenHash(); } @@ -362,18 +351,21 @@ public function populateBuildFileRow(): void public function parseDataFileParameters(): void { - // We expect GET to contain the following values: - $vars = ['buildid', 'type', 'md5', 'filename']; - foreach ($vars as $var) { - if (empty($_GET[$var])) { - abort(Response::HTTP_BAD_REQUEST, "Variable '$var' not set but required."); - } + $validator = Validator::make(request()->query(), [ + 'buildid' => 'required', + 'type' => 'required', + 'md5' => 'required', + 'filename' => 'required', + ]); + + if ($validator->fails()) { + abort(Response::HTTP_BAD_REQUEST, $validator->errors()->first()); } - $this->buildid = $_GET['buildid']; - $this->type = htmlspecialchars($_GET['type']); - $this->md5 = htmlspecialchars($_GET['md5']); - $this->backupfilename = htmlspecialchars($_GET['filename']); + $this->buildid = request()->query('buildid'); + $this->type = request()->query('type'); + $this->md5 = request()->query('md5'); + $this->backupfilename = request()->query('filename'); $this->getAuthTokenHash(); } diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 6c58b77e1d..b1d0789b56 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -1441,11 +1441,29 @@ parameters: path: app/Http/Controllers/SubmissionController.php - - rawMessage: 'Parameter #1 $string of function htmlspecialchars expects string, mixed given.' + rawMessage: 'Parameter #1 $string of function strlen expects string, (array|string) given.' identifier: argument.type count: 1 path: app/Http/Controllers/SubmissionController.php + - + rawMessage: 'Parameter #2 $subject of function preg_match expects string, (array|string) given.' + identifier: argument.type + count: 1 + path: app/Http/Controllers/SubmissionController.php + + - + rawMessage: 'Part $expected_md5 (array|non-empty-string) of encapsed string cannot be cast to string.' + identifier: encapsedStringPart.nonString + count: 2 + path: app/Http/Controllers/SubmissionController.php + + - + rawMessage: 'Part $expected_md5 (array|string) of encapsed string cannot be cast to string.' + identifier: encapsedStringPart.nonString + count: 3 + path: app/Http/Controllers/SubmissionController.php + - rawMessage: ''' Call to deprecated method executePrepared() of class CDash\Database: @@ -6108,12 +6126,6 @@ parameters: count: 2 path: app/Utils/UnparsedSubmissionProcessor.php - - - rawMessage: 'Construct empty() is not allowed. Use more strict comparison.' - identifier: empty.notAllowed - count: 2 - path: app/Utils/UnparsedSubmissionProcessor.php - - rawMessage: 'Loose comparison via "!=" between string and mixed is not allowed.' identifier: notEqual.notAllowed @@ -6162,12 +6174,6 @@ parameters: count: 1 path: app/Utils/UnparsedSubmissionProcessor.php - - - rawMessage: 'Parameter #1 $string of function htmlspecialchars expects string, mixed given.' - identifier: argument.type - count: 11 - path: app/Utils/UnparsedSubmissionProcessor.php - - rawMessage: Property App\Utils\UnparsedSubmissionProcessor::$backupfilename has no type specified. identifier: missingType.property