Francois Gouget : testbot/web: Explicitly call exit on return from Redirect().
Module: tools Branch: master Commit: d1f74feaa53d4d724b4a3e470eea45d478e7987b URL: https://source.winehq.org/git/tools.git/?a=commit;h=d1f74feaa53d4d724b4a3e47... Author: Francois Gouget <fgouget(a)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(a)codeweavers.com> Signed-off-by: Alexandre Julliard <julliard(a)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($$)
participants (1)
-
Alexandre Julliard