Use Fcntl qw(O_RDONLY) for simple cases. This is better than adding a comment which may get out-of-date if more constants are needed. But still use a comment to avoid too much duplication in more complex cases.
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/bin/Engine.pl | 2 +- testbot/lib/WineTestBot/Utils.pm | 2 +- testbot/web/GetFile.pl | 2 +- testbot/web/GetTaskFile.pl | 2 +- testbot/web/Screenshot.pl | 2 +- testbot/web/Submit.pl | 2 +- 6 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/testbot/bin/Engine.pl b/testbot/bin/Engine.pl index db9d82d30..7880b8e80 100755 --- a/testbot/bin/Engine.pl +++ b/testbot/bin/Engine.pl @@ -37,7 +37,7 @@ sub BEGIN }
use Errno qw(EAGAIN); -use Fcntl; +use Fcntl; # For O_XXX and FD_XXX use MIME::Parser; use POSIX ":sys_wait_h"; use Socket; diff --git a/testbot/lib/WineTestBot/Utils.pm b/testbot/lib/WineTestBot/Utils.pm index 27b222fd2..5315c75d4 100644 --- a/testbot/lib/WineTestBot/Utils.pm +++ b/testbot/lib/WineTestBot/Utils.pm @@ -33,7 +33,7 @@ our @EXPORT = qw(SecureConnection MakeSecureURL GetTaskURL GenerateRandomString BuildTag SanitizeTag LocaleName NotifyAdministrator BatchQuote ShQuote ShArgv2Cmd);
-use Fcntl; +use Fcntl qw(O_CREAT O_EXCL O_WRONLY);
use WineTestBot::Config;
diff --git a/testbot/web/GetFile.pl b/testbot/web/GetFile.pl index a8699497b..59602bf19 100644 --- a/testbot/web/GetFile.pl +++ b/testbot/web/GetFile.pl @@ -21,7 +21,7 @@ use strict;
use Apache2::Const -compile => qw(REDIRECT); use CGI; -use Fcntl; # For O_XXX +use Fcntl qw(O_RDONLY); use WineTestBot::Jobs;
diff --git a/testbot/web/GetTaskFile.pl b/testbot/web/GetTaskFile.pl index 41111197d..0b7f167d2 100644 --- a/testbot/web/GetTaskFile.pl +++ b/testbot/web/GetTaskFile.pl @@ -22,7 +22,7 @@ use strict;
use Apache2::Const -compile => qw(REDIRECT); use CGI; -use Fcntl; # for O_READONLY +use Fcntl qw(O_RDONLY); use WineTestBot::Config;
diff --git a/testbot/web/Screenshot.pl b/testbot/web/Screenshot.pl index a8a3d88ed..685c4a45c 100644 --- a/testbot/web/Screenshot.pl +++ b/testbot/web/Screenshot.pl @@ -22,7 +22,7 @@ use strict; use Apache2::Const -compile => qw(REDIRECT); use CGI; use CGI::Cookie; -use Fcntl; +use Fcntl qw(O_RDONLY); use WineTestBot::Config; use WineTestBot::CGI::Sessions; use WineTestBot::Engine::Notify; diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index 472622dd5..e2173d898 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -26,7 +26,7 @@ use ObjectModel::CGI::FreeFormPage; our @ISA = qw(ObjectModel::CGI::FreeFormPage);
use CGI qw(:standard); -use Fcntl; # For O_XXX +use Fcntl qw(O_RDONLY); use IO::Handle; use POSIX qw(:fcntl_h); # For SEEK_XXX use File::Basename;
Not only are they unneeded, but when combined with qw(:standard) they prevent calling $self->escapeHTML() (where $self is an ObjectModel::CGI::Page object).
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/web/Logout.pl | 1 - testbot/web/Submit.pl | 1 - 2 files changed, 2 deletions(-)
diff --git a/testbot/web/Logout.pl b/testbot/web/Logout.pl index 17e81d2b8..b0fd5cdc1 100644 --- a/testbot/web/Logout.pl +++ b/testbot/web/Logout.pl @@ -24,7 +24,6 @@ package LogoutPage; use ObjectModel::CGI::Page; our @ISA = qw(ObjectModel::CGI::Page);
-use CGI qw(:standard); use WineTestBot::CGI::Sessions;
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl index e2173d898..ef7816b46 100644 --- a/testbot/web/Submit.pl +++ b/testbot/web/Submit.pl @@ -25,7 +25,6 @@ package SubmitPage; use ObjectModel::CGI::FreeFormPage; our @ISA = qw(ObjectModel::CGI::FreeFormPage);
-use CGI qw(:standard); use Fcntl qw(O_RDONLY); use IO::Handle; use POSIX qw(:fcntl_h); # For SEEK_XXX
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@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"; }
-
Francois Gouget