Skip to content

Profiler respect stateless attribute #50218

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

Merged
merged 1 commit into from
May 5, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,9 @@ public function toolbarAction(Request $request, string $token = null): Response
throw new NotFoundHttpException('The profiler must be enabled.');
}

if ($request->hasSession() && ($session = $request->getSession())->isStarted() && $session->getFlashBag() instanceof AutoExpireFlashBag) {
if (!$request->attributes->getBoolean('_stateless') && $request->hasSession()
&& ($session = $request->getSession())->isStarted() && $session->getFlashBag() instanceof AutoExpireFlashBag
) {
// keep current flashes for one more request if using AutoExpireFlashBag
$session->getFlashBag()->setAll($session->getFlashBag()->peekAll());
}
Expand Down Expand Up @@ -172,7 +174,11 @@ public function searchBarAction(Request $request): Response

$this->cspHandler?->disableCsp();

$session = $request->hasSession() ? $request->getSession() : null;

$session = null;
if ($request->attributes->getBoolean('_stateless') && $request->hasSession()) {
Copy link
Contributor

@themasch themasch Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know I am late, but I thought posting it here was less interruptive then creating a new issue or discussion for this question:

The PR touches 3 placed where it gets the session and adds checks for the _stateless request attribute.
Two checks check for !stateless && hasSession while one of them (in searchBarAction) checks for stateless && hasSession (without the !).

I found this while debugging for what starts Session was used while the request was declared stateless. when calling the profiler. As far as I can tell, the only place using the session there is the this spot.

This line looks incorret to me, I think its missing the ! in front. But since I'm probably wrong (since this is like this for a year now, with no one noticing) I'd like to learn why I am wrong!

Copy link
Contributor

@MatTheCat MatTheCat Jul 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested and you’re not wrong. Maybe you really are the first to notice 😅

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the implications of this correct, then that means that for users that have stateless: false, restoring the search bar state from session should not work, as it would not use the session in those cases. It would only use the session if stateless: true (e.g. when there should be no session). Correct?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for users that have stateless: false, restoring the search bar state from session should not work

Yes, that also includes users who did not set _stateless. Are you up for a PR? Looks like you already know the fix!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can give it a try: #57679
Hope thats correct so far.

$session = $request->getSession();
}

return new Response(
$this->twig->render('@WebProfiler/Profiler/search.html.twig', [
Expand Down Expand Up @@ -247,7 +253,7 @@ public function searchAction(Request $request): Response
$limit = $request->query->get('limit');
$token = $request->query->get('token');

if ($request->hasSession()) {
if (!$request->attributes->getBoolean('_stateless') && $request->hasSession()) {
$session = $request->getSession();

$session->set('_profiler_search_ip', $ip);
Expand Down