Help understanding why $request->hasSession() doesn't work with API requests

[rodrigopedra]

Last week I opened PR #37842 to add the following if guard to the end of Illuminate\Routing\RedirectController:

if ($request->hasSession()) {
    $request->session()->reflash();
}

$request in this context is a Illuminate\Http\Request instance.

I assumed $request->hasSession() would prevent requests without sessions attached to it to fail when calling reflash(), but mr. @GrahamCampbell , after reviewing it added this comment:

👎 here. This will break APIs that don't have sessions.

Originally posted by @GrahamCampbell in #37842 (comment)

As mr. @GrahamCampbell work is outstanding, with contributions to so many projects, and also someone whose work I really admire, I was surprised to learn $request->hasSession() would not prevent an API request without a session attached to it to skip this if guard.

I tried asking on that issue's comment if he could help me understand why the if clause above is not sufficient to prevent any breakage on stateless requests, but I understand closed issues are hard to get any attention from core maintainers.

Also I totally understand that mr. @GrahamCampbell might have a very busy day helping out so many open-source projects - which is an effort I really appreciate - and working on StyleCI which is a tool I recommended a lot for peers and customers I worked with, to notice my requests for clarifications. Specially as I feel it is something silly I could be missing in front of my eyes.

I thought on opening an issue, as my previous understanding pointed me to believe that using $request->hasSession() would be sufficient to guard against the case mentioned by mr. @GrahamCampbell .

But as in the past few days I could not create any examples where using that if clause could break an API call, I thought the issue would be closed or directed to the forums.

So I decided to ask for help understanding why this would break an API request, or for examples. Please forgive me if this is something very obvious, but it is bogging me down in the past few days.

On almost 6 years working with Laravel, and participating actively on the Laracasts' forums, this is also my first time ever posting a request for help on a forum regarding Laravel. I am not very confident of my English skills. It takes me a lot of time and effort to write a piece of text like this. So I always tried searching for solutions on already answered questions, as I am afraid when questioning something I can't solve I would not be able to explain it properly. So please be patient and if something is not clear enough feel free to ask me for any clarifications. I will do my best to answer them.

Lastly, let me explain my motivation for opening PR #37842

I am working on a very nascent app as a pro-bono for a local church to manage patients on the musical therapy clinic they maintain. Patients are mostly young children with some cognition or health condition. The app is basically an appointment management app, but with added data, such as session reports, and patient history.

As we are working on one module at a time I added a redirect route like this:

Route::redirect('/', '/agenda')->name('home');

/agenda would be a week calendar view with the current week's appointments.

So far so good. But later when more modules are added, the home route should point to a /dashboard path that will be implemented later.

So when I implemented a "Change My Password" feature, to allow a logged in user to change their password I added this redirect at the end:

return redirect()->route('home')->with('success', 'Password changed');

The 'success' message would show a toast notification to the user.

But as I am redirecting to the home route, which is also a redirect, the double redirect ends missing the flash message as the session is not reflashed.

I redirect to home instead of redirecting to /agenda, to avoid fixing this later when the home route changes where it is redirecting.

That is I opened PR #37842 to reflash the session on Illuminate\Routing\RedirectController. And I added the guard to avoid conflicting with requests with no sessions attached, such as API requests.

I thought that would work, but for my surprise, mr. @GrahamCampbell pointed out using that if clause this would break API calls that have no sessions.

For this particular app, I ended up just hardcoding the redirect for now, an taking a note to fixing it later after we work on the other modules.

But what go me haunted is that I used $request->hasSession() previously on some other projects, on middleware that were shared between the web (stateful) and api (stateless) stacks.

I started being afraid some of those apps could break with use-cases I have not anticipated because my lack of knowledge.

So I started testing to find any way it could break, and searched a lot for a method that would allow me to add a similar if clause in such mixed cases. But I could not find any.

If you read until here, thank you for patience.

If you can help me understanding why using $request->hasSession() is not sufficient to prevent breaking API calls, or help me pointing out a better way to guard against stateless requests: thank you in advance.

I would also like to thank mr. @GrahamCampbell for reviewing PR #37842 and if he could help me understand this, that would be really awesome.

And lastly I wanted to thank mr. @driesvints that noticed my further comments after PR #37842 was closed. You do a great job on the managing Laravel issues, and have all my respect. I am a great fan of yours. Thank you.

Hope y'all have a blessed weekend.

P.S.:

By no means I am trying to reopen or get PR #37842 merged.

I am truly trying to understand mr. @GrahamCampbell comment and why using that if clause would break an API request, or a request without a session attached to it.

[driesvints]

I redirect to home instead of redirecting to /agenda, to avoid fixing this later when the home route changes where it is redirecting.

I think this is the actual problem. A redirect usually doesn't persists this kind of session data and has never had in Laravel I believe. You should target the correct route to which you want to redirect.

Your PR could also impose a breaking change. In some situations people might not expect certain session data to still be present after a redirect. There could be various use cases why this isn't wanted to I believe it isn't wise to change this behavior.

About Graham's remark: I do agree that the hasSession method would prevent the breakage so I don't think that's a valid remark.

[rodrigopedra]

Thanks @driesvints for your reply.

The only place I know it is used on the framework's code is this:

framework/src/Illuminate/Broadcasting/BroadcastController.php

Lines 19 to 21 in 108385b

	if ($request->hasSession()) {
	$request->session()->reflash();
	}

(I happen to have dealt with it recently when working with broadcast)

But I agree it is and edge case. Nevertheless thanks again for clarifying this to me.

Have a nice day =)