Francois Gouget : testbot/web: Explicitly call exit on return from Redirect().
Alexandre Julliard
julliard at winehq.org
Thu Mar 10 16:09:36 CST 2022
Module: tools
Branch: master
Commit: d1f74feaa53d4d724b4a3e470eea45d478e7987b
URL: https://source.winehq.org/git/tools.git/?a=commit;h=d1f74feaa53d4d724b4a3e470eea45d478e7987b
Author: Francois Gouget <fgouget at codeweavers.com>
Date: Thu Mar 10 11:20:21 2022 +0100
testbot/web: Explicitly call exit on return from Redirect().
Having Redirect() call exit() itself is confusing and requires
documenting that it does not return in each call point and/or adding
a defensive exit() call anyway.
Using the exit(Redirect(...)) construct is more explicit, concise and
robust.
Signed-off-by: Francois Gouget <fgouget at codeweavers.com>
Signed-off-by: Alexandre Julliard <julliard at winehq.org>
---
testbot/lib/ObjectModel/CGI/CollectionBlock.pm | 18 +++++++-----------
testbot/lib/ObjectModel/CGI/ItemPage.pm | 10 ++++------
testbot/lib/ObjectModel/CGI/Page.pm | 2 +-
testbot/lib/WineTestBot/CGI/PageBase.pm | 6 +++---
testbot/web/JobDetails.pl | 8 +++-----
testbot/web/Login.pl | 3 +--
testbot/web/Submit.pl | 13 +++----------
testbot/web/admin/Log.pl | 4 ++--
testbot/web/admin/SpecialJobs.pl | 3 +--
testbot/web/admin/UserDetails.pl | 9 +++------
10 files changed, 28 insertions(+), 48 deletions(-)
diff --git a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
index a5e8187..9027b87 100644
--- a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
+++ b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
@@ -539,20 +539,16 @@ sub OnAction($$)
"=" . uri_escape($MasterColValues->[$ColIndex]);
}
}
- $self->{EnclosingPage}->Redirect($Target);
- return 1;
+ exit($self->{EnclosingPage}->Redirect($Target));
}
- else
+
+ my $Ok = 1;
+ foreach my $Key (@{$self->{Collection}->GetKeys()})
{
- my $Ok = 1;
- foreach my $Key (@{$self->{Collection}->GetKeys()})
+ if (defined $self->{EnclosingPage}->GetParam($self->SelName($Key)) && $Ok)
{
- if (defined($self->{EnclosingPage}->GetParam($self->SelName($Key))) &&
- $Ok)
- {
- my $Item = $self->{Collection}->GetItem($Key);
- $Ok = $self->CallOnItemAction($Item, $Action);
- }
+ my $Item = $self->{Collection}->GetItem($Key);
+ $Ok = $self->CallOnItemAction($Item, $Action);
}
}
}
diff --git a/testbot/lib/ObjectModel/CGI/ItemPage.pm b/testbot/lib/ObjectModel/CGI/ItemPage.pm
index 6d94491..9401154 100644
--- a/testbot/lib/ObjectModel/CGI/ItemPage.pm
+++ b/testbot/lib/ObjectModel/CGI/ItemPage.pm
@@ -151,13 +151,11 @@ sub OnAction($$)
if ($Action eq "OK")
{
return !1 if (!$self->Save());
- $self->RedirectToList();
- exit;
+ exit($self->RedirectToList());
}
- elsif ($Action eq "Cancel")
+ if ($Action eq "Cancel")
{
- $self->RedirectToList();
- exit;
+ exit($self->RedirectToList());
}
return $self->SUPER::OnAction($Action);
@@ -177,7 +175,7 @@ sub RedirectToList($)
"=" . url_escape($MasterColValues->[$ColIndex]);
}
}
- $self->Redirect($Target); # does not return
+ return $self->Redirect($Target);
}
1;
diff --git a/testbot/lib/ObjectModel/CGI/Page.pm b/testbot/lib/ObjectModel/CGI/Page.pm
index 8abaaaa..b0450d0 100644
--- a/testbot/lib/ObjectModel/CGI/Page.pm
+++ b/testbot/lib/ObjectModel/CGI/Page.pm
@@ -246,7 +246,7 @@ sub Redirect($$)
{
my ($self, $Location) = @_;
- $self->{PageBase}->Redirect($self, $Location);
+ return $self->{PageBase}->Redirect($self, $Location);
}
sub GetCurrentSession($)
diff --git a/testbot/lib/WineTestBot/CGI/PageBase.pm b/testbot/lib/WineTestBot/CGI/PageBase.pm
index 2681ece..322552d 100644
--- a/testbot/lib/WineTestBot/CGI/PageBase.pm
+++ b/testbot/lib/WineTestBot/CGI/PageBase.pm
@@ -56,7 +56,7 @@ sub new($$$$@)
! $Session->User->HasRole($RequiredRole))
{
my $LoginURL = "/Login.pl?Target=" . uri_escape($ENV{"REQUEST_URI"});
- $self->Redirect($Page, MakeSecureURL($LoginURL));
+ exit($self->Redirect($Page, MakeSecureURL($LoginURL)));
}
}
@@ -75,7 +75,7 @@ sub CheckSecurePage($$)
if ($UseSSL && ! SecureConnection())
{
- $self->Redirect($Page, MakeSecureURL($ENV{"REQUEST_URI"}));
+ exit($self->Redirect($Page, MakeSecureURL($ENV{"REQUEST_URI"})));
}
}
@@ -413,7 +413,7 @@ sub Redirect($$$)
}
$self->{Request}->headers_out->set("Location", $Location);
$self->{Request}->status(Apache2::Const::REDIRECT);
- exit;
+ return 0; # a suitable exit code
}
sub GetCurrentSession($)
diff --git a/testbot/web/JobDetails.pl b/testbot/web/JobDetails.pl
index 919c7a5..8d93957 100644
--- a/testbot/web/JobDetails.pl
+++ b/testbot/web/JobDetails.pl
@@ -52,7 +52,7 @@ sub _initialize($$$)
$self->{Job} = CreateJobs()->GetItem($JobId);
if (!defined $self->{Job})
{
- $self->Redirect("/"); # does not return
+ exit($self->Redirect("/"));
}
$self->{JobId} = $JobId;
@@ -193,8 +193,7 @@ sub OnCancel($)
return !1;
}
- $self->Redirect("/JobDetails.pl?Key=" . $self->{JobId}); # does not return
- exit;
+ exit($self->Redirect("/JobDetails.pl?Key=" . $self->{JobId}));
}
sub OnRestart($)
@@ -215,8 +214,7 @@ sub OnRestart($)
return !1;
}
- $self->Redirect("/JobDetails.pl?Key=" . $self->{JobId}); # does not return
- exit;
+ exit($self->Redirect("/JobDetails.pl?Key=" . $self->{JobId}));
}
sub OnAction($$$)
diff --git a/testbot/web/Login.pl b/testbot/web/Login.pl
index d9757ed..ac64fc2 100644
--- a/testbot/web/Login.pl
+++ b/testbot/web/Login.pl
@@ -135,8 +135,7 @@ sub OnLogIn($)
{
$Target = "/";
}
- $self->Redirect(MakeSecureURL($Target)); # does not return
- exit;
+ exit($self->Redirect(MakeSecureURL($Target)));
}
sub OnAction($$)
diff --git a/testbot/web/Submit.pl b/testbot/web/Submit.pl
index 98d1338..79d1c87 100644
--- a/testbot/web/Submit.pl
+++ b/testbot/web/Submit.pl
@@ -1265,8 +1265,7 @@ sub _SubmitJob($$)
return undef;
}
- $self->Redirect("/JobDetails.pl?Key=". $NewJob->GetKey()); # does not return
- exit;
+ exit($self->Redirect("/JobDetails.pl?Key=". $NewJob->GetKey()));
}
sub OnSubmit($)
@@ -1323,14 +1322,8 @@ sub OnOK($)
{
my ($self) = @_;
- if (defined $self->{JobKey})
- {
- $self->Redirect("/JobDetails.pl?Key=$self->{JobKey}"); # does not return
- }
- else
- {
- $self->Redirect("/"); # does not return
- }
+ my $Target = defined $self->{JobKey} ? "/JobDetails.pl?Key=$self->{JobKey}" : "/";
+ exit($self->Redirect($Target));
}
sub OnAction($$)
diff --git a/testbot/web/admin/Log.pl b/testbot/web/admin/Log.pl
index cb253bd..456e73f 100644
--- a/testbot/web/admin/Log.pl
+++ b/testbot/web/admin/Log.pl
@@ -71,8 +71,8 @@ sub GetActions($)
sub OnDownload($)
{
my ($self) = @_;
- $self->Redirect("/admin/SendLog.pl?Hours=". $self->GetParam("Hours")); # does not return
- exit;
+
+ exit($self->Redirect("/admin/SendLog.pl?Hours=". $self->GetParam("Hours")));
}
sub OnAction($$)
diff --git a/testbot/web/admin/SpecialJobs.pl b/testbot/web/admin/SpecialJobs.pl
index 534336c..02d9f8e 100644
--- a/testbot/web/admin/SpecialJobs.pl
+++ b/testbot/web/admin/SpecialJobs.pl
@@ -237,8 +237,7 @@ sub OnSubmit($)
return undef;
}
- $self->Redirect("/"); # does not return
- exit;
+ exit($self->Redirect("/"));
}
sub OnAction($$)
diff --git a/testbot/web/admin/UserDetails.pl b/testbot/web/admin/UserDetails.pl
index 2cf0f59..8c26930 100644
--- a/testbot/web/admin/UserDetails.pl
+++ b/testbot/web/admin/UserDetails.pl
@@ -74,8 +74,7 @@ sub OnApprove($)
return !1 if (!$self->Save());
$self->{ErrMessage} = $self->{Item}->Approve();
return !1 if (defined $self->{ErrMessage});
- $self->RedirectToList(); # does not return
- exit;
+ exit($self->RedirectToList());
}
sub OnReject($)
@@ -87,8 +86,7 @@ sub OnReject($)
return !1 if (defined $self->{ErrMessage});
# Forcefully log out that user by deleting his web sessions
DeleteSessions($self->{Item});
- $self->RedirectToList(); # does not return
- exit;
+ exit($self->RedirectToList());
}
sub OnOK($)
@@ -101,8 +99,7 @@ sub OnOK($)
# Forcefully log out that user by deleting his web sessions
DeleteSessions($self->{Item});
}
- $self->RedirectToList(); # does not return
- exit;
+ exit($self->RedirectToList());
}
sub OnAction($$)
More information about the wine-cvs
mailing list