security-http (LogoutUrlGenerator) error #47577

krzyc · 2022-09-14T16:32:56Z

Symfony version(s) affected

4.4.44

Description

Method generateLogoutUrl of LogoutUrlGenerator throws "Call to a member function getBaseUrl() on null" error.
It can be reproduced by generating error after authenticating as a user. Error is not present after logging out.
Similar issue is described on #27174
On Symfony 6.1 this problem does not occur on same setup.
Tested using PHP 8.1 (Ubuntu supplied) with built in server.

How to reproduce

Install symfony/website-skeleton:"^4.4" with dev environment
Add db based authentication
Login
Generate error

Possible Solution

Checking for null request in LogoutUrlGenerator - there was previously supplied PR which fixes this problem:
#27175
It was rejected because problem was not reproduced (or should not exist).
But I think that in any case if called function can return null then it should be checked against null before calling its methods.

Additional Context

Call to a member function getBaseUrl() on null
Exception Stack Trace
Error
in vendor/symfony/security-http/Logout/LogoutUrlGenerator.php (line 110)
in vendor/symfony/security-http/Logout/LogoutUrlGenerator.php -> generateLogoutUrl (line 64)
in vendor/symfony/security-bundle/DataCollector/SecurityDataCollector.php -> getLogoutPath (line 136)
in vendor/symfony/http-kernel/Profiler/Profiler.php -> collect (line 178)
in vendor/symfony/http-kernel/EventListener/ProfilerListener.php -> collect (line 100)
in vendor/symfony/event-dispatcher/Debug/WrappedListener.php -> onKernelResponse (line 126)
in vendor/symfony/event-dispatcher/EventDispatcher.php -> __invoke (line 264)
in vendor/symfony/event-dispatcher/EventDispatcher.php -> doDispatch (line 239)
in vendor/symfony/event-dispatcher/EventDispatcher.php -> callListeners (line 73)
in vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php -> dispatch (line 168)
in vendor/symfony/http-kernel/HttpKernel.php -> dispatch (line 192)
in vendor/symfony/http-kernel/HttpKernel.php -> filterResponse (line 245)
in vendor/symfony/http-kernel/HttpKernel.php -> handleThrowable (line 115)
in vendor/symfony/http-kernel/EventListener/DebugHandlersListener.php -> terminateWithException (line 129)
in vendor/symfony/error-handler/ErrorHandler.php :: Symfony\Component\HttpKernel\EventListener{closure} (line 601)
ErrorHandler->handleException()

xabbuh · 2022-09-14T17:48:54Z

Can you create a small example application that allows to reproduce your issue?

krzyc · 2022-09-14T19:59:43Z

@xabbuh I have created minimal app to show issue:
https://github.com/krzyc/symfony-issue-47577
Commited with SQLite db - username: admin password: admin

specky-rum · 2022-09-14T21:57:42Z

Hi, fwiw we're getting this error too. I'm getting the exact same error trace as the OP whenever there is a 500 error.

Some observations:

It only happens on our development version, not on production. I presume this is because we're not displaying the error pages on production. They are reported correctly to our error log though.
When this happens on our development version the original error is not logged.
It has been happening since we updated Symfony from 4.4.40 to 4.4.44.

krzyc · 2022-09-15T06:53:31Z

@specky-rum Yes, this error happens in process of collecting data for displaying actual error in dev (Web Profiler) environment. SecurityDataCollector tries to get logout URL, but generator throws error because of no request in RequestStack. So there are two problems: why there is no request in RequestStack at this point (some time ago this problem did not exist so maybe it was introduced in some latest versions of Symfony), and why generateLogoutUrl is not checking for null value which is legal return value for getCurrentRequest so it should be handled in my opinion - then SecurityDataCollector will silently fetch thrown Exception as described in comments (fail silently when the logout URL cannot be generated), but only Exceptions are catched, not errors.

fnagel · 2022-09-15T16:30:37Z

Can confirm this issue with symfony/symfony 4.4.45 too.

mfreimann · 2022-10-01T23:27:29Z

I too am experiencing the problem whenever an exception occurs.

wouterj · 2022-10-02T11:52:08Z

Thanks to @krzyc's reproducer and git-bisect, I've been able to track this down to this change: #47358

This has been identified in another profiler related issue as well: #47405 (comment)

Padam87 · 2022-10-03T00:57:58Z

Also having this issue with 6.1.5

6.1.3 works, thanks @wouterj

    "extra": {
        "symfony": {
            "allow-contrib": false,
            "require": "<=6.1.3"
        }
    }

dehy · 2022-10-03T09:53:12Z

Having this exception in 5.4.13

Error:
Call to a member function getBaseUrl() on null

  at /home/adcnx/app-vendor/symfony/security-http/Logout/LogoutUrlGenerator.php:104
  at Symfony\Component\Security\Http\Logout\LogoutUrlGenerator->generateLogoutUrl()
     (/home/adcnx/app-vendor/symfony/security-http/Logout/LogoutUrlGenerator.php:65)
  at Symfony\Component\Security\Http\Logout\LogoutUrlGenerator->getLogoutPath()
     (/home/adcnx/app-vendor/symfony/security-bundle/DataCollector/SecurityDataCollector.php:122)
  at Symfony\Bundle\SecurityBundle\DataCollector\SecurityDataCollector->collect()
     (/home/adcnx/app-vendor/symfony/http-kernel/Profiler/Profiler.php:161)
  at Symfony\Component\HttpKernel\Profiler\Profiler->collect()
     (/home/adcnx/app-vendor/symfony/http-kernel/EventListener/ProfilerListener.php:108)
  at Symfony\Component\HttpKernel\EventListener\ProfilerListener->onKernelResponse()
     (/home/adcnx/app-vendor/symfony/event-dispatcher/Debug/WrappedListener.php:117)
  at Symfony\Component\EventDispatcher\Debug\WrappedListener->__invoke()
     (/home/adcnx/app-vendor/symfony/event-dispatcher/EventDispatcher.php:230)
  at Symfony\Component\EventDispatcher\EventDispatcher->callListeners()
     (/home/adcnx/app-vendor/symfony/event-dispatcher/EventDispatcher.php:59)
  at Symfony\Component\EventDispatcher\EventDispatcher->dispatch()
     (/home/adcnx/app-vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php:154)
  at Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher->dispatch()
     (/home/adcnx/app-vendor/symfony/http-kernel/HttpKernel.php:186)
  at Symfony\Component\HttpKernel\HttpKernel->filterResponse()
     (/home/adcnx/app-vendor/symfony/http-kernel/HttpKernel.php:239)
  at Symfony\Component\HttpKernel\HttpKernel->handleThrowable()
     (/home/adcnx/app-vendor/symfony/http-kernel/HttpKernel.php:109)
  at Symfony\Component\HttpKernel\HttpKernel->terminateWithException()
     (/home/adcnx/app-vendor/symfony/http-kernel/EventListener/DebugHandlersListener.php:131)
  at Symfony\Component\HttpKernel\EventListener\DebugHandlersListener::Symfony\Component\HttpKernel\EventListener\{closure}()
     (/home/adcnx/app-vendor/symfony/error-handler/ErrorHandler.php:607)
  at Symfony\Component\ErrorHandler\ErrorHandler->handleException()

dehy · 2022-10-04T15:32:06Z

On 6.0.13

Error:
Call to a member function getBaseUrl() on null

  at /home/adcnx/app-vendor/symfony/security-http/Logout/LogoutUrlGenerator.php:97
  at Symfony\Component\Security\Http\Logout\LogoutUrlGenerator->generateLogoutUrl()
     (/home/adcnx/app-vendor/symfony/security-http/Logout/LogoutUrlGenerator.php:60)
  at Symfony\Component\Security\Http\Logout\LogoutUrlGenerator->getLogoutPath()
     (/home/adcnx/app-vendor/symfony/security-bundle/DataCollector/SecurityDataCollector.php:114)
  at Symfony\Bundle\SecurityBundle\DataCollector\SecurityDataCollector->collect()
     (/home/adcnx/app-vendor/symfony/http-kernel/Profiler/Profiler.php:151)
  at Symfony\Component\HttpKernel\Profiler\Profiler->collect()
     (/home/adcnx/app-vendor/symfony/http-kernel/EventListener/ProfilerListener.php:108)
  at Symfony\Component\HttpKernel\EventListener\ProfilerListener->onKernelResponse()
     (/home/adcnx/app-vendor/symfony/event-dispatcher/Debug/WrappedListener.php:111)
  at Symfony\Component\EventDispatcher\Debug\WrappedListener->__invoke()
     (/home/adcnx/app-vendor/symfony/event-dispatcher/EventDispatcher.php:230)
  at Symfony\Component\EventDispatcher\EventDispatcher->callListeners()
     (/home/adcnx/app-vendor/symfony/event-dispatcher/EventDispatcher.php:59)
  at Symfony\Component\EventDispatcher\EventDispatcher->dispatch()
     (/home/adcnx/app-vendor/symfony/event-dispatcher/Debug/TraceableEventDispatcher.php:152)
  at Symfony\Component\EventDispatcher\Debug\TraceableEventDispatcher->dispatch()
     (/home/adcnx/app-vendor/symfony/http-kernel/HttpKernel.php:186)
  at Symfony\Component\HttpKernel\HttpKernel->filterResponse()
     (/home/adcnx/app-vendor/symfony/http-kernel/HttpKernel.php:239)
  at Symfony\Component\HttpKernel\HttpKernel->handleThrowable()
     (/home/adcnx/app-vendor/symfony/http-kernel/HttpKernel.php:109)
  at Symfony\Component\HttpKernel\HttpKernel->terminateWithException()
     (/home/adcnx/app-vendor/symfony/http-kernel/EventListener/DebugHandlersListener.php:125)
  at Symfony\Component\HttpKernel\EventListener\DebugHandlersListener::Symfony\Component\HttpKernel\EventListener\{closure}()
     (/home/adcnx/app-vendor/symfony/error-handler/ErrorHandler.php:541)
  at Symfony\Component\ErrorHandler\ErrorHandler->handleException()
     (/home/adcnx/app-vendor/sentry/sentry/src/ErrorHandler.php:357)
  at Sentry\ErrorHandler->handleException()

SimonVanacco · 2022-10-06T12:15:42Z

For those on the 5.4.x branch, restricting to 5.4.12 works !

  "extra": {
        "symfony": {
            "allow-contrib": false,
            "require": "<=5.4.12"
        }
    }

(thanks @Padam87 for the idea)

wouterj · 2022-10-06T13:11:23Z

Great to see so many people have found this issue, it means we have many candidates to work on a fix :)

In the other issue, a more in-depth search has been done and the root cause has been found: #47405 (comment) (the root cause being that there is no request while collecting data).

Anyone up for a PR to fix the issue?

andyexeter · 2022-10-12T08:43:57Z

@wouterj the SecurityDataCollector already wraps the call to getLogoutPath in a try/catch for any Exception:

symfony/src/Symfony/Bundle/SecurityBundle/DataCollector/SecurityDataCollector.php

Lines 108 to 113 in 8713873

    
           $logoutUrl = null; 
        
           try { 
        
               $logoutUrl = $this->logoutUrlGenerator?->getLogoutPath(); 
        
           } catch (\Exception) { 
        
               // fail silently when the logout URL cannot be generated 
        
           }

Changing this to catch any Throwable resolves the issue for me:

$logoutUrl = null;
try {
    $logoutUrl = $this->logoutUrlGenerator?->getLogoutPath();
} catch (\Throwable) {
    // fail silently when the logout URL cannot be generated
}

Would this suffice as a PR if I submitted it?

krzyc · 2022-10-12T09:19:32Z

@wouterj Personally I think that generating proper exception is cleaner then intercepting error - there is a PR already submitted: https://github.com/symfony/symfony/pull/27175/files
For me not checking for null value, which is legal return value (even if it should not be null at that time), is not correct independently from a problem why there is no request - but this PR was rejected.
@xabbuh What do you think about it? Why we should not check for null request value in generateLogoutUrl? Throwing LogicException looks good to me.

xabbuh · 2022-10-12T09:40:19Z

Adding the null check works as a first step to me. But we should IMO still fix the underlying issue as currently if I understand this correctly the collector cannot collect all data that it is supposed to be able to collect.

curry684 · 2022-10-13T09:41:53Z

For those that, like me, end up in a situation where this is hiding exceptions but you cannot downgrade without breaking other features, just add some code directly in vendor/symfony/security-http/Logout/LogoutUrlGenerator at line 97 (for 6.1.5):

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

            // These lines fix it until your next Composer update
            if (!$request) {
                return 'nobody cares';
            }
            $url = UrlGeneratorInterface::ABSOLUTE_URL === $referenceType ? $request->getUriForPath($logoutPath) : $request->getBaseUrl().$logoutPath;

krzyc · 2022-10-13T15:07:34Z

Adding the null check works as a first step to me. But we should IMO still fix the underlying issue as currently if I understand this correctly the collector cannot collect all data that it is supposed to be able to collect.

This problem originates in terminateWithException of HttpKernel. Pushing request to requestStack before calling handleThrowable (https://github.com/symfony/http-kernel/blob/6.1/HttpKernel.php#L109) in terminateWithException function fixes problem. @xabbuh Do you think this is correct solution? (I have submitted a PR). It is called by exceptionHandler from DebugHandlersListener.

Before cited commit #47358 requestStack was not empty because it was popped in finishRequest.

… exception (krzyc) This PR was squashed before being merged into the 4.4 branch. Discussion ---------- [HttpKernel] Fix empty request stack when terminating with exception | Q | A | ------------- | --- | Branch? | 4.4, 5.4, 6.0, 6.1, 6.2 | Bug fix? | yes | New feature? | no | Deprecations? | no | Tickets | Fix #47577 | License | MIT | Doc PR | After #47358 the RequestStack is empty when request terminates with exception, which prevents SecurityDataCollector to generate logout URL and generates fatal error. Commits ------- e4d6e7b [HttpKernel] Fix empty request stack when terminating with exception

fnagel · 2022-10-21T07:53:01Z

For those on the 4.4.x branch, restricting to 4.4.44 works!

  "extra": {
        "symfony": {
            "allow-contrib": false,
            "require": "<=4.4.44"
        }
    }

krzyc added the Bug label Sep 14, 2022

carsonbot added the Status: Needs Review label Sep 14, 2022

xabbuh added Security Status: Waiting feedback labels Sep 14, 2022

xabbuh removed the Status: Waiting feedback label Sep 17, 2022

wouterj mentioned this issue Oct 2, 2022

CSP is not removed for exception pages in 6.1.4 #47405

Closed

wouterj mentioned this issue Oct 11, 2022

Logout Url Generator does not check if the request exists #47838

Closed

krzyc mentioned this issue Oct 13, 2022

[HttpKernel] Fix empty request stack when terminating with exception #47857

Merged

nicolas-grekas closed this as completed Oct 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

security-http (LogoutUrlGenerator) error #47577

security-http (LogoutUrlGenerator) error #47577

krzyc commented Sep 14, 2022 •

edited

xabbuh commented Sep 14, 2022

krzyc commented Sep 14, 2022 •

edited

specky-rum commented Sep 14, 2022

krzyc commented Sep 15, 2022 •

edited

fnagel commented Sep 15, 2022 •

edited

mfreimann commented Oct 1, 2022

wouterj commented Oct 2, 2022 •

edited

Padam87 commented Oct 3, 2022

dehy commented Oct 3, 2022

dehy commented Oct 4, 2022

SimonVanacco commented Oct 6, 2022

wouterj commented Oct 6, 2022 •

edited

andyexeter commented Oct 12, 2022 •

edited

krzyc commented Oct 12, 2022

xabbuh commented Oct 12, 2022

curry684 commented Oct 13, 2022

krzyc commented Oct 13, 2022 •

edited

fnagel commented Oct 21, 2022

security-http (LogoutUrlGenerator) error #47577

security-http (LogoutUrlGenerator) error #47577

Comments

krzyc commented Sep 14, 2022 • edited

Symfony version(s) affected

Description

How to reproduce

Possible Solution

Additional Context

xabbuh commented Sep 14, 2022

krzyc commented Sep 14, 2022 • edited

specky-rum commented Sep 14, 2022

krzyc commented Sep 15, 2022 • edited

fnagel commented Sep 15, 2022 • edited

mfreimann commented Oct 1, 2022

wouterj commented Oct 2, 2022 • edited

Padam87 commented Oct 3, 2022

dehy commented Oct 3, 2022

dehy commented Oct 4, 2022

SimonVanacco commented Oct 6, 2022

wouterj commented Oct 6, 2022 • edited

andyexeter commented Oct 12, 2022 • edited

krzyc commented Oct 12, 2022

xabbuh commented Oct 12, 2022

curry684 commented Oct 13, 2022

krzyc commented Oct 13, 2022 • edited

fnagel commented Oct 21, 2022

krzyc commented Sep 14, 2022 •

edited

krzyc commented Sep 14, 2022 •

edited

krzyc commented Sep 15, 2022 •

edited

fnagel commented Sep 15, 2022 •

edited

wouterj commented Oct 2, 2022 •

edited

wouterj commented Oct 6, 2022 •

edited

andyexeter commented Oct 12, 2022 •

edited

krzyc commented Oct 13, 2022 •

edited