14

Do I understand correctly that after doing $this->dispatcher->forward() or $this->response->redirect() I need to manually ensure that the rest of the code does't get executed? Like below, or am I missing something?

public function signinAction()
{
    if ($this->isUserAuthenticated())
    {
        $this->response->redirect('/profile');
        return;
    }

    // Stuff if he isn't authenticated…
}
Ian Bytchek
  • 8,804
  • 6
  • 46
  • 72

3 Answers3

28

After almost a year of working on a hardcore project that uses Phalcon beyond its capacity, I wanted to clarify a few things and answer my own question. To understand how to properly do redirects and forwards you need to understand a little about how the Dispatcher::dispatch method works.

Take a look at the code here, though it's all C mumbo-jumbo to most of us, its really well written and documented. In the nutshell this is what it does:

  1. The dispatcher enters the while loop until the _finished property becomes true or it discovers a recursion.
  2. Inside the loop, it immediately sets that property to true, so when it starts the next iteration it will automatically break.
  3. It then gets the controller / action information, which are originally supplied by the router in the application, and does various checks. Before and after that it also completes a lot of event-related business.
  4. Finally it calls the action method in the controller and updates the _returnedValue property with (guess what!) the returned value.
  5. If during the action call you call Dispatcher::forward method, it will update the _finished property back to false, which will allow the while loop to continue from the step 2 of this list.

So, after you do redirect or forward, you need to ensure that you code doesn't get executed only if that is part of the expected logic. In other words you don't have to return the result of return $this->response->redirect or return $this->dispatcher->forward.

Doing the last might seem convenient, but not very correct and might lead to problems. In 99.9% cases your controller should not return anything. The exception would be when you actually know what you are doing and want to change the behaviour of the rendering process in your application by returning the response object. On top of that your IDE might complain about inconsistent return statements.

To finalise, the correct way to redirect from within the controller:

// Calling redirect only sets the 30X response status. You also should
// disable the view to prevent the unnecessary rendering.

$this->response->redirect('/profile');
$this->view->disable();

// If you are in the middle of something, you probably don't want 
// the rest of the code running.

return; 

And to forward:

$this->dispatcher->forward(['action' => 'profile']);

// Again, exit if you don't need the rest of the logic.

return;
Ian Bytchek
  • 8,804
  • 6
  • 46
  • 72
  • wouldn't a simple `die()` be enough after `response->redirect()`? (I mean instead of disabling the view and returning) – marcv Nov 21 '14 at 13:53
  • 2
    In most cases yes, in some cases no, e.g., when there's some logic to run after action that is outside it and most frameworks, including Phalcon, do have that logic. In Phalcon case, from [here](https://github.com/phalcon/cphalcon/blob/8aba8584d54237af9f2c0e5eba3dcc8a950cd225/ext/dispatcher.c#L964) on you can see what happens after the last action, though in most cases that doesn't play any role if you `die` on it. `die` is a hacky way to finish the request, there are a few opinions around why using it is a bad practice, especially when using a it inside a framework that handles that request. – Ian Bytchek Nov 21 '14 at 14:16
10

You need to use it like this:

return $this->response->redirect('/profile');

or

return $this->dispatcher->forward(array(
    'action' => 'profile'
))
crada
  • 379
  • 2
  • 6
2

Use send() like this

public function signinAction()
{
    if ($this->isUserAuthenticated())
    {
        return $this->response->redirect('profile')->send();
    }
}
NSukonny
  • 1,090
  • 11
  • 18
  • 1
    This helped me fix it. I was only using response->redirect (with view->disable() beforehand), but still my actions (that should have been protected by a login) got executed (even though the login-page was shown), adding the missing "send()" got it working. – Select0r Jul 21 '17 at 11:13
  • 1
    Thanks! I had separated function to redirect, so `return` doesn't work. Your solution helped me – Robin71 Sep 10 '17 at 16:21