[tools] testbot/web: Explicitly call exit on return from Redirect().
Francois Gouget
fgouget at codeweavers.com
Wed Mar 9 13:12:23 CST 2022
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>
---
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/SpecialJobs2.pl | 3 +--
testbot/web/admin/UserDetails.pl | 9 +++------
11 files changed, 29 insertions(+), 50 deletions(-)
diff --git a/testbot/lib/ObjectModel/CGI/CollectionBlock.pm b/testbot/lib/ObjectModel/CGI/CollectionBlock.pm
index a5e8187009..9027b87758 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 9b9f2ff7e5..3dab64cf71 100644
--- a/testbot/lib/ObjectModel/CGI/ItemPage.pm
+++ b/testbot/lib/ObjectModel/CGI/ItemPage.pm
@@ -141,13 +141,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);
@@ -167,7 +165,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 8abaaaa9d2..b0450d0e78 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 d38e8d96b6..b7edd8fcaa 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"})));
}
}
@@ -415,7 +415,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 919c7a5e3b..8d93957ce6 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 d9757edb92..ac64fc28ea 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 98d13381e0..79d1c8772d 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 cb253bd03e..456e73f55d 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 534336c483..02d9f8ea9d 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/SpecialJobs2.pl b/testbot/web/admin/SpecialJobs2.pl
index c5a8f6587f..3fa5ab11bb 100644
--- a/testbot/web/admin/SpecialJobs2.pl
+++ b/testbot/web/admin/SpecialJobs2.pl
@@ -240,8 +240,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 2cf0f597fb..8c26930e8e 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($$)
--
2.30.2
More information about the wine-devel
mailing list