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

Issues with redirects - had to use exit to make it work and blank page #1501

Closed
daljit3 opened this issue Nov 18, 2018 · 50 comments
Closed

Issues with redirects - had to use exit to make it work and blank page #1501

daljit3 opened this issue Nov 18, 2018 · 50 comments
Assignees

Comments

@daljit3
Copy link

daljit3 commented Nov 18, 2018


name: Bug report
about: Help us improve the framework by reporting bugs!


Describe the bug
There are two issues with the redirect() method.

  1. I am not able to use return redirect()->to('/'); so I have to follow it up with an exit to make it work. e.g.
public function hello() {
//this won't work;
return redirect()->to('/');
//this works
redirect()->to('/');
exit;
}
  1. The other issue is none of the redirects are working now since approximately after yesterday (17 nov 2018) evening. To make sure, I haven't accidently broken it, I just downloaded a new copy of CI4-develop and after setup I could see welcome message to confirm everything working fine. Then I made these modifications below to the Home controller. If I try mylocalsite/home/daljit it just shows a blank page.
 namespace App\Controllers;

use CodeIgniter\Controller;

class Home extends Controller
{
	public function index()
	{
		return view('welcome_message');
	}

	//--------------------------------------------------------------------
    public function daljit()
    {
        return redirect()->to("/home/hello");
    }

    public function hello()
    {
        echo "Hello!";
    }
}

CodeIgniter 4 version
CodeIgniter 4 develop branch.

@jim-parry jim-parry self-assigned this Nov 18, 2018
@jim-parry
Copy link
Contributor

Ok, the oldInput changes have been reverted, and the unit tests show that bug.
They do not show any redirect() problems.
@daljit3 Can you confirm that redirect works again, and that the old() bug is back?

@jim-parry
Copy link
Contributor

Hmm - after setting the baseURL & indexPage properties in App, I get a blank page even for .../home,
although the default home apge continues to work. This may not be a redirect problem at all, but something deeper.
I have looked at the commits over the last week, and don't see what would cause this.
daljit3, when did you last pull the develop branch from the repo, before the most recent one that seems to be problematic?

@daljit3
Copy link
Author

daljit3 commented Nov 18, 2018

@jim-parry - no sorry redirect still doesn't work and I kind of rely on redirects to get to edit form page to do the testing for old(). Just for clarification, I updated my local copy again just now.

Also, I seem to have found another issue which wasn't there before but I need to investigate more what this new issue is and why I am getting this exception all of sudden as yesterday it was working. Could it be linked to that redirect issue?

CodeIgniter\HTTP\Exceptions\HTTPException 
BASEPATH/HTTP\Exceptions\HTTPException.php at line 106

daljit3, when did you last pull the develop branch from the repo, before the most recent one that seems to be problematic?

I actually update my local copy everyday using git pull and everything except old() bug worked fine.

I can confirm that this issue only started after I did git pull today after seeing your comment that it;s fixed.
I am lucky that I uploaded yesterday's code to our server and went live. I just checked and redirects work fine on my live site.

@jim-parry
Copy link
Contributor

Very strange.
Now you are getting an exception isnide of the Exception class?
And whatever happened occurred over the last 24-26 hours.
Digging further here too.

@daljit3
Copy link
Author

daljit3 commented Nov 18, 2018

Very strange indeed. The reason I am so sure about that it stopped working today is, as I mentioned above, that I published my local site yesterday. My server copy isn't synced to my local as I manually ftp the changes as of now.

@daljit3
Copy link
Author

daljit3 commented Nov 18, 2018

And, because I went live, for that very reason I was doing some tests today on Sunday otherwise I wouldn't have bothered checking on a weekend.

@jim-parry
Copy link
Contributor

@daljit3 I think this should solve both the redirect problem here, as well as the original old() issue.
If you could test, it would be appreciated.

@daljit3
Copy link
Author

daljit3 commented Nov 19, 2018

@jim-parry Yes just tested again:

  1. redirects now work with return statment too
  2. redirect blank page issue is fixed
  3. Old() is working as intended i.e. correctly returns array value

However, now I see another issue (Sorry Jim! :p)
In my views I print validation errors;
<?=(service('validation')->listErrors())?>
Now, without even submitting the form, I see this line printed in an orange colored debug-view container div
1 ROOTPATH/system/Validation/Views/list.php

Also wondering what was the issue with redirects?

@natanfelles
Copy link
Contributor

@daljit3 When you call the listErrors() a view is loaded.

You can check if has errors with getErrors() before to load the view.

@jim-parry
Copy link
Contributor

The redirect issue was caused by my dropping the sendHeaders(), to address a different bug :-/
Solved one but caused another :(

@jim-parry
Copy link
Contributor

Solution confirmed

@daljit3
Copy link
Author

daljit3 commented Nov 22, 2018

@jim-parry hi - it seems like redirects are ending on blank again!! Just updated my local copy today.

@jim-parry
Copy link
Contributor

You update daily, so they were working yesterday and now not today?

@daljit3
Copy link
Author

daljit3 commented Nov 22, 2018

I was working on a different project so I hadn't updated for the past couple of days. I just did and it went blank.

@jim-parry
Copy link
Contributor

i'm trying to isolate "when" this broke. so, sometime in the last couple of days...

@jim-parry
Copy link
Contributor

I am looking at #1504, #1505 or #1513 as causing the problem

@daljit3
Copy link
Author

daljit3 commented Nov 22, 2018

Okay - if you want me to do any quick tests - I am available here for another hour and half. Please let me know.

@jim-parry
Copy link
Contributor

The biggest help would be a unit test case that fails when it shouldn't. It is obvious to me that the ones we have now are missing the boat - they are not catching the problem, even I thought we caught it last week. There could be several things happening here.
I believe the relevant tests are in CommonFunctionTest.
Thanks!

@daljit3
Copy link
Author

daljit3 commented Nov 22, 2018

I'll look and see if I know how to set that up on my local.

The exception error is back too! :(

@daljit3
Copy link
Author

daljit3 commented Nov 22, 2018

I just ran php unit test and I get 105 failures but if I tried vendor\bin\phpunit --filter CommonFunctionTest I get "No code coverage driver is available". I might not have got it config properly?

@jim-parry jim-parry reopened this Nov 22, 2018
@jim-parry
Copy link
Contributor

to run just the one test, vendor/bin/phpunit tests/system/CommonFunctionsTest.php

@natanfelles
Copy link
Contributor

@daljit3 Please, check if you have the PHP display_error active. And if your \Config\App::baseURL has the scheme (http or https), otherwise it ends in a blank page.

@lonnieezell
Copy link
Member

With the latest code, I tried the following code, navigated to http://localhost:8080 and it redirected me to /home/hello as expected.

<?php namespace App\Controllers;

use CodeIgniter\Controller;

class Home extends Controller
{
	public function index()
	{
		return redirect()->to('/home/hello');

		return view('welcome_message');
	}

	//--------------------------------------------------------------------

	public function hello()
	{
		die('hello');
	}
}

That makes me think it's less a bug and more a configuration error on your app, or something? Can you provide exact code you're able to replicate it on, and tell us what the exception is that's showing up?

@daljit3
Copy link
Author

daljit3 commented Nov 23, 2018

@lonnieezell @natanfelles @jim-parry
Yes I do have "http" in my .env, app.baseURL = 'http://mylocalsite.dev/'

I think I have found what's causing issue for me.

As mentioned before, return redirect()->to('/'); never worked for me before @jim-parry made some changes the other day. However, I still some places where I have

redirect('/user-home');
exit;

To fix this issue, I have to replace with return redirect()->to('/user-home');

The bizzare thing is that both option worked for me with after @jim-parry fix and it only stopped since I raised this once again.

Here's my quick and dirty test with results on a fresh installation today at 09:30 AM 23/11/2018.

class Home extends Controller
{
	public function index()
	{
        echo "<h2>Hello there!</h2>";
        echo "<ol>";
        echo "<li><a href='/home/test1' target='_blank'>Go to Test 1</a></li>";
        echo "<li><a href='/home/test2' target='_blank'>Go to Test 2</a></li>";
        echo "<li><a href='/home/test3' target='_blank'>Go to Test 3</a></li>";
        echo "<li><a href='/home/test4' target='_blank'>Go to Test 4</a></li>";
        echo "<li><a href='/home/test5' target='_blank'>Go to Test 5</a></li>";
        echo "</ol>";
	}

    public function test1()
    {
        redirect("/home/login");
        exit;

        //Fails with this error
        /*
            CRITICAL - 2018-11-23 03:23:38 -->
            #0 C:\xampp-7.2.2\htdocs\myproject\system\HTTP\RedirectResponse.php(86): CodeIgniter\HTTP\Exceptions\HTTPException::forInvalidRedirectRoute('')
            #1 C:\xampp-7.2.2\htdocs\myproject\system\Common.php(821): CodeIgniter\HTTP\RedirectResponse->route(false)
            #2 C:\xampp-7.2.2\htdocs\myproject\application\Controllers\Home.php(18): redirect('/home/login')
            #3 C:\xampp-7.2.2\htdocs\myproject\system\CodeIgniter.php(825): App\Controllers\Home->test1()
            #4 C:\xampp-7.2.2\htdocs\myproject\system\CodeIgniter.php(325): CodeIgniter\CodeIgniter->runController(Object(App\Controllers\Home))
            #5 C:\xampp-7.2.2\htdocs\myproject\system\CodeIgniter.php(235): CodeIgniter\CodeIgniter->handleRequest(NULL, Object(Config\Cache), false)
            #6 C:\xampp-7.2.2\htdocs\myproject\public\index.php(45): CodeIgniter\CodeIgniter->run()
            #7 {main}
                 * */
    }
    public function test2() {
        // This test works OK for me
        return redirect()->to("/home/login");
    }
    public function test3() {
        // This test works OK for me
        return redirect()->to("/home/login/?referer=test2");
    }
    public function test4() {
        $refUrl = currentUrlWithQueryString();
        $redirect_to_url = '/home/login/?ref='.$refUrl;

        redirect($redirect_to_url);
        exit;

        /* Failes with this error below

        CRITICAL - 2018-11-23 03:39:20 -->
        #0 C:\xampp-7.2.2\htdocs\myproject\system\HTTP\RedirectResponse.php(86): CodeIgniter\HTTP\Exceptions\HTTPException::forInvalidRedirectRoute('')
        #1 C:\xampp-7.2.2\htdocs\myproject\system\Common.php(821): CodeIgniter\HTTP\RedirectResponse->route(false)
        #2 C:\xampp-7.2.2\htdocs\myproject\application\Controllers\Home.php(52): redirect('/home/login/?re...')
        #3 C:\xampp-7.2.2\htdocs\myproject\system\CodeIgniter.php(825): App\Controllers\Home->test4()
        #4 C:\xampp-7.2.2\htdocs\myproject\system\CodeIgniter.php(325): CodeIgniter\CodeIgniter->runController(Object(App\Controllers\Home))
        #5 C:\xampp-7.2.2\htdocs\myproject\system\CodeIgniter.php(235): CodeIgniter\CodeIgniter->handleRequest(NULL, Object(Config\Cache), false)
        #6 C:\xampp-7.2.2\htdocs\myproject\public\index.php(45): CodeIgniter\CodeIgniter->run()
        #7 {main}
        */
    }
    public function test5() {
	    // This test works OK for me
        $refUrl = currentUrlWithQueryString();
        $redirect_to_url = '/home/login/?ref='.$refUrl;

        redirect()->to($redirect_to_url);
        exit;
    }

    public function login() {
        echo "<h2>Welcome to our Login page! :) </h2>";
        echo "<ol>";
        echo "<li><a href='/home'>Go to Home</a></li>";
        echo "</ol>";

    }
}

///---------------- Some helpers -----------------------------------//

function currentUrlWithQueryString($includeBaseUrl=false) {

    $uri = $_SERVER['REQUEST_URI'];
    if($includeBaseUrl == true) {
        $uri = base_url($uri);
    }
    return $uri;
}

@InsiteFX
Copy link
Contributor

You can no longer use .dev for localhost use .local

These changes were made in chrome and other web browsers.

@daljit3
Copy link
Author

daljit3 commented Nov 23, 2018

@InsiteFX yes I am aware of that and that's not the issue.

@lonnieezell
Copy link
Member

Ah, ok, I see what's going on and looks like docs will take care of most of it. Suggestions after the break:

  1. The redirect() method is built to have the response returned. That is the proper use. So times when you redirect() then exit will never send the actual redirect headers. A blank page is what you should see here.
  2. redirect($url) uses the reverse routing rules to determine the route to go to. You're getting an exception because it's unable to find a route name and/or controller to route to as per the reverse routing needs. That could be cleaned up to provide a better experience. They are there as the default to help with one of the elements that used to be on OWASP's Top 10 list - Unvalidated Redirects and Forwards.

Here's what I see is needed to fix this up:

  1. Update redirect() docs to specify that it's using reverse routing rules (and why)
  2. Potentially provide additional details on the URI Routing page
  3. Look into providing a cleaner, more descriptive error when no matched routing rule exists.

@daljit3
Copy link
Author

daljit3 commented Nov 27, 2018

@lonnieezell yes that makes sense however please take a look at this test below ...

It doesn't redirect to login but just displays the text "You are here because you are logged in". That's incorrect. It should have redirected to /home/login.

The other tests 1 to 5 above still seem to work OK so can't figure out what's going wrong with this one.

I tested this is on fresh installation which I downloaded few mins ago.

namespace App\Controllers;

use CodeIgniter\Controller;

class Home extends Controller
{
    public function test6() {
		redirectIfNotLoggedIn();
		echo "You are here because you are logged in!";
    }
   public function login() {
		echo "You must login first!";
    }
}

///---------------- Some helpers -----------------------------------//
function redirectIfNotLoggedIn($refUrl=null) {
    if($refUrl == null) {
        $refUrl = currentUrlWithQueryString();
    }
    if(isUserLoggedIn() == false) {
        return redirect()->to('/home/login/?ref='.$refUrl);
    }
}
function isUserLoggedIn() {
    if (session('loggedin') == true) {
        return true;
    }
    return false;
}
function currentUrlWithQueryString($includeBaseUrl=false) {
    $uri = $_SERVER['REQUEST_URI'];
    if($includeBaseUrl == true) {
        $uri = base_url($uri);
    }
    return $uri;
}

@natanfelles
Copy link
Contributor

natanfelles commented Nov 27, 2018

You must return to stop the script:

   public function test6() {
		if(isUserLoggedIn() == false) {
				RETURN redirect()->to('/home/login/?ref='.'$refUrl');
		}
		echo "You are here because you are logged in!";
    }

@daljit3
Copy link
Author

daljit3 commented Nov 27, 2018

@natanfelles -

I have that return set up in redirectIfNotLoggedIn(); ?? Will this not work?

I had it working fine when redirect('/'); exit; was working.

@natanfelles
Copy link
Contributor

@daljit3
Copy link
Author

daljit3 commented Nov 27, 2018

@natanfelles Sorry if I wasn't clear but are you referring to @lonnieezell on my old method of using redirect/exit? I have all the redirects now with the returns.

I am trying to understand why the below isn't redirecting. I am returning in the redirectifnotloggedin();

    public function test6() {
		redirectIfNotLoggedIn();
		echo "You are here because you are logged in!";
    }
function redirectIfNotLoggedIn($refUrl=null) {
    if($refUrl == null) {
        $refUrl = currentUrlWithQueryString();
    }
    if(isUserLoggedIn() == false) {
        return redirect()->to('/home/login/?ref='.$refUrl);
    }
}

@natanfelles
Copy link
Contributor

To complete: You must return in the Controller method as well. Like the example I gave you.

I used only part of your functions because they do not allow it to be used consistently.

@daljit3
Copy link
Author

daljit3 commented Nov 27, 2018

I may be wrong but I am not convinced I need to return from controller too? I mean I expect the redirect to happen when user is not signed in inside redirectIfNotLoggedIn() method and otherwise just continue.

I can surely achieve that using different logic e.g. the way you suggested i.e. in the controller. However, I need to check that at various diffrent points so I would end up repeated If/Else checks throughout my project or perhaps I will end up writing my own redirect helper.

function my_redirect($url) {
	header("Location: $url");
	exit(0);    
} 

@natanfelles
Copy link
Contributor

natanfelles commented Nov 27, 2018

Yes. It continues, because it returns a RedirectResponse (as the Lonnie's comment explain).

It will not exit because if you exit, Filters, etc, are not executed.

You could to use your custom my_redirect() if you want, but this will preclude some of the tasks that the framework does before finalizing the response.

@daljit3
Copy link
Author

daljit3 commented Nov 27, 2018

I just feel the correct behaviour is that it should redirect() from redirectIfNotLoggedIn() when the condition fails as it's still being called in from a controller method.

@lonnieezell
Copy link
Member

Here's the code in question, right?

public function test6() {
		redirectIfNotLoggedIn();
		echo "You are here because you are logged in!";
    }

While redirectIfNotLoggedIn() does return the RedirectResponse, nothing is ever done with that response in the test6 method. In order for it to work, that redirectResponse that you just got from redirectIfNotLoggedIn() needs to be returned out of the controller, otherwise the headers are never sent to the browser.

At this point, we're dealing with a support issue, and not a bug. If you still have questions, please take it to the forums. I'm only leaving this open to remind us that we have a few things (listed above) that we need to take care of.

@daljit3
Copy link
Author

daljit3 commented Nov 27, 2018

Yes understand. Thanks

@niteraven7
Copy link

@daljit3 Are you using "php spark serve" for local development and do you have a filter (FilterInterface) in your code? I'm going on a hunch here.

@cobafesbuk
Copy link

Hi, I'm currently migrating my old CodeIgniter code.
I also encounter this issue today.

The redirect() won't work if there's no exit() after it. return redirect() is also not working.
Just wanted to say that it worked without exit() in the old CodeIgniter.

@lonnieezell
Copy link
Member

Well - this isn’t old CodeIgniter so that parts not helpful :)

We’ve gone over and over this and I’m pretty sure it’s solid at this point. Can you provide a code sample?

If I had to take a blind guess it’s that you’re trying to redirect out of a controller constructor and that doesn’t work any longer. You should check out Controller Gilters in the docs.

@cobafesbuk
Copy link

Hi, thanks lonnieezell for pointing it out. I just did a bit of test and found that the return works on function that's not constructor.

I read about the Controller Filters but couldn't found out how to intergrate my variables in constructor to the Filters so for now I'll just use the exit() for workaround.

@ghost
Copy link

ghost commented Mar 13, 2020

experiencing this problem too.

I may be wrong but I am not convinced I need to return from controller too? I mean I expect the redirect to happen when user is not signed in inside redirectIfNotLoggedIn() method and otherwise just continue.

I can surely achieve that using different logic e.g. the way you suggested i.e. in the controller. However, I need to check that at various diffrent points so I would end up repeated If/Else checks throughout my project or perhaps I will end up writing my own redirect helper.

function my_redirect($url) {
	header("Location: $url");
	exit(0);    
} 

this worked for me!

@avegacms
Copy link
Contributor

experiencing this problem too.

I may be wrong but I am not convinced I need to return from controller too? I mean I expect the redirect to happen when user is not signed in inside redirectIfNotLoggedIn() method and otherwise just continue.
I can surely achieve that using different logic e.g. the way you suggested i.e. in the controller. However, I need to check that at various diffrent points so I would end up repeated If/Else checks throughout my project or perhaps I will end up writing my own redirect helper.

function my_redirect($url) {
	header("Location: $url");
	exit(0);    
} 

this worked for me!

Hi. In this case you may use Filters. See that example https://www.patreon.com/posts/visit-logging-27277581

@ghost
Copy link

ghost commented Mar 13, 2020

experiencing this problem too.

I may be wrong but I am not convinced I need to return from controller too? I mean I expect the redirect to happen when user is not signed in inside redirectIfNotLoggedIn() method and otherwise just continue.
I can surely achieve that using different logic e.g. the way you suggested i.e. in the controller. However, I need to check that at various diffrent points so I would end up repeated If/Else checks throughout my project or perhaps I will end up writing my own redirect helper.

function my_redirect($url) {
	header("Location: $url");
	exit(0);    
} 

this worked for me!

Hi. In this case you may use Filters. See that example https://www.patreon.com/posts/visit-logging-27277581

The redirect functions were not working. thats wat i needed, the filters will not do.
what i want:

in my admin controller i use a library with some checks. it wil update my routes file and will redirect me to the admin controller again from within my library. Here the redirect->to(current_url()) is not working and it just keeps executing the code but it should redirect with a refresh to the same url.

adding this works:
header("Location: ".current_url());
exit(0);

@MGatner
Copy link
Member

MGatner commented Mar 13, 2020

You who are having issues still, are you remembering to return the result of redirect()? And also not using it in the controller constructor? If you really really can’t get it to work a better alternative to the “header...exit” code above would be to throw a RedirectException, has a slightly nicer cleanup:
https://codeigniter4.github.io/CodeIgniter4/general/errors.html#redirectexception

@pixobit
Copy link
Contributor

pixobit commented Apr 25, 2020

I'm having issues with this as well in the latest version...
It seems like redirect()->to(url) doesn't do anything

@VishwasGagrani
Copy link

VishwasGagrani commented Jul 27, 2022

I am facing the same problem. These don't work.

redirect('displayProfileView_Named_Route');

redirect()->route('displayProfileView_Named_Route');

However when I use it with response, it works

$this->response->redirect('/displayProfileView');

As suggested above this too works:

  header("Location: displayProfileView");
exit(0);   

Can this be an issue related to .htaccess?
My htaccess:

Options +FollowSymlinks -Indexes
RewriteEngine on

DirectoryIndex index.php
RewriteCond $1 !^(index\.php|resources|robots\.txt)
RewriteCond %{REQUEST_FILENAME} !-f
RewriteCond %{REQUEST_FILENAME} !-d
RewriteRule ^(.*)$ index.php/$1 [L,QSA]

@kenjis
Copy link
Member

kenjis commented Jul 27, 2022

See https://codeigniter4.github.io/CodeIgniter4/general/common_functions.html#redirect

@samualmartin
Copy link

@daljit3

I also faced the same issue, but i was able to solve it . i belive it an mistake in the routes file or the redirection url.
try changing it like(removing the '/ ' before view file or try 'home/hello'.

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

No branches or pull requests