It should be used whenever sending a URL to a third-party. Also document SecureConnection() and MakeSecureURL().
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/lib/WineTestBot/Utils.pm | 38 +++++++++++++++++++++++++++++--- testbot/web/Register.pl | 5 ++--- 2 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/testbot/lib/WineTestBot/Utils.pm b/testbot/lib/WineTestBot/Utils.pm index 13529c323..bc501ff32 100644 --- a/testbot/lib/WineTestBot/Utils.pm +++ b/testbot/lib/WineTestBot/Utils.pm @@ -27,7 +27,8 @@ WineTestBot::Utils - Utility functions =cut
use Exporter 'import'; -our @EXPORT = qw(SecureConnection MakeSecureURL GetTaskURL GenerateRandomString +our @EXPORT = qw(SecureConnection MakeSecureURL MakeOfficialURL GetTaskURL + GenerateRandomString OpenNewFile CreateNewFile CreateNewLink CreateNewDir GetMTime DurationToString BuildEMailRecipient IsValidFileName BuildTag SanitizeTag LocaleName NotifyAdministrator @@ -50,7 +51,8 @@ use WineTestBot::Config;
Returns true if the user accessed the website over a secure connection.
-This relies on the web server setting the $HTTPS environment variable. +This relies on the web server setting the $HTTPS environment variable for CGI +scripts.
=back =cut @@ -69,12 +71,16 @@ Builds a URL that accesses this website using https if possible. The parameter should be an absolute path that includes neither the protocol nor the hostname.
-Note that this method uses $HTTP_HOST which may not match the official website +This relies on the web server setting the $HTTP_HOST environment variable for +CGI scripts. However $HTTP_HOST which may not match the official website hostname. As such this should only be used for providing URLs back to the user accessing the website, not for URLs sent to third-parties.
+See also MakeOfficialURL(). + =back =cut + sub MakeSecureURL($) { my ($URL) = @_; @@ -83,6 +89,32 @@ sub MakeSecureURL($) return $Protocol . ($ENV{"HTTP_HOST"} || $WebHostName) . $URL; }
+=pod +=over 12 + +=item C<MakeOfficialURL()> + +Creates a URL pointing to the official website. +The parameter should be an absolute path that includes neither the protocol +nor the hostname. + +This is the method to use in non-CGI scripts and to build URLs sent to any +third-party (e.g. via email); where a third party is any user other than the +one currently browsing the website. + +See also MakeSecureURL(). + +=back +=cut + +sub MakeOfficialURL($) +{ + my ($URL) = @_; + + my $Protocol = $UseSSL ? "https://" : "http://"; + return "$Protocol$WebHostName$URL"; +} + sub GetTaskURL($$$;$$) { my ($JobId, $StepNo, $TaskNo, $ShowScreenshot, $LogName) = @_; diff --git a/testbot/web/Register.pl b/testbot/web/Register.pl index bbea0eb12..b10be75e7 100644 --- a/testbot/web/Register.pl +++ b/testbot/web/Register.pl @@ -139,9 +139,8 @@ sub OnSendRequest($) { $Msg .= "Remarks:\n" . $self->GetParam("Remarks") . "\n"; } - my $URL = ($UseSSL ? "https://" : "http://") . $WebHostName . - "/admin/UserDetails.pl?Key=" . uri_escape($self->GetParam("Name")); - $Msg .= "\nTo approve or deny the request, please go to " . $URL; + my $URL = MakeOfficialURL("/admin/UserDetails.pl?Key=". uri_escape($self->GetParam("Name"))); + $Msg .= "\nTo approve or deny the request, please go to $URL"; NotifyAdministrator("winetestbot account request", $Msg);
return 1;
Non-CGI scripts should not have $HTTP_HOST anyway so this is the same as MakeSecureURL().
Signed-off-by: Francois Gouget fgouget@codeweavers.com --- testbot/bin/WineRunBuild.pl | 2 +- testbot/bin/WineRunReconfig.pl | 8 ++++---- testbot/bin/WineRunTask.pl | 4 ++-- testbot/bin/WineRunWineTest.pl | 4 ++-- testbot/bin/WineSendLog.pl | 4 ++-- testbot/lib/WineTestBot/Users.pm | 8 ++++---- 6 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/testbot/bin/WineRunBuild.pl b/testbot/bin/WineRunBuild.pl index 1bddc0402..7e8fcede1 100755 --- a/testbot/bin/WineRunBuild.pl +++ b/testbot/bin/WineRunBuild.pl @@ -334,7 +334,7 @@ if (!$Domain->IsPoweredOn()) NotifyAdministrator("Putting the ". $VM->Name ." VM offline", "The ". $VM->Name ." VM should have been powered on to run the task\n". "below but its state was ". $Domain->GetStateDescription() ." instead.\n". - MakeSecureURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n\n". + MakeOfficialURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n\n". "So the VM has been put offline and the TestBot will try to regain\n". "access to it."); WrapUpAndExit('queued', undef, undef, 'boterror vm off'); diff --git a/testbot/bin/WineRunReconfig.pl b/testbot/bin/WineRunReconfig.pl index c1e6e4443..e86326889 100755 --- a/testbot/bin/WineRunReconfig.pl +++ b/testbot/bin/WineRunReconfig.pl @@ -336,7 +336,7 @@ if (!$Domain->IsPoweredOn()) NotifyAdministrator("Putting the ". $VM->Name ." VM offline", "The ". $VM->Name ." VM should have been powered on to run the task\n". "below but its state was ". $Domain->GetStateDescription() ." instead.\n". - MakeSecureURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n\n". + MakeOfficialURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n\n". "So the VM has been put offline and the TestBot will try to regain\n". "access to it."); WrapUpAndExit('queued', undef, undef, 'boterror vm off'); @@ -432,7 +432,7 @@ if ($TA->GetFile("Reconfig.log", "$TaskDir/task.log")) NotifyAdministrator("The ". $VM->Name ." build failed", "The ". $VM->Name ." build failed and $Status\n\n". "See the link below for more details:\n". - MakeSecureURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n"); + MakeOfficialURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n"); $NewStatus = "badbuild"; } my $LogErrMsg = CreateLogErrorsCache($LogInfo); @@ -546,7 +546,7 @@ if ($NewStatus eq 'completed') "Could not recreate the $IdleSnapshot snapshot:\n\n". "$ErrMessage\n\n". "See the link below for more details:\n". - MakeSecureURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n"); + MakeOfficialURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n"); FatalError("Could not recreate the $IdleSnapshot snapshot: $ErrMessage\n"); }
@@ -562,7 +562,7 @@ if ($NewStatus eq 'completed') ." after its update:\n\n". "$ErrMessage\n\n". "See the link below for more details:\n". - MakeSecureURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n"); + MakeOfficialURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n"); } else { diff --git a/testbot/bin/WineRunTask.pl b/testbot/bin/WineRunTask.pl index eb81cf04d..d872ba5fa 100755 --- a/testbot/bin/WineRunTask.pl +++ b/testbot/bin/WineRunTask.pl @@ -386,7 +386,7 @@ if (!$Domain->IsPoweredOn()) NotifyAdministrator("Putting the ". $VM->Name ." VM offline", "The ". $VM->Name ." VM should have been powered on to run the task\n". "below but its state was ". $Domain->GetStateDescription() ." instead.\n". - MakeSecureURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n\n". + MakeOfficialURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n\n". "So the VM has been put offline and the TestBot will try to regain\n". "access to it."); WrapUpAndExit('queued', undef, undef, undef, 'boterror vm off'); @@ -466,7 +466,7 @@ elsif ($Step->Type eq "suite") if (defined($WebHostName)) { my $URL = GetTaskURL($JobId, $StepNo, $TaskNo, 1); - $Script .= "-u ". BatchQuote(MakeSecureURL($URL)) ." "; + $Script .= "-u ". BatchQuote(MakeOfficialURL($URL)) ." "; } my $Tag = $VM->Type ne "win64" ? "" : $Step->FileType eq "exe64" ? "64" : "32"; $Tag = BuildTag($VM->Name, $Tag); diff --git a/testbot/bin/WineRunWineTest.pl b/testbot/bin/WineRunWineTest.pl index 352671934..62a7fc5bc 100755 --- a/testbot/bin/WineRunWineTest.pl +++ b/testbot/bin/WineRunWineTest.pl @@ -383,7 +383,7 @@ if (!$Domain->IsPoweredOn()) NotifyAdministrator("Putting the ". $VM->Name ." VM offline", "The ". $VM->Name ." VM should have been powered on to run the task\n". "below but its state was ". $Domain->GetStateDescription() ." instead.\n". - MakeSecureURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n\n". + MakeOfficialURL(GetTaskURL($JobId, $StepNo, $TaskNo)) ."\n\n". "So the VM has been put offline and the TestBot will try to regain\n". "access to it."); WrapUpAndExit('queued', undef, undef, 'boterror vm off'); @@ -437,7 +437,7 @@ if ($Step->Type eq "suite") if (defined $WebHostName) { my $URL = GetTaskURL($JobId, $StepNo, $TaskNo, 1); - $Script .= "-u ". ShQuote(MakeSecureURL($URL)) ." "; + $Script .= "-u ". ShQuote(MakeOfficialURL($URL)) ." "; } my $Info = $VM->Description ? $VM->Description : ""; if ($VM->Details) diff --git a/testbot/bin/WineSendLog.pl b/testbot/bin/WineSendLog.pl index 65c95de4d..954edaebd 100755 --- a/testbot/bin/WineSendLog.pl +++ b/testbot/bin/WineSendLog.pl @@ -48,6 +48,7 @@ use WineTestBot::Jobs; use WineTestBot::Log; use WineTestBot::LogUtils; use WineTestBot::StepsTasks; +use WineTestBot::Utils;
my $PART_BOUNDARY = "==13F70BD1-BA1B-449A-9CCB-B6A8E90CED47=="; @@ -184,8 +185,7 @@ sub SendLog($) my $StepsTasks = CreateStepsTasks(undef, $Job); my @SortedKeys = sort { $a <=> $b } @{$StepsTasks->GetKeys()};
- my $JobURL = ($UseSSL ? "https://" : "http://") . - "$WebHostName/JobDetails.pl?Key=". $Job->GetKey(); + my $JobURL = MakeOfficialURL("/JobDetails.pl?Key=". $Job->GetKey());
# diff --git a/testbot/lib/WineTestBot/Users.pm b/testbot/lib/WineTestBot/Users.pm index 772a2b7e8..bd9cf358b 100644 --- a/testbot/lib/WineTestBot/Users.pm +++ b/testbot/lib/WineTestBot/Users.pm @@ -138,8 +138,8 @@ sub Approve($) return $ErrMessage; }
- my $URL = MakeSecureURL("/ResetPassword.pl?Name=" . uri_escape($self->Name) . - "&ResetCode=" . uri_escape($self->ResetCode)); + my $URL = MakeOfficialURL("/ResetPassword.pl?Name=". uri_escape($self->Name) + ."&ResetCode=". uri_escape($self->ResetCode)); my $Recipient = $self->GetEMailRecipient(); open (SENDMAIL, "|/usr/sbin/sendmail -oi -t -odq"); print SENDMAIL <<"EOF"; @@ -177,8 +177,8 @@ sub SendResetCode($) return $ErrMessage; }
- my $URL = MakeSecureURL("/ResetPassword.pl?Name=" . uri_escape($self->Name) . - "&ResetCode=" . uri_escape($self->ResetCode)); + my $URL = MakeOfficialURL("/ResetPassword.pl?Name=". uri_escape($self->Name) + ."&ResetCode=". uri_escape($self->ResetCode)); my $UserName = $self->Name; my $Recipient = $self->GetEMailRecipient(); open (SENDMAIL, "|/usr/sbin/sendmail -oi -t -odq");
-
Francois Gouget