[tools 3/3] testbot/web: Use the page's escapeHTML() method.

Francois Gouget fgouget at codeweavers.com
Wed Mar 30 12:06:55 CDT 2022


It should normally forward the call to CGI->escapeHTML() but there is no
reason to assume it does. Also having a mix of both is confusing.

Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
---
In particular the reason why JobDetails.pm was using
$self->CGI->escapeHTML() is that it did 'use CGI qw(:standard)' so that
$self->escapeHTML($Str) was interpreted as CGI::escapeHTML($self, $Str),
thus escaping $self instead of $Str!
---
 testbot/lib/ObjectModel/CGI/FormPage.pm | 14 +++++++-------
 testbot/lib/WineTestBot/CGI/PageBase.pm |  6 +++---
 testbot/web/JobDetails.pl               | 10 +++++-----
 testbot/web/Submit.pl                   | 15 +++++++--------
 testbot/web/admin/SpecialJobs.pl        |  6 +++---
 5 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/testbot/lib/ObjectModel/CGI/FormPage.pm b/testbot/lib/ObjectModel/CGI/FormPage.pm
index b85ccb297..9ce9fa93b 100644
--- a/testbot/lib/ObjectModel/CGI/FormPage.pm
+++ b/testbot/lib/ObjectModel/CGI/FormPage.pm
@@ -258,7 +258,7 @@ sub GenerateField($$$)
   my ($self, $PropertyDescriptor, $Display) = @_;
 
   print "<div class='ItemProperty'><label>",
-        $self->CGI->escapeHTML($self->GetDisplayName($PropertyDescriptor)) .
+        $self->escapeHTML($self->GetDisplayName($PropertyDescriptor)),
         "</label>";
 
   my $Value = $self->GetDisplayValue($PropertyDescriptor);
@@ -281,9 +281,9 @@ sub GenerateField($$$)
       print "<select name='", $PropertyDescriptor->GetName(), "'>\n";
       foreach my $V (@{$PropertyDescriptor->GetValues()})
       {
-        print "  <option value='", $self->CGI->escapeHTML($V), "'";
+        print "  <option value='", $self->escapeHTML($V), "'";
         print " selected='selected'" if ($Value eq $V);
-        print ">", $self->CGI->escapeHTML($V), "</option>\n";
+        print ">", $self->escapeHTML($V), "</option>\n";
       }
       print "</select>";
     }
@@ -302,7 +302,7 @@ sub GenerateField($$$)
       print "'>";
       if (defined $Value)
       {
-        print $self->CGI->escapeHTML($Value), "'";
+        print $self->escapeHTML($Value), "'";
       }
       print "</textarea>";
       $self->GenerateRequired($PropertyDescriptor);
@@ -315,7 +315,7 @@ sub GenerateField($$$)
             "' maxlength='", $PropertyDescriptor->GetMaxLength(), "' size='$Size'";
       if (defined $Value && $InputType ne "password")
       {
-        print " value='", $self->CGI->escapeHTML($Value), "'";
+        print " value='", $self->escapeHTML($Value), "'";
       }
       print " />";
       $self->GenerateRequired($PropertyDescriptor);
@@ -326,7 +326,7 @@ sub GenerateField($$$)
   {
     if ($Value)
     {
-      print $self->CGI->escapeHTML($Value);
+      print $self->escapeHTML($Value);
     }
     else
     {
@@ -355,7 +355,7 @@ sub GenerateTitle($)
   my $Title = $self->GetTitle();
   if ($Title)
   {
-    print "<h1 id='PageTitle'>", $self->CGI->escapeHTML($Title), "</h1>\n";
+    print "<h1 id='PageTitle'>", $self->escapeHTML($Title), "</h1>\n";
   }
 }
 
diff --git a/testbot/lib/WineTestBot/CGI/PageBase.pm b/testbot/lib/WineTestBot/CGI/PageBase.pm
index d5fcc478a..58102d397 100644
--- a/testbot/lib/WineTestBot/CGI/PageBase.pm
+++ b/testbot/lib/WineTestBot/CGI/PageBase.pm
@@ -309,7 +309,7 @@ sub GenerateErrorDiv($$)
   if ($ErrMessage)
   {
     print "<noscript>\n";
-    print "<div id='errormessage'>", $Page->CGI->escapeHTML($ErrMessage), "</div>\n";
+    print "<div id='errormessage'>", $Page->escapeHTML($ErrMessage), "</div>\n";
     print "</noscript>\n";
   }
 }
@@ -395,7 +395,7 @@ sub GenerateHeader($$)
 {
   my ($self, $Page) = @_;
 
-  my $Title = $Page->CGI->escapeHTML($Page->GetPageTitle());
+  my $Title = $Page->escapeHTML($Page->GetPageTitle());
   print <<EOF;
 <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" "http://www.w3.org/TR/html4/strict.dtd">
 <html>
@@ -457,7 +457,7 @@ EOF
     print "        <li><p><a href='", MakeSecureURL("/Logout.pl"), "'>Log out";
     if (defined($Session))
     {
-      print " ", $Page->CGI->escapeHTML($Session->User->Name);
+      print " ", $Page->escapeHTML($Session->User->Name);
     }
     print "</a></p></li>\n";
   }
diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl
index 5e302c098..5b8b7013a 100644
--- a/testbot/web/JobDetails.pl
+++ b/testbot/web/JobDetails.pl
@@ -320,7 +320,7 @@ sub GenerateMoreInfoLink($$$$;$$)
   my ($Action, $Url) = $self->GetMoreInfoLink($LinkKey, $Label, $Set, $Value);
   my $Title = ($Value =~ /^(.*)\.report$/) ? " title='$1'" : "";
 
-  my $Html = "<a href='". $self->CGI->escapeHTML($Url) ."'$Title>$Action $Label</a>";
+  my $Html = "<a href='". $self->escapeHTML($Url) ."'$Title>$Action $Label</a>";
   if (defined $Value)
   {
     $Url = "/GetTaskFile.pl?Job=". uri_escape($self->{JobId})
@@ -474,8 +474,8 @@ EOF
           " <span class='right'><a class='title' href='#$Prev'>↑</a><a class='title' href='#$Next'>↓</a></span></h2>\n";
 
     print "<details><summary>",
-          $self->CGI->escapeHTML($VM->Description || $VM->Name), "</summary>",
-          $self->CGI->escapeHTML($VM->Details || "No details!"),
+          $self->escapeHTML($VM->Description || $VM->Name), "</summary>",
+          $self->escapeHTML($VM->Details || "No details!"),
           ($StepTask->Missions ? "<br>Missions: ". $StepTask->Missions : ""),
           "</details>\n";
 
@@ -489,7 +489,7 @@ EOF
                   "&StepKey=" . uri_escape($StepTask->StepNo) .
                   "&TaskKey=" . uri_escape($StepTask->TaskNo);
         print "<div class='Screenshot'><img src='" .
-              $self->CGI->escapeHTML($URI) . "' alt='Screenshot' /></div>\n";
+              $self->escapeHTML($URI) . "' alt='Screenshot' /></div>\n";
       }
       $self->GenerateMoreInfoLink($Key, "final screenshot", "Screenshot");
     }
@@ -509,7 +509,7 @@ EOF
       #
 
       my ($Action, $Url) = $self->GetMoreInfoLink($Key, GetLogLabel($MoreInfo->{Full}), "Full", $MoreInfo->{Full});
-      $Url = $self->CGI->escapeHTML($Url);
+      $Url = $self->escapeHTML($Url);
       my $HideLog = $Action eq "Hide" ? " ondblclick='HideLog(event, \"$Url\")'" : "";
 
       my $LogIsEmpty = $self->GenerateFullLog($TaskDir, $MoreInfo->{Full}, $HideLog);
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index ef7816b46..3d49aaeaa 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -494,7 +494,7 @@ sub _initialize($$$)
       }
     }
 
-    my $FieldBase = $self->CGI->escapeHTML($VMKey);
+    my $FieldBase = $self->escapeHTML($VMKey);
     foreach my $Build (sort @Builds)
     {
       my $VMRow = {
@@ -537,7 +537,7 @@ sub _GenerateStateField($$)
   if (defined $self->{$Name})
   {
     print "<input type='hidden' name='$Name' value='",
-          $self->CGI->escapeHTML($self->{$Name}), "'>\n";
+          $self->escapeHTML($self->{$Name}), "'>\n";
   }
 }
 
@@ -758,12 +758,12 @@ sub GenerateFields($)
       foreach my $Key (@SortedKeys)
       {
         my $Branch = $Branches->GetItem($Key);
-        print "<option value='", $self->CGI->escapeHTML($Key), "'";
+        print "<option value='", $self->escapeHTML($Key), "'";
         if (defined $self->{Branch} and $Key eq $self->{Branch})
         {
           print " selected";
         }
-        print ">", $self->CGI->escapeHTML($Branch->Name), "</option>";
+        print ">", $self->escapeHTML($Branch->Name), "</option>";
       }
       print "</select> <span class='Required'>*</span></div></div>\n";
     }
@@ -839,7 +839,7 @@ EOF
 
       my $Name = $VM->Name;
       $Name .= " ($VMRow->{Build})" if ($VM->Type eq "wine");
-      print "<td>", $self->CGI->escapeHTML($Name), "</td>\n";
+      print "<td>", $self->escapeHTML($Name), "</td>\n";
 
       print "<td>";
       if ($VMRow->{Exe32} and $VMRow->{Exe64})
@@ -868,10 +868,9 @@ EOF
       print "</td>\n";
 
       print "<td><details><summary>",
-            $self->CGI->escapeHTML($VM->Description || $VM->Name);
+            $self->escapeHTML($VM->Description || $VM->Name);
       print " [", $VM->Status ,"]" if ($VM->Status =~ /^(?:offline|maintenance)$/);
-      print "</summary>",
-            $self->CGI->escapeHTML($VM->Details || "No details!"),
+      print "</summary>", $self->escapeHTML($VM->Details || "No details!"),
             "</details></td>";
       print "</tr>\n";
     }
diff --git a/testbot/web/admin/SpecialJobs.pl b/testbot/web/admin/SpecialJobs.pl
index 79160e42f..49f80a0f9 100644
--- a/testbot/web/admin/SpecialJobs.pl
+++ b/testbot/web/admin/SpecialJobs.pl
@@ -117,14 +117,14 @@ sub GenerateJobFields($$$$$)
   foreach my $Option (@{$JobTemplate->{Options}})
   {
     my $Selected = $JobTemplate->{VMKey} eq "*$Option" ? " selected" : "";
-    print "<option value='*", $self->CGI->escapeHTML($Option),
+    print "<option value='*", $self->escapeHTML($Option),
         "'$Selected>$Option</option>\n";
   }
   foreach my $VM (@{$JobTemplate->{VMs}})
   {
     my $Selected = $JobTemplate->{VMKey} eq $VM->Name ? " selected" : "";
-    print "<option value='", $self->CGI->escapeHTML($VM->Name),
-        "'$Selected>", $self->CGI->escapeHTML($VM->Name), "</option>\n";
+    print "<option value='", $self->escapeHTML($VM->Name),
+        "'$Selected>", $self->escapeHTML($VM->Name), "</option>\n";
   }
   print "</select> </div></div>\n";
 }
-- 
2.30.2



More information about the wine-devel mailing list