From 0f93331ed3ef327c6fda76bbf4885a40c36b7ca9 Mon Sep 17 00:00:00 2001 From: Joseph Snyder Date: Wed, 15 Jan 2025 14:11:01 -0500 Subject: [PATCH] Account for non-xml submission objects When given a non-xml submission file, catch the exception of the bad submission and only reject the submission of the Validate setting is enabled. Fixes: #2662 --- app/Http/Controllers/SubmissionController.php | 31 +++++++++++++------ app/Jobs/ProcessSubmission.php | 3 -- app/cdash/include/ctestparser.php | 17 +++++++--- phpstan-baseline.neon | 15 --------- 4 files changed, 34 insertions(+), 32 deletions(-) diff --git a/app/Http/Controllers/SubmissionController.php b/app/Http/Controllers/SubmissionController.php index a0b20214c..c78c99a0c 100644 --- a/app/Http/Controllers/SubmissionController.php +++ b/app/Http/Controllers/SubmissionController.php @@ -136,17 +136,28 @@ private function submitProcess(): Response // Figure out what type of XML file this is. $stored_filename = 'inbox/' . $filename; - $xml_info = SubmissionUtils::get_xml_type(fopen(Storage::path($stored_filename), 'r'), $stored_filename); - - // If validation is enabled and if this file has a corresponding schema, validate it - $validation_errors = $xml_info['xml_handler']::validate(storage_path('app/' . $stored_filename)); - if (count($validation_errors) > 0) { - $error_string = implode(PHP_EOL, $validation_errors); - - // We always log validation failures, but we only send messages back to the client if configured to do so - Log::warning("Submission validation failed for file '$filename':" . PHP_EOL); + $xml_info = []; + try { + $xml_info = SubmissionUtils::get_xml_type(fopen(Storage::path($stored_filename), 'r'), $stored_filename); + } catch (BadSubmissionException $e) { + $xml_info['xml_handler'] = ''; + $message = "Could not determine submission file type for: '{$stored_filename}'"; + Log::warning($message . PHP_EOL); if ((bool) config('cdash.validate_xml_submissions') === true) { - abort(400, "XML validation failed: rejected file $filename:" . PHP_EOL . $error_string); + abort(400, $message); + } + } + if ($xml_info['xml_handler'] !== '') { + // If validation is enabled and if this file has a corresponding schema, validate it + $validation_errors = $xml_info['xml_handler']::validate(storage_path('app/' . $stored_filename)); + if (count($validation_errors) > 0) { + $error_string = implode(PHP_EOL, $validation_errors); + + // We always log validation failures, but we only send messages back to the client if configured to do so + Log::warning("Submission validation failed for file '$filename':" . PHP_EOL); + if ((bool) config('cdash.validate_xml_submissions') === true) { + abort(400, "XML validation failed: rejected file $filename:" . PHP_EOL . $error_string); + } } } diff --git a/app/Jobs/ProcessSubmission.php b/app/Jobs/ProcessSubmission.php index bbd0ec77e..624ca78c1 100644 --- a/app/Jobs/ProcessSubmission.php +++ b/app/Jobs/ProcessSubmission.php @@ -4,7 +4,6 @@ use AbstractSubmissionHandler; use ActionableBuildInterface; -use App\Exceptions\BadSubmissionException; use App\Models\SuccessfulJob; use App\Utils\UnparsedSubmissionProcessor; use BuildPropertiesJSONHandler; @@ -134,7 +133,6 @@ private function requeueSubmissionFile($buildid): bool * * @return void * - * @throws BadSubmissionException */ public function handle() { @@ -199,7 +197,6 @@ public function failed(Throwable $exception): void * This method could be running on a worker that is either remote or local, so it accepts * a file handle or a filename that it can query the CDash API for. * - * @throws BadSubmissionException **/ private function doSubmit($filename, $projectid, $buildid = null, $expected_md5 = ''): AbstractSubmissionHandler|UnparsedSubmissionProcessor|false { diff --git a/app/cdash/include/ctestparser.php b/app/cdash/include/ctestparser.php index bb55ea450..a3c1b7617 100644 --- a/app/cdash/include/ctestparser.php +++ b/app/cdash/include/ctestparser.php @@ -159,8 +159,6 @@ function parse_put_submission($filehandler, $projectid, $expected_md5, ?int $bui /** * Main function to parse the incoming xml from ctest - * - * @throws BadSubmissionException */ function ctest_parse($filehandle, string $filename, $projectid, $expected_md5 = '', ?int $buildid = null): AbstractSubmissionHandler|false { @@ -185,9 +183,20 @@ function ctest_parse($filehandle, string $filename, $projectid, $expected_md5 = $Project = new Project(); $Project->Id = $projectid; - + $xml_info = []; // Figure out what type of XML file this is. - $xml_info = SubmissionUtils::get_xml_type($filehandle, $filename); + try { + $xml_info = SubmissionUtils::get_xml_type($filehandle, $filename); + } catch (BadSubmissionException $e) { + $xml_info['file_handle'] = $filehandle; + $xml_info['xml_handler'] = null; + $xml_info['xml_type'] = ''; + $message = "Could not determine submission file type for: '{$filename}'"; + Log::warning($message . PHP_EOL); + if ((bool) config('cdash.validate_xml_submissions') === true) { + abort(400, $message); + } + } $filehandle = $xml_info['file_handle']; $handler_ref = $xml_info['xml_handler']; $file = $xml_info['xml_type']; diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 5dcdbe376..7e44ca377 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -14002,21 +14002,6 @@ parameters: count: 1 path: app/cdash/include/ctestparser.php - - - message: "#^Parameter \\#1 \\$stream of function feof expects resource, mixed given\\.$#" - count: 1 - path: app/cdash/include/ctestparser.php - - - - message: "#^Parameter \\#1 \\$stream of function fread expects resource, mixed given\\.$#" - count: 2 - path: app/cdash/include/ctestparser.php - - - - message: "#^Parameter \\#1 \\$stream of function rewind expects resource, mixed given\\.$#" - count: 1 - path: app/cdash/include/ctestparser.php - - message: "#^Parameter \\#2 \\$data of function xml_parse expects string, string\\|false given\\.$#" count: 2