[Security] Fix for "Call to a member function getBaseUrl() on null" when generating a logout URL and there is no current request #27175

maartendekeizer · 2018-05-07T08:52:44Z

Q	A
Branch?	2.7 (to be switched when merging)
Bug fix?	yes
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	#27174
License	MIT
Doc PR	n/a

Adds a check if the request exists.

generating a logout URL and there is no current request

xabbuh · 2018-05-07T09:00:43Z

src/Symfony/Component/Security/Http/Logout/LogoutUrlGenerator.php


            $request = $this->requestStack->getCurrentRequest();

+            if (!$request) {


How can there be no request at this stage? Could you create a small example application that allows to reproduce?

After $response = $kernel->handle($request); in the front-controller Symfony\Component\HttpKernel\HttpKernel::finishRequest is executed and pops the request from the requestStack. The requestStack is now empty.
Events and other code that is executed after $kernel->handle (like terminate events) will find the empty requestStack, which is correct, there is no request anymore (response is already send).

That's right, but why is the data collector triggered at this stage at all? Collection data should happen earlier during the kernel.response event.

Finally, I found the problem why the data collection is triggered.
During the kernel.terminate the thrown exception is caught and forwarded to Symfony\Component\HttpKernel\HttpKernel::handleException, just like other exceptions in other phases of the request lifecycle.
The Symfony\Component\HttpKernel\EventListener\ExceptionListener dispatches a kernel.exception event, this event is listened by Symfony\Component\HttpKernel\EventListener\ExceptionListener.
Symfony\Component\HttpKernel\EventListener\ExceptionListener will start a new sub-request, all the normal kernel events are dispatched including kernel.response.
At this moment the data collection is triggered, Symfony\Component\HttpKernel\EventListener\ProfilerListener listens to the kernel.response event and starts the collection.

When the subrequest is handled, it will be pushed onto the stack which therefore shouldn't be empty. So I still think we should first look into an example application that allows to reproduce the issue and see if there isn't another root cause.

@maartendekeizer would you be able to provide a reproducer we could play with to see how this can arise?

I can only reproduce it on IIS 10, used PHP 7.1.5 and PHP 7.1.18. Theire is no issue when I use Apache2 with PHP 7.1.18 or Nginx with PHP 7.1.18.
https://github.com/maartendekeizer/symfony-demo-for-27175
After checkout and run composer install, visit the page /secure, login with the button. Change src/EventListener/TestWithFailureSubscriber.php

- //$a->doSomeThing(); // uncomment this line to create an error + $a->doSomeThing(); // uncomment this line to create an error

Refresh the /secure page

I cannot reproduce this behaviour with the built-in web server neither. Can you try to debug where the actual difference is when using IIS?

ogizanagi

It should target the 2.7 branch as it seems to me it is the lowest branch this issue is likely to happen.

xabbuh · 2018-05-29T10:28:40Z

Can you extract the stack trace of the failure so we can see where things go possibly wrong?

maartendekeizer · 2018-05-29T11:47:31Z

I add the following to LogoutUrlGenerator if ($request === null) { xdebug_print_function_stack();exit; }
this results in the following stack dump

Xdebug: user triggered in F:\workspace\symfony-demo-for-27175\vendor\symfony\security\Http\Logout\LogoutUrlGenerator.php on line 115
--
1 | 2.8394 | 12355296 | Symfony\Component\Debug\ErrorHandler->handleException( ???, ??? ) | ...\ErrorHandler.php:0
2 | 2.8438 | 12441312 | Symfony\Component\Debug\ExceptionHandler->handle( ??? ) | ...\ErrorHandler.php:556
3 | 2.8459 | 12517016 | call_user_func:{F:\workspace\symfony-demo-for-27175\vendor\symfony\debug\ExceptionHandler.php:145} ( ???, ??? ) | ...\ExceptionHandler.php:145
4 | 2.8459 | 12517016 | Symfony\Component\HttpKernel\EventListener\DebugHandlersListener->Symfony\Component\HttpKernel\EventListener\{closure}( ??? ) | ...\ExceptionHandler.php:145
5 | 2.8459 | 12517016 | Symfony\Component\HttpKernel\HttpKernel->terminateWithException( ???, ??? ) | ...\DebugHandlersListener.php:113
6 | 2.8459 | 12517016 | Symfony\Component\HttpKernel\HttpKernel->handleException( ???, ???, ??? ) | ...\HttpKernel.php:98
7 | 3.0033 | 13726848 | Symfony\Component\HttpKernel\HttpKernel->filterResponse( ???, ???, ??? ) | ...\HttpKernel.php:245
8 | 3.0033 | 13726976 | Symfony\Component\HttpKernel\Debug\TraceableEventDispatcher->dispatch( ???, ??? ) | ...\HttpKernel.php:187
9 | 3.0042 | 13730920 | Symfony\Component\EventDispatcher\EventDispatcher->dispatch( ???, ??? ) | ...\TraceableEventDispatcher.php:139
10 | 3.0043 | 13731616 | Symfony\Component\EventDispatcher\EventDispatcher->doDispatch( ???, ???, ??? ) | ...\EventDispatcher.php:44
11 | 3.0049 | 13732152 | Symfony\Component\EventDispatcher\Debug\WrappedListener->__invoke( ???, ???, ??? ) | ...\EventDispatcher.php:212
12 | 3.0049 | 13732152 | call_user_func:{F:\workspace\symfony-demo-for-27175\vendor\symfony\event-dispatcher\Debug\WrappedListener.php:104} ( ???, ???, ???, ??? ) | ...\WrappedListener.php:104
13 | 3.0049 | 13732152 | Symfony\Component\HttpKernel\EventListener\ProfilerListener->onKernelResponse( ???, ???, ??? ) | ...\WrappedListener.php:104
14 | 3.0049 | 13732152 | Symfony\Component\HttpKernel\Profiler\Profiler->collect( ???, ???, ??? ) | ...\ProfilerListener.php:91
15 | 3.0057 | 13742056 | Symfony\Bundle\SecurityBundle\DataCollector\SecurityDataCollector->collect( ???, ???, ??? ) | ...\Profiler.php:162
16 | 3.0057 | 13742056 | Symfony\Component\Security\Http\Logout\LogoutUrlGenerator->getLogoutPath( ??? ) | ...\SecurityDataCollector.php:115
17 | 3.0057 | 13742056 | Symfony\Component\Security\Http\Logout\LogoutUrlGenerator->generateLogoutUrl( ???, ??? ) | ...\LogoutUrlGenerator.php:65
18 | 8.3672 | 13757888 | xdebug_print_function_stack (  )

xabbuh · 2018-07-06T08:55:26Z

@maartendekeizer I am going to close here as I am convinced that this is not the right solution to your problem. If you manage to debug where there is the behaviour difference between Nginx/the built-in web server and IIS, please provide more details in #27174 and I happily take a look at it. Thank you for understanding.

[Security] Fix for "Call to a member function getBaseUrl() on null" when

adfd09a

generating a logout URL and there is no current request

carsonbot added Status: Needs Review Security Bug labels May 7, 2018

xabbuh reviewed May 7, 2018

View reviewed changes

ogizanagi reviewed May 7, 2018

View reviewed changes

nicolas-grekas added this to the 2.7 milestone May 7, 2018

fabpot modified the milestones: 2.7, 2.8 May 28, 2018

xabbuh closed this Jul 6, 2018

krzyc mentioned this pull request Sep 14, 2022

security-http (LogoutUrlGenerator) error #47577

Closed


		$request = $this->requestStack->getCurrentRequest();

		if (!$request) {

Uh oh!

[Security] Fix for "Call to a member function getBaseUrl() on null" when generating a logout URL and there is no current request #27175

[Security] Fix for "Call to a member function getBaseUrl() on null" when generating a logout URL and there is no current request #27175

Uh oh!

Conversation

maartendekeizer commented May 7, 2018 • edited by nicolas-grekas Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ogizanagi left a comment

Choose a reason for hiding this comment

Uh oh!

xabbuh commented May 29, 2018

Uh oh!

maartendekeizer commented May 29, 2018

Uh oh!

xabbuh commented Jul 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

maartendekeizer commented May 7, 2018 •

edited by nicolas-grekas

Loading