Skip to content

Commit

Permalink
Switch to submission time validation of XML files
Browse files Browse the repository at this point in the history 
Add a test which utilizes the submission function of KWTester to
submit the same data as the manual validation steps.
  • Loading branch information
@josephsnyder
josephsnyder committed Nov 19, 2024
1 parent 7657cc7 commit f02158b
Show file tree
Hide file tree
Showing 6 changed files with 146 additions and 88 deletions.
31 changes: 31 additions & 0 deletions app/Http/Controllers/SubmissionController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
use App\Models\Site;
use App\Utils\AuthTokenUtil;
use App\Utils\UnparsedSubmissionProcessor;
use App\Utils\SubmissionUtils;
use App\Exceptions\CDashXMLValidationException;
use CDash\Model\Build;
use CDash\Model\PendingSubmissions;
use CDash\Model\Project;
Expand Down Expand Up @@ -128,6 +130,35 @@ private function submitProcess(): Response
} elseif (intval($this->project->Id) < 1) {
abort(Response::HTTP_NOT_FOUND, 'The requested project does not exist.');
}
if ((bool) config('cdash.validate_xml_submissions') === true ) {
// Figure out what type of XML file this is.
try {
$filename = "inbox/".$filename;
$xml_info = SubmissionUtils::get_xml_type(fopen(Storage::path($filename), 'r'), $filename);
} catch (CDashXMLValidationException $e) {
foreach ($e->getDecodedMessage() as $error) {
Log::error($error);
}
abort(Response::HTTP_NOT_FOUND, 'Unable to determine the Type of the submission file.');
}
$filehandle = $xml_info['file_handle'];
$handler_ref = $xml_info['xml_handler'];
$file = $xml_info['xml_type'];
$schema_file = $xml_info['xml_schema'];

// If validation is enabled and if this file has a corresponding schema, validate it
if (isset($schema_file)) {
try {
SubmissionUtils::validate_xml(storage_path("app/".$filename), $schema_file);
} catch (CDashXMLValidationException $e) {
foreach ($e->getDecodedMessage() as $error) {
Log::error("Validating $filename: ".$error);
}
abort(400, "Xml validation failed: rejected file $filename");
}
}
}


// Check if CTest provided us enough info to assign a buildid.
$pendingSubmissions = new PendingSubmissions();
Expand Down
2 changes: 1 addition & 1 deletion app/Jobs/ProcessSubmission.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ private function doSubmit($filename, $projectid, $buildid = null,
}

// Parse the XML file
$handler = ctest_parse($filehandle, $filename, $projectid, $expected_md5, $buildid);
$handler = ctest_parse($filehandle, $filename, $projectid, $expected_md5, $buildid);
fclose($filehandle);
unset($filehandle);

Expand Down
26 changes: 2 additions & 24 deletions app/cdash/include/ctestparser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,10 @@

use CDash\Database;
use App\Models\BuildFile;
use App\Utils\SubmissionUtils;
use CDash\Model\Project;
use Illuminate\Support\Facades\Log;
use Illuminate\Support\Facades\Storage;
use App\Utils\SubmissionUtils;
use App\Exceptions\CDashXMLValidationException;

class CDashParseException extends RuntimeException
{
Expand Down Expand Up @@ -173,31 +172,10 @@ function ctest_parse($filehandle, string $filename, $projectid, $expected_md5 =
}

// Figure out what type of XML file this is.
try {
$xml_info = SubmissionUtils::get_xml_type($filehandle, $filename);
} catch (CDashXMLValidationException $e) {
foreach ($e->getDecodedMessage() as $error) {
Log::error($error);
}
return false;
}
$xml_info = SubmissionUtils::get_xml_type($filehandle, $filename);
$filehandle = $xml_info['file_handle'];
$handler_ref = $xml_info['xml_handler'];
$file = $xml_info['xml_type'];
$schema_file = $xml_info['xml_schema'];

// If validation is enabled and if this file has a corresponding schema, validate it
if (((bool) config('cdash.validate_xml_submissions')) === true && isset($schema_file)) {
try {
SubmissionUtils::validate_xml(storage_path("app/".$filename), $schema_file);
} catch (CDashXMLValidationException $e) {
foreach ($e->getDecodedMessage() as $error) {
Log::error("Validating $filename: ".$error);
}
abort(400, "Xml validation failed: rejected file $filename");
}
}

$handler = isset($handler_ref) ? new $handler_ref($projectid) : null;

rewind($filehandle);
Expand Down
3 changes: 3 additions & 0 deletions app/cdash/tests/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -613,5 +613,8 @@ set_tests_properties(/Feature/RemoteWorkers PROPERTIES DEPENDS install_3)

add_laravel_test(/Unit/app/Console/Command/ValidateXmlCommandTest)

add_php_test(submissionvalidation)
set_tests_properties(submissionvalidation PROPERTIES DEPENDS createpublicdashboard)

add_subdirectory(ctest)
add_subdirectory(autoremovebuilds)
39 changes: 39 additions & 0 deletions app/cdash/tests/test_submissionvalidation.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php
require_once dirname(__FILE__) . '/cdash_test_case.php';



class SubmissionValidationTestCase extends KWWebTestCase
{
protected $PDO;
protected $ConfigFile;
protected $Original;

public function _construct()
{

parent::__construct();
}

public function submit($fileName)
{
$rep = dirname(__FILE__) . '/../../../tests/data/XmlValidation';
$file = "$rep/$fileName";

return $this->submission('PublicDashboard', $file);
}

public function testSubmissionValidation()
{

$this->ConfigFile = dirname(__FILE__) . '/../../../.env';
$this->Original = file_get_contents($this->ConfigFile);
file_put_contents($this->ConfigFile, "VALIDATE_XML_SUBMISSIONS=true\n", FILE_APPEND | LOCK_EX);

$this->assertFalse($this->submit("invalid_Configure.xml"), "Submission of invalid_syntax_Build.xml was not succeessful");
$this->assertFalse($this->submit("invalid_syntax_Build.xml"), "Submission of invalid_syntax_Build.xml was not succeessful");
$this->assertTrue($this->submit("valid_Build.xml"), "Submission of valid_Build.xml has succeeded");

file_put_contents($this->ConfigFile, $this->Original);
}
}
133 changes: 70 additions & 63 deletions phpstan-baseline.neon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ parameters:
path: app/Console/Commands/UpdateDependencies.php

-
message: "#^Part \\$xml_type \\(mixed\\) of encapsed string cannot be cast to string\\.$#"
message: "#^Parameter \\#2 \\$schema_file of static method App\\\\Utils\\\\SubmissionUtils\\:\\:validate_xml\\(\\) expects string, mixed given\\.$#"
count: 1
path: app/Console/Commands/ValidateXml.php

Expand All @@ -186,6 +186,11 @@ parameters:
count: 1
path: app/Console/Kernel.php

-
message: "#^Method App\\\\Exceptions\\\\CDashXMLValidationException\\:\\:getDecodedMessage\\(\\) should return array\\<int, string\\> but returns mixed\\.$#"
count: 1
path: app/Exceptions/CDashXMLValidationException.php

-
message: "#^PHPDoc type array of property App\\\\Exceptions\\\\Handler\\:\\:\\$dontFlash is not the same as PHPDoc type array\\<int, string\\> of overridden property Illuminate\\\\Foundation\\\\Exceptions\\\\Handler\\:\\:\\$dontFlash\\.$#"
count: 1
Expand Down Expand Up @@ -2869,6 +2874,11 @@ parameters:
count: 1
path: app/Http/Controllers/SubmissionController.php

-
message: "#^Method App\\\\Http\\\\Controllers\\\\SubmissionController\\:\\:submitProcess\\(\\) should return Illuminate\\\\Http\\\\Response but returns false\\.$#"
count: 1
path: app/Http/Controllers/SubmissionController.php

-
message: "#^Only booleans are allowed in a negated boolean, int\\|false given\\.$#"
count: 1
Expand All @@ -2894,6 +2904,11 @@ parameters:
count: 1
path: app/Http/Controllers/SubmissionController.php

-
message: "#^Parameter \\#2 \\$schema_file of static method App\\\\Utils\\\\SubmissionUtils\\:\\:validate_xml\\(\\) expects string, mixed given\\.$#"
count: 1
path: app/Http/Controllers/SubmissionController.php

-
message: "#^Part \\$projectname \\(mixed\\) of encapsed string cannot be cast to string\\.$#"
count: 4
Expand Down Expand Up @@ -14503,11 +14518,6 @@ parameters:
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Access to an undefined property object\\:\\:\\$backupFileName\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Binary operation \"\\.\" between 'An XML submission…' and mixed results in an error\\.$#"
count: 1
Expand All @@ -14518,50 +14528,17 @@ parameters:
count: 2
path: app/cdash/include/ctestparser.php

-
message: "#^Binary operation \"\\.\" between mixed and '\\.xml' results in an error\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Binary operation \"\\.\" between non\\-falsy\\-string and \\(list\\<string\\>\\|string\\|null\\) results in an error\\.$#"
count: 2
path: app/cdash/include/ctestparser.php

-
message: "#^Call to an undefined method object\\:\\:getBuildName\\(\\)\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Call to an undefined method object\\:\\:getBuildStamp\\(\\)\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Call to an undefined method object\\:\\:getSiteName\\(\\)\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Call to an undefined method object\\:\\:getSubProjectName\\(\\)\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: """
#^Call to deprecated function add_log\\(\\)\\:
04/04/2023 Use \\\\Illuminate\\\\Support\\\\Facades\\\\Log for logging instead$#
"""
count: 3
path: app/cdash/include/ctestparser.php

-
message: """
#^Call to deprecated method executePrepared\\(\\) of class CDash\\\\Database\\:
04/22/2023 Use Laravel query builder or Eloquent instead$#
"""
count: 1
count: 2
path: app/cdash/include/ctestparser.php

-
Expand All @@ -14574,7 +14551,7 @@ parameters:

-
message: "#^Construct empty\\(\\) is not allowed\\. Use more strict comparison\\.$#"
count: 4
count: 3
path: app/cdash/include/ctestparser.php

-
Expand Down Expand Up @@ -14658,52 +14635,37 @@ parameters:
path: app/cdash/include/ctestparser.php

-
message: "#^Loose comparison via \"\\!\\=\" is not allowed\\.$#"
count: 2
path: app/cdash/include/ctestparser.php

-
message: "#^Loose comparison via \"\\=\\=\" is not allowed\\.$#"
message: "#^Loose comparison using \\=\\= between null and null will always evaluate to true\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Offset 'uri' might not exist on array\\{timed_out\\: bool, blocked\\: bool, eof\\: bool, unread_bytes\\: int, stream_type\\: string, wrapper_type\\: string, wrapper_data\\: mixed, mode\\: string, \\.\\.\\.\\}\\.$#"
message: "#^Loose comparison via \"\\!\\=\" is not allowed\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Parameter \\#1 \\$stream of function feof expects resource, mixed given\\.$#"
message: "#^Loose comparison via \"\\=\\=\" is not allowed\\.$#"
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\\.$#"
message: "#^Offset 'uri' might not exist on array\\{timed_out\\: bool, blocked\\: bool, eof\\: bool, unread_bytes\\: int, stream_type\\: string, wrapper_type\\: string, wrapper_data\\: mixed, mode\\: string, \\.\\.\\.\\}\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Parameter \\#2 \\$data of function xml_parse expects string, string\\|false given\\.$#"
count: 2
path: app/cdash/include/ctestparser.php

-
message: "#^Parameter \\#2 \\$handler of function xml_set_character_data_handler expects callable\\(\\)\\: mixed, array\\{object, 'text'\\} given\\.$#"
message: "#^Undefined variable\\: \\$handler_ref$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Parameter \\#2 \\$start_handler of function xml_set_element_handler expects callable\\(\\)\\: mixed, array\\{object, 'startElement'\\} given\\.$#"
message: "#^Unreachable statement \\- code above always terminates\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

-
message: "#^Parameter \\#3 \\$end_handler of function xml_set_element_handler expects callable\\(\\)\\: mixed, array\\{object, 'endElement'\\} given\\.$#"
message: "#^Variable \\$handler_ref in isset\\(\\) is never defined\\.$#"
count: 1
path: app/cdash/include/ctestparser.php

Expand Down Expand Up @@ -27165,6 +27127,46 @@ parameters:
count: 1
path: app/cdash/tests/test_submission_assign_buildid.php

-
message: "#^Method SubmissionValidationTestCase\\:\\:_construct\\(\\) has no return type specified\\.$#"
count: 1
path: app/cdash/tests/test_submissionvalidation.php

-
message: "#^Method SubmissionValidationTestCase\\:\\:submit\\(\\) has no return type specified\\.$#"
count: 1
path: app/cdash/tests/test_submissionvalidation.php

-
message: "#^Method SubmissionValidationTestCase\\:\\:submit\\(\\) has parameter \\$fileName with no type specified\\.$#"
count: 1
path: app/cdash/tests/test_submissionvalidation.php

-
message: "#^Method SubmissionValidationTestCase\\:\\:testSubmissionValidation\\(\\) has no return type specified\\.$#"
count: 1
path: app/cdash/tests/test_submissionvalidation.php

-
message: "#^Property SubmissionValidationTestCase\\:\\:\\$ConfigFile has no type specified\\.$#"
count: 1
path: app/cdash/tests/test_submissionvalidation.php

-
message: "#^Property SubmissionValidationTestCase\\:\\:\\$Original has no type specified\\.$#"
count: 1
path: app/cdash/tests/test_submissionvalidation.php

-
message: "#^Property SubmissionValidationTestCase\\:\\:\\$PDO has no type specified\\.$#"
count: 1
path: app/cdash/tests/test_submissionvalidation.php

-
message: "#^Static call to __construct\\(\\) is only allowed on a parent class in the constructor\\.$#"
count: 1
path: app/cdash/tests/test_submissionvalidation.php

-
message: "#^Method SubmitSortingDataTestCase\\:\\:submitFile\\(\\) has no return type specified\\.$#"
count: 1
Expand Down Expand Up @@ -33014,6 +33016,11 @@ parameters:
count: 1
path: tests/Feature/UserCommand.php

-
message: "#^Part \\$file \\(mixed\\) of encapsed string cannot be cast to string\\.$#"
count: 1
path: tests/Unit/app/Console/Command/ValidateXmlCommandTest.php

-
message: "#^Parameter \\#1 \\$objectOrClass of class ReflectionClass constructor expects class\\-string\\<T of object\\>\\|T of object, array\\<int, string\\>\\|string given\\.$#"
count: 1
Expand Down

0 comments on commit f02158b

Please sign in to comment.