Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fatal error when redirecting with no URI #677

Closed
wants to merge 1 commit into from

Conversation

omicr0n
Copy link

@omicr0n omicr0n commented Apr 19, 2016

Easy fix, if no uri is passed then return from the function. I ran into this problem having some dynamic code for users that'll redirect to there profile if already logged in, the problem is t he error generated from this fatal error doesn't generate a call stack. Thus I had to debug my code from starting from seeing what was being passed to this function which was an empty string then starting from the master controller and working my way to the called url controller/action. http://i.imgur.com/qUdrELS.png

Easy fix, if no uri is passed then return from the function.  I ran into this problem having some dynamic code for users that'll redirect to there profile if already logged in, the problem is t he error generated from this fatal error doesn't generate a call stack.  Thus I had to debug my code from starting from seeing what was being passed to this function which was an empty string then starting from the master controller and working my way to the called url controller/action.  http://i.imgur.com/qUdrELS.png
@WinterSilence
Copy link

WinterSilence commented Apr 20, 2016

@omicr0n trouble in your routes, not in this class. in my projects HTTP::redirect(''); work fine and redirects to first page. Wrap route uri in ().

    Route::set(
        'frontend', 
        '(<controller>(/<slug>(/<action>)))',
        ['controller' => '[0-9a-zA-Z]+', 'slug' => '[-0-9a-z_]+', 'action' => '[a-z_]+']
    )

@omicr0n
Copy link
Author

omicr0n commented Apr 20, 2016

@WinterSilence I don't think that's the issue, the problem is my code is along the lines of

$user = ORM::factory('Users', $id);

$this->redirect($user->username);

The problem which sure is partly my fault and related to my code by not checking if $user->loaded() but if $id isn't set properly or doesn't contain a proper value then nothing is loaded in the $user object thus null is passed to redirect and the error is caused about Can only throw objects. So in your case to replicate it'd be HTTP::redirect(null);

@WinterSilence
Copy link

You send wrong value and PR don't fix problem - if add this code, then my redirect to home page not work.

@acoulton
Copy link
Member

@omicr0n thanks for the PR but this isn't a safe fix for the issue you're seeing. It's very common to see people relying on the ->redirect() call exiting from the current code block, so we can't just ignore the call and return.

IMHO passing any empty value to redirect is invalid and the method should throw an \InvalidArgumentException. If you want to redirect to the homepage then you should explicitly call ->redirect('/');. However that would break code for @WinterSilence and others so would have to be for 3.4.

The fatal error is happening because the method returns the result of the magic ->location($uri) getter/setter. When you call it with NULL, it returns the current value (a string/null).

To fix this for 3.3 without breaking compatibility, you should change throw $e->location($uri); to $e->location($uri); return $e; which will at least avoid a fatal error and ensure that the method redirects somewhere though exactly where will depend on your routing.

If you submit a new PR for that targeted to 3.3/develop then we could merge it.

@acoulton acoulton closed this Apr 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants