[5.8] Previous URL in Session contains current full URL

[tswestendorp]

• Laravel Version: 5.8.2
• PHP Version: 7.2.10
• Database Driver & Version: MySQL 5.7

Description:

session()->previousUrl() contains the current $request->fullUrl() instead of the real previous url.

Current situation is that \Illuminate\Session\Middleware\StartSession::storeCurrentUrl() is called before $next($request) is called, I would assume that the response should have been generated before storing the current url.

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

$this->addCookieToResponse(
    $response = $next($request), $session
);

Steps To Reproduce:

public function testPreviousUrl(): void
{
    session()->setPreviousUrl($previousUrl = 'https://www.example.com/foo/bar/');

    \Route::get('foo', function() {
        return response(session()->previousUrl());
    })->middleware('web');

    $this->get('foo')->assertSeeText($previousUrl);
}

Generates

1) PreviousUrlTest::testPreviousUrl
   ✖ Failed asserting that 'https://***/foo' contains "https://www.example.com/foo/bar/".

[driesvints]

The way it works currently is correct. The problem with your example is that in context of a test you're switching the url of the current session of a test instead of the session when the route is called.

This works:

Route::get('/', function () {
    session()->setPreviousUrl($previousUrl = 'https://www.example.com/foo/bar/');

    return response(session()->previousUrl());
});

[tswestendorp]

@driesvints, you're right, I setup my test wrong. But how about when the session already contains a previous url like the example given below.

public function testPreviousUrl(): void
{
    \Route::get('foo', function() {
        return response(back()->getTargetUrl().' -- '.session()->get('bar'));
    })->middleware(\Illuminate\Session\Middleware\StartSession::class);

    $this->withSession([
        'bar' => $baz = 'baz',
        '_previous.url' => $previousUrl = 'https://www.example.com/foo/bar/',
    ])->get('foo')
        ->assertSeeText($previousUrl)
        ->assertSeeText($baz);
}

Here the previous url is set by some previous request and when calling back() we should return to that previous url (right?), so the target url should be the url of the previous request.

1) Tests\PreviousUrlTest::testPreviousUrl
   ✖ Failed asserting that 'https://www.example.com/foo -- baz' contains "https://www.example.com/foo/bar/".

[driesvints]

@tswestendorp you shouldn't perform two route calls in the same test. Use separate test methods.

[okdewit]

@driesvints There are some issue when using laravel browserkit tests though:

Given some controller route with a return redirect()->back():

<?php
public function archive($model)
{
    $model->archive();
    return redirect()->back();
}

And a browserkit test:

<?php
$this->actingAs($user)
    ->visit('model.show')
    ->click('archive')
    ->see(...)

You run into an infinite redirect on Laravel 5.8, rapidly filling up your storage/framework/session directory with session files.

I do think short test methods is a great "best practice" advise, but when using some functional testing to tie important things together you easily run into problems with this change.

The whole purpose of the ->click('button') method in browserkit tests is to visit two or more routes in one test.

[driesvints]

@okdewit can you create an issue on the browser kit testing repo with a full example of how this breaks?

[okdewit]

My issue was fixed by #27935 in Laravel 5.8.5 👍