Eliminate pdo_real_escape_numeric() function (#1728)

Integers are always safe to insert in SQL queries. Thus, we can eliminate the `pdo_real_escape_numeric()` function and replace all of the existing usages with integer casts. In the vast majority of cases, this function was no longer being used to sanitize values for SQL queries anyway.
Kitware · Oct 2, 2023 · cf1b6c8 · cf1b6c8
1 parent 9d0c733
commit cf1b6c8
Show file tree

Hide file tree

Showing 13 changed files with 99 additions and 273 deletions.
diff --git a/app/Http/Controllers/AdminController.php b/app/Http/Controllers/AdminController.php
@@ -493,11 +493,8 @@ public function upgrade()
 
         // Compute the testtime
         if ($ComputeTestTiming) {
-            @$TestTimingDays = $_POST['TestTimingDays'];
-            if ($TestTimingDays != null) {
-                $TestTimingDays = pdo_real_escape_numeric($TestTimingDays);
-            }
-            if (is_numeric($TestTimingDays) && $TestTimingDays > 0) {
+            $TestTimingDays = (int) ($_POST['TestTimingDays'] ?? 0);
+            if ($TestTimingDays > 0) {
                 ComputeTestTiming($TestTimingDays);
                 $xml .= add_XML_value('alert', 'Timing for tests has been computed successfully.');
             } else {
@@ -507,11 +504,8 @@ public function upgrade()
 
         // Compute the user statistics
         if ($ComputeUpdateStatistics) {
-            @$UpdateStatisticsDays = $_POST['UpdateStatisticsDays'];
-            if ($UpdateStatisticsDays != null) {
-                $UpdateStatisticsDays = pdo_real_escape_numeric($UpdateStatisticsDays);
-            }
-            if (is_numeric($UpdateStatisticsDays) && $UpdateStatisticsDays > 0) {
+            $UpdateStatisticsDays = (int) ($_POST['UpdateStatisticsDays'] ?? 0);
+            if ($UpdateStatisticsDays > 0) {
                 ComputeUpdateStatistics($UpdateStatisticsDays);
                 $xml .= add_XML_value('alert', 'User statistics has been computed successfully.');
             } else {

diff --git a/app/Http/Controllers/ManageProjectRolesController.php b/app/Http/Controllers/ManageProjectRolesController.php
@@ -61,19 +61,19 @@ public function viewPage(): View|RedirectResponse
         @$adduser = $_POST['adduser'];
         @$removeuser = $_POST['removeuser'];
 
-        @$userid = $_POST['userid'];
-        if ($userid != null) {
-            $userid = pdo_real_escape_numeric($userid);
+        $userid = $_POST['userid'] ?? null;
+        if ($userid !== null) {
+            $userid = (int) $userid;
         }
 
-        @$role = $_POST['role'];
-        if ($role != null) {
-            $role = pdo_real_escape_numeric($role);
+        $role = $_POST['role'] ?? null;
+        if ($role !== null) {
+            $role = (int) $role;
         }
 
-        @$emailtype = $_POST['emailtype'];
-        if ($emailtype != null) {
-            $emailtype = pdo_real_escape_numeric($emailtype);
+        $emailtype = $_POST['emailtype'] ?? null;
+        if ($emailtype !== null) {
+            $emailtype = (int) $emailtype;
         }
 
         @$credentials = $_POST['credentials'];

diff --git a/app/Http/Controllers/SiteController.php b/app/Http/Controllers/SiteController.php
@@ -213,12 +213,12 @@ public function editSite(): View|RedirectResponse
         }
 
         // If we have a projectid that means we should list all the sites
-        @$projectid = $_GET['projectid'];
-        if ($projectid != null) {
-            $projectid = pdo_real_escape_numeric($projectid);
+        $projectid = $_GET['projectid'] ?? null;
+        if ($projectid !== null) {
+            $projectid = (int) $projectid;
         }
-        if (isset($projectid) && is_numeric($projectid)) {
-            $project_array = $db->executePreparedSingleRow('SELECT name FROM project WHERE id=?', [intval($projectid)]);
+        if ($projectid !== null) {
+            $project_array = $db->executePreparedSingleRow('SELECT name FROM project WHERE id=?', [$projectid]);
             $xml .= '<project>';
             $xml .= add_XML_value('id', $projectid);
             $xml .= add_XML_value('name', $project_array['name']);
@@ -234,7 +234,7 @@ public function editSite(): View|RedirectResponse
                                     AND build.starttime>?
                                     AND site.id=build.siteid
                                 ORDER BY site.name ASC
-                            ', [intval($projectid), $beginUTCTime]);
+                            ', [$projectid, $beginUTCTime]);
 
             foreach ($site2project as $site2project_array) {
                 $siteid = intval($site2project_array['id']);
@@ -256,14 +256,14 @@ public function editSite(): View|RedirectResponse
         }
 
         // If we have a siteid we look if the user has claimed the site or not
-        @$siteid = $_GET['siteid'];
-        if ($siteid != null) {
-            $siteid = pdo_real_escape_numeric($siteid);
+        $siteid = $_GET['siteid'] ?? null;
+        if ($siteid !== null) {
+            $siteid = (int) $siteid;
         }
-        if (isset($siteid) && is_numeric($siteid)) {
+        if ($siteid !== null) {
             $xml .= '<user>';
             $xml .= '<site>';
-            $site_array = $db->executePreparedSingleRow('SELECT * FROM site WHERE id=?', [intval($siteid)]);
+            $site_array = $db->executePreparedSingleRow('SELECT * FROM site WHERE id=?', [$siteid]);
 
             $siteinformation_array = [];
             $siteinformation_array['description'] = 'NA';
@@ -287,7 +287,7 @@ public function editSite(): View|RedirectResponse
                          WHERE siteid=?
                          ORDER BY timestamp DESC
                          LIMIT 1
-                     ', [intval($siteid)]);
+                     ', [$siteid]);
             if (!empty($query)) {
                 $siteinformation_array = $query;
                 if ($siteinformation_array['processoris64bits'] == -1) {
@@ -353,7 +353,7 @@ public function editSite(): View|RedirectResponse
                                  AND up.role>0
                                  AND su.siteid=?
                                  AND su.userid=?
-                         ', [intval($siteid), intval($userid)]);
+                         ', [$siteid, $userid]);
             echo pdo_error();
             if (!empty($user2site)) {
                 $xml .= add_XML_value('siteclaimed', '0');
@@ -378,25 +378,26 @@ public function viewSite(int $siteid): View
         $site_array = $db->executePreparedSingleRow("SELECT * FROM site WHERE id=?", [$siteid]);
         $sitename = $site_array['name'];
 
-        @$currenttime = $_GET['currenttime'];
-        if ($currenttime != null) {
-            $currenttime = pdo_real_escape_numeric($currenttime);
+        $currenttime = $_GET['currenttime'] ?? null;
+        if ($currenttime !== null) {
+            $currenttime = (int) $currenttime;
         }
 
-        $siteinformation_array = [];
-        $siteinformation_array['description'] = 'NA';
-        $siteinformation_array['processoris64bits'] = 'NA';
-        $siteinformation_array['processorvendor'] = 'NA';
-        $siteinformation_array['processorvendorid'] = 'NA';
-        $siteinformation_array['processorfamilyid'] = 'NA';
-        $siteinformation_array['processormodelid'] = 'NA';
-        $siteinformation_array['processorcachesize'] = 'NA';
-        $siteinformation_array['numberlogicalcpus'] = 'NA';
-        $siteinformation_array['numberphysicalcpus'] = 'NA';
-        $siteinformation_array['totalvirtualmemory'] = 'NA';
-        $siteinformation_array['totalphysicalmemory'] = 'NA';
-        $siteinformation_array['logicalprocessorsperphysical'] = 'NA';
-        $siteinformation_array['processorclockfrequency'] = 'NA';
+        $siteinformation_array = [
+            'description' => 'NA',
+            'processoris64bits' => 'NA',
+            'processorvendor' => 'NA',
+            'processorvendorid' => 'NA',
+            'processorfamilyid' => 'NA',
+            'processormodelid' => 'NA',
+            'processorcachesize' => 'NA',
+            'numberlogicalcpus' => 'NA',
+            'numberphysicalcpus' => 'NA',
+            'totalvirtualmemory' => 'NA',
+            'totalphysicalmemory' => 'NA',
+            'logicalprocessorsperphysical' => 'NA',
+            'processorclockfrequency' => 'NA',
+        ];
 
         // Current timestamp is the beginning of the dashboard and we want the end
         $currenttimestamp = gmdate(FMT_DATETIME, $currenttime + 3600 * 24);
@@ -447,8 +448,8 @@ public function viewSite(int $siteid): View
 
         $xml = begin_XML_for_XSLT();
 
-        @$projectid = pdo_real_escape_numeric($_GET['project']);
-        if ($projectid) {
+        $projectid = (int) $_GET['project'];
+        if ($projectid > 0) {
             $project = new Project();
             $project->Id = $projectid;
             $project->Fill();
@@ -503,7 +504,7 @@ public function viewSite(int $siteid): View
         $xml .= add_XML_value('logicalprocessorsperphysical', $siteinformation_array['logicalprocessorsperphysical']);
         $xml .= add_XML_value('processorclockfrequency', getByteValueWithExtension($processor_clock_frequency, 1000) . 'Hz');
         $xml .= add_XML_value('outoforder', $site_array['outoforder']);
-        if ($projectid && $project->ShowIPAddresses) {
+        if ($projectid > 0 && $project->ShowIPAddresses) {
             $xml .= add_XML_value('ip', $site_array['ip']);
             $xml .= add_XML_value('latitude', $site_array['latitude']);
             $xml .= add_XML_value('longitude', $site_array['longitude']);

diff --git a/app/cdash/app/Controller/Api/QueryTests.php b/app/cdash/app/Controller/Api/QueryTests.php
@@ -204,9 +204,8 @@ public function getResponse()
         // If parentid is set we need to lookup the date for this build
         // because it is not specified as a query string parameter.
         if (isset($_GET['parentid'])) {
-            $parentid = pdo_real_escape_numeric($_GET['parentid']);
             $parent_build = new Build();
-            $parent_build->Id = $parentid;
+            $parent_build->Id = (int) $_GET['parentid'];
             $this->setDate($parent_build->GetDate());
         } else {
             // Handle the optional arguments that dictate our time range.

diff --git a/app/cdash/app/Controller/Api/ViewTest.php b/app/cdash/app/Controller/Api/ViewTest.php
@@ -23,6 +23,7 @@
 use CDash\Database;
 use CDash\Model\Build;
 use App\Models\BuildInformation;
+use Illuminate\Support\Facades\DB;
 
 require_once 'include/filterdataFunctions.php';
 
@@ -557,16 +558,16 @@ private function loadTestDetails()
             return;
         }
 
-        $projectid = pdo_real_escape_numeric($_GET['projectid']);
+        $projectid = (int) $_GET['projectid'];
 
         $previous_buildids = [];
         if (array_key_exists('previous_builds', $_GET)) {
             foreach (explode(', ', $_GET['previous_builds']) as $previous_buildid) {
                 if (is_numeric($previous_buildid) && $previous_buildid > 1) {
-                    $previous_build_row = \DB::table('build')
+                    $previous_build_row = DB::table('build')
                         ->where('id', $previous_buildid)
                         ->first();
-                    if ($previous_build_row->projectid == $projectid) {
+                    if ((int) $previous_build_row->projectid === $projectid) {
                         $previous_buildids[] = $previous_buildid;
                     }
                 }
@@ -581,7 +582,7 @@ private function loadTestDetails()
         if (array_key_exists('time_end', $_GET)) {
             $time_end = pdo_real_escape_string($_GET['time_end']);
         }
-        $groupid = pdo_real_escape_numeric($_GET['groupid']);
+        $groupid = (int) $_GET['groupid'];
 
         $response = [];
         $tests_response = [];

diff --git a/app/cdash/include/filterdataFunctions.php b/app/cdash/include/filterdataFunctions.php
@@ -92,7 +92,7 @@ public function __construct()
         // If so, we won't add SQL clauses for some other filters.
         // Instead we handle them in PHP code via build_survives_filter().
         $this->HasSubProjectsFilter = false;
-        $filtercount = pdo_real_escape_numeric(@$_REQUEST['filtercount']);
+        $filtercount = (int) ($_REQUEST['filtercount'] ?? 0);
         for ($i = 1; $i <= $filtercount; ++$i) {
             if (empty($_REQUEST['field' . $i])) {
                 continue;
@@ -104,17 +104,18 @@ public function __construct()
             }
         }
         $this->FiltersAffectedBySubProjects = [
-                'buildduration',
-                'builderrors',
-                'buildwarnings',
-                'configureduration',
-                'configureerrors',
-                'configurewarnings',
-                'testsduration',
-                'testsfailed',
-                'testsnotrun',
-                'testspassed',
-                'testtimestatus'];
+            'buildduration',
+            'builderrors',
+            'buildwarnings',
+            'configureduration',
+            'configureerrors',
+            'configurewarnings',
+            'testsduration',
+            'testsfailed',
+            'testsnotrun',
+            'testspassed',
+            'testtimestatus',
+        ];
     }
 
     public function getDefaultFilter()
@@ -1073,7 +1074,7 @@ function get_filterdata_from_request($page_id = '')
     for ($i = 1; $i <= $filtercount; ++$i) {
         if (array_key_exists("field{$i}count", $_GET)) {
             // Handle block of filters.
-            $subfiltercount = pdo_real_escape_numeric(@$_GET["field{$i}count"]);
+            $subfiltercount = (int) ($_GET["field{$i}count"] ?? 0);
             $filter = [
                 'filters' => [],
             ];

diff --git a/app/cdash/include/pdo.php b/app/cdash/include/pdo.php
@@ -148,27 +148,6 @@ function pdo_real_escape_string(mixed $unescaped_string): string
     return substr($str, 1, strlen($str) - 2); // remove enclosing quotes
 }
 
-/**
- * Case for numeric empty string in Postgres.
- *
- * @deprecated 04/01/2023
- */
-function pdo_real_escape_numeric(mixed $unescaped_string): float|int|string
-{
-    if (config('database.default') === 'pgsql' && $unescaped_string == '') {
-        // MySQL interprets an empty string as zero when assigned to a numeric field,
-        // for PostgreSQL this must be done explicitly:
-        $unescaped_string = '0';
-    }
-
-    // Return zero if we don't end up with a numeric value.
-    $escaped_string = pdo_real_escape_string($unescaped_string);
-    if (!is_numeric($escaped_string)) {
-        return 0;
-    }
-    return $escaped_string;
-}
-
 /**
  * DEPRECATED - use Database::getInstance()->execute(...)
  *

diff --git a/app/cdash/public/api/v1/buildgroup.php b/app/cdash/public/api/v1/buildgroup.php
@@ -27,8 +27,8 @@
 // Require administrative access to view this page.
 init_api_request();
 
-if (array_key_exists('projectid', $_REQUEST)) {
-    $projectid = pdo_real_escape_numeric($_REQUEST['projectid']);
+if (isset($_REQUEST['projectid'])) {
+    $projectid = (int) $_REQUEST['projectid'];
 } else {
     $project = get_project_from_request();
     $projectid = $project->Id;
@@ -61,7 +61,7 @@ function rest_delete()
 {
     if (isset($_GET['buildgroupid'])) {
         // Delete the specified BuildGroup.
-        $buildgroupid = pdo_real_escape_numeric($_GET['buildgroupid']);
+        $buildgroupid = (int) $_GET['buildgroupid'];
         $Group = new BuildGroup();
         $Group->SetId($buildgroupid);
         $Group->Delete();
@@ -197,8 +197,7 @@ function rest_post($pdo, $projectid)
             $groupid = $group['id'];
 
             $Build = new Build();
-            $buildid = pdo_real_escape_numeric($buildinfo['id']);
-            $Build->Id = $buildid;
+            $Build->Id = (int) $buildinfo['id'];
             $Build->FillFromId($Build->Id);
             $prevgroupid = $Build->GroupId;
 
@@ -208,7 +207,7 @@ function rest_post($pdo, $projectid)
                 SET groupid = :groupid
                 WHERE groupid = :prevgroupid AND
                       buildid = :buildid');
-            pdo_execute($stmt, [$groupid, $prevgroupid, $buildid]);
+            pdo_execute($stmt, [$groupid, $prevgroupid, $Build->Id]);
 
             // Soft delete any previous rules.
             $buildgrouprule = new BuildGroupRule($Build);
@@ -315,18 +314,14 @@ function rest_put($projectid)
         }
 
         $BuildGroup = new BuildGroup();
-        $BuildGroup->SetId(pdo_real_escape_numeric($buildgroup['id']));
+        $BuildGroup->SetId((int) $buildgroup['id']);
         $BuildGroup->SetName(pdo_real_escape_string($buildgroup['name']));
         $BuildGroup->SetDescription(
             pdo_real_escape_string($buildgroup['description']));
-        $BuildGroup->SetSummaryEmail(
-            pdo_real_escape_numeric($buildgroup['summaryemail']));
-        $BuildGroup->SetEmailCommitters(
-            pdo_real_escape_numeric($buildgroup['emailcommitters']));
-        $BuildGroup->SetIncludeSubProjectTotal(
-            pdo_real_escape_numeric($buildgroup['includesubprojecttotal']));
-        $BuildGroup->SetAutoRemoveTimeFrame(
-            pdo_real_escape_numeric($buildgroup['autoremovetimeframe']));
+        $BuildGroup->SetSummaryEmail((int) $buildgroup['summaryemail']);
+        $BuildGroup->SetEmailCommitters((int) $buildgroup['emailcommitters']);
+        $BuildGroup->SetIncludeSubProjectTotal((int) $buildgroup['includesubprojecttotal']);
+        $BuildGroup->SetAutoRemoveTimeFrame((int) $buildgroup['autoremovetimeframe']);
 
         if (!$BuildGroup->Save()) {
             abort(500, 'Failed to save BuildGroup');
@@ -335,7 +330,7 @@ function rest_put($projectid)
 
     if (isset($_REQUEST['dynamiclist']) && !empty($_REQUEST['dynamiclist'])) {
         // Update a list of dynamic builds.
-        $buildgroupid = pdo_real_escape_numeric($_REQUEST['buildgroupid']);
+        $buildgroupid = (int) $_REQUEST['buildgroupid'];
         $build_group = new BuildGroup();
         $build_group->SetId($buildgroupid);
         $old_rules = $build_group->GetRules();