NotificationsYou must be signed in to change notification settings
Fork9.7k
Star30.8k

[WebProfilerBundle] Fix intercept external redirects#52584

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

fabpot merged 1 commit intosymfony:6.3fromHeahDude:fix/intercept-external-redirects

Dec 9, 2023

Merged

[WebProfilerBundle] Fix intercept external redirects#52584

fabpot merged 1 commit intosymfony:6.3fromHeahDude:fix/intercept-external-redirects

Dec 9, 2023

Conversation

Copy link

Contributor

HeahDude commentedNov 14, 2023

Q	A
Branch?	6.3
Bug fix?	yes
New feature?	no
Deprecations?	no
Issues	~
License	MIT

When intercepting a redirect to an external host the current output gives:

This request redirects tohttp://current-host.orghttp://target-host.org/

With this PR, it fixes it too:

This request redirects tohttp://target-host.org/

We could eventually get rid of the first part of the condition, WDYT?

carsonbot added Status: Needs Review Bug WebProfilerBundle labels

Nov 14, 2023

carsonbot added this to the6.3 milestone

Nov 14, 2023

nicolas-grekas reviewed

Nov 17, 2023

View reviewed changes

src/Symfony/Bundle/WebProfilerBundle/Resources/views/Profiler/toolbar_redirect.html.twig OutdatedShow resolvedHide resolved

Copy link

Member

smnandre commentedNov 19, 2023

Maybe we could do this computation in the Controller directly ?

Currently the code look like this (just formatted for clarity)

WebProfilerBundle/EventListener/WebDebugToolbarListener.php

$content =$this->twig->render('@WebProfiler/Profiler/toolbar_redirect.html.twig', ['location' =>$response->headers->get('Location'),'host' =>$request->getSchemeAndHttpHost()]);$response->setContent($content);$response->setStatusCode(200);$response->headers->remove('Location');

Something like this could do the job WDYT ?

$content =$this->twig->render('@WebProfiler/Profiler/toolbar_redirect.html.twig', ['location' =>$location =$response->headers->get('Location'),'absolute_location' => (newUrlHelper($requestStack))->getAbsoluteUrl($location),]);$response->setContent($content);$response->setStatusCode(200);$response->headers->remove('Location');

And that'd allow to keep in one place all those 'Location' manipulation / transformations.

Because, even with the best intentions, i'm not sure the template code would be ever "obvious" (e g :host in the template is in fact "scheme + host" but notapp.request.host )

WDYT ?