laravel / framework

The Laravel Framework.
https://laravel.com
MIT License
32.66k stars 11.04k forks source link

Redirect Loop - ERR_TOO_MANY_REDIRECTS #27894

Closed MarekAdamicky closed 5 years ago

MarekAdamicky commented 5 years ago

Description:

Website is in the Redirect Loop when I am using function with redirect()->back(); from other site.

Steps To Reproduce:

routes\web.php

Route::get('/test', 'Auth\AuthController@goBack');

Auth\AuthController.php

public function goBack()
{
    return redirect()->back();
}
  1. http://localhost/public
  2. http://localhost/public/go => Loop
MarekAdamicky commented 5 years ago

Fix - Direct url will not create a link back.

MarekAdamicky commented 5 years ago

It's a bug. On the Laravel 5.7.28 same code works correctly.

My suggestion:

If there is no previous page, the function should return home page.

koenhoeijmakers commented 5 years ago

How would laravel "know" what route is your "home" route?

MarekAdamicky commented 5 years ago

I understand that if I go to the URL directly, there is no previous page.

However, if I was first on a different page within the domain (home page) and I changed the URL in the browser directly, it worked before and there was no redirect loop.

It's different when I use an older version of Laravel 5.7.28 and when I use Laravel 5.8.4. I get different behavior. Why? This is a problem. Laravel's functionality has changed.

Maybe it worked through a session that had a different value in Laravel 5.7. But now there is only a new url in the session for the previous page.

For example, I have a GET function to switch the application language. The function will change the language and return to the previous page even if I changed the url directly. Now I'll get a loop for the URL www.domain.com/lang/en.

I corrected my method as follows: return (request()->header('referer')) ? redirect()->back() : redirect()->home();

My suggestion:

If there is no previous page, it returns an error page and not redirector loop.

Your opinion?

Organizm238 commented 5 years ago

@MarekAdamicky Just tested it with fresh Laravel installations (5.7 and 5.8). I have tested opening

Route::get('/test', function () {
    return back();
});

in Incognito tab to surely don't have any 'previous page' in session. Here are results:

  1. Both versions work the same.
  2. Both versions redirect to home page in such case.
  3. The code for redirecting back has not changed from 5.7 to 5.8, I have checked every single line.

So, the problem should be in your code. Or in the way you test it. For example, you do have previous url in session and for some reason it equals the URL with redirect back code. In this case you will surely get 'too many redirect' error. I suggest you doing this:

  1. Try to reproduce the error in Incognito tab. It will help you to debug it.
  2. If the problem persists, update 'steps to reproduce', so we can surely reproduce it. Right now you don't even go to your /test route in your 'steps to reproduce' :)
driesvints commented 5 years ago

Your return (request()->header('referer')) ? redirect()->back() : redirect()->home(); exactly demonstrates why this is broken for you as you haven't set a referer header.

fitztrev commented 5 years ago

@driesvints I'm able to confirm this issue. Here's a shell script to setup a recreation:

#!/bin/sh

mkdir redirect-tests
cd redirect-tests

composer create-project --prefer-dist laravel/laravel=5.7.28 laravel57
echo "Route::get('/back', function () {
    return back();
});" >> laravel57/routes/web.php

composer create-project --prefer-dist laravel/laravel=5.8.3 laravel58
echo "Route::get('/back', function () {
    return back();
});" >> laravel58/routes/web.php

Then run each of these:

echo "Click here to see it's OK with 5.7: http://127.0.0.1:5700/back" && php redirect-tests/laravel57/artisan serve --port 5700 --quiet
echo "Click here to see the issue with 5.8: http://127.0.0.1:5800/back" && php redirect-tests/laravel58/artisan serve --port 5800 --quiet

I actually experienced this same issue in my application since upgrading and I didn't remember it behaving like that.

driesvints commented 5 years ago

Okay I'll re-open this as quite a few people are experiencing issues with this it seems. Thanks for verifying.

godstanis commented 5 years ago

There seems to be a problem with URL::previous() also. It behaves differently from 5.7. Will try to reproduce it tomorrow.

godstanis commented 5 years ago

Somehow there is _previous attribute in Illuminate/Session/Store.php attributes parameter in Laravel 5.8, while it doesn't exist in Laravel 5.7 (at the moment when previous is called, maybe the problem is in Session middleware).

It's reproduced via simple route:

Route::get('test', function() {
    return \URL::previous();
});

It outputs https://my-site.com in Laravel 5.7 and https://my-site.com/test in Laravel 5.8.

Could someone test it to make sure it's not my env problems?

fitztrev commented 5 years ago

Could someone test it to make sure it's not my env problems?

@stasgar I'm getting the same.

driesvints commented 5 years ago

The only real change I see is this commit: https://github.com/laravel/framework/commit/97fc8bdcafc89f6e485f24db1a3c951f5a400fa5

If you revert these changes, does it work like before?

godstanis commented 5 years ago

If you revert these changes, does it work like before?

Nope, it doesnt. Maybe the problem lies deeper, like in Symfony http components?

driesvints commented 5 years ago

Bit at a loss here since I have no idea what could have affected this in Laravel itself. The functionality for this hasn't changed over the past year(s).

godstanis commented 5 years ago

upd: nope, it's not.

godstanis commented 5 years ago

@driesvints

This one 9859d17. Copied-pasted the whole class contents (before these changes) into Laravel 5.8's file in vendor and previous started to work as expected.

driesvints commented 5 years ago

@stasgar are you overwriting any functionality as describe in https://laravel.com/docs/5.8/upgrade#sessions ?

godstanis commented 5 years ago

@driesvints nope, that's the point.

driesvints commented 5 years ago

Okay, checking in on this.

driesvints commented 5 years ago

@stasgar this is a guess but can you try replacing the handle method of the StartSession middleware with the code below? Please make sure to discard any changes you already have by removing the vendor directory and doing a fresh composer install. Let me know if that solves the problem.

    /**
     * Handle an incoming request.
     *
     * @param  \Illuminate\Http\Request  $request
     * @param  \Closure  $next
     * @return mixed
     */
    public function handle($request, Closure $next)
    {
        if (! $this->sessionConfigured()) {
            return $next($request);
        }

        // If a session driver has been configured, we will need to start the session here
        // so that the data is ready for an application. Note that the Laravel sessions
        // do not make use of PHP "native" sessions in any way since they are crappy.
        $request->setLaravelSession(
            $session = $this->startSession($request)
        );

        $this->collectGarbage($session);

        $response = $next($request);

        $this->storeCurrentUrl($request, $session);

        $this->addCookieToResponse(
            $response, $session
        );

        // Again, if the session has been configured we will need to close out the session
        // so that the attributes may be persisted to some storage medium. We will also
        // add the session identifier cookie to the application response headers now.
        $this->saveSession($request);

        return $response;
    }
godstanis commented 5 years ago

@driesvints yeah, this change solves it too.

driesvints commented 5 years ago

We've merged the PR which fixes this.

Dharmik98 commented 5 years ago

I am using the laravel version 5.7.28 and getting same error while using into middleware to transfer if the request having value. Any suggestion So I can solve this:

Code: middleware
if (!empty($request))
        {
          $data = $request;
          return redirect()->route('check-code')->with('request',$data);

        }
Code:controller
public function checkCode($request){

        dd($request);
    } 

Route::get('/check-code','PromoController@checkCode')->name('check-code');

I know its not here to ask problem,but i am not getting anywhere solution for it.