Skip to content
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

IBX-1218: Fixed a Symfony\Component\HttpFoundation::getSession() call… #240

Open
wants to merge 1 commit into
base: 1.3
Choose a base branch
from

Conversation

McLone
Copy link

@McLone McLone commented Oct 7, 2021

… without checking its existence with hasSession()

Question Answer
JIRA issue IBX-1218
Type bug
Target Ibexa version v3.3
BC breaks no

This PR fix a thrown exception (\Symfony\Component\HttpFoundation\Exception\SessionNotFoundException) if no session is available in the request.

Checklist:

  • Provided PR description.
  • Tested the solution manually.
  • Provided automated test coverage.
  • Checked that target branch is set correctly (master for features, the oldest supported for bugs).
  • Ran PHP CS Fixer for new PHP code (use $ composer fix-cs).
  • Asked for a review (ping @ezsystems/engineering-team).

… without checking its existence with hasSession()
@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 7, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@adamwojs
Copy link
Member

Hi @McLone! Could you please provide some steps to reproduce this issue?

@McLone
Copy link
Author

McLone commented Oct 12, 2021

Hi @adamwojs!
I'd love to, but it would be hard for me to make a detailed step at this point.

I fell on it when I tried to apply a "base functional tests" based on Symfony\Bundle\FrameworkBundle\Test\WebTestCase on a new project based on OSS 3.3.
This "base functional tests" is used in another OSS 3.3 project without problem, even without this fix.
So, for now, I don't really know how to consistently reproduce this "no session context".

IMHO the problem was obvious enough not to investigate further, but I'll try to provide reproduction steps in, I hope, a not too distant future.

@Steveb-p
Copy link
Contributor

Steveb-p commented Oct 12, 2021

@McLone actually, I've found something similar when I was designing IbexaTestKernel.

If router / security is not configured, then a lot of services related to session & request contexts do not exist. So that's probably the cause.

So technically this comes from incomplete configuration, which might happen in test environment.

@McLone
Copy link
Author

McLone commented Oct 14, 2021

Incomplete configuration indeed !
I found out that I encountered the crash because \eZ\Bundle\EzPublishCoreBundle\EventListener\SessionInitByPostListener is called in a sub-request, after the main request has thrown a NotFoundHttpException by \eZ\Bundle\EzPublishCoreBundle\EventListener\RejectExplicitFrontControllerRequestsListener. As sub request does not receive session factory by \Symfony\Component\HttpKernel\EventListener\AbstractSessionListener, there's no session for SessionInitByPostListener, therefore the SessionNotFoundException.

To avoid this in a Symfony\Bundle\FrameworkBundle\Test\WebTestCase context, you just have to set 'SCRIPT_FILENAME' => 'app.php' in the "server" param of WebTestCase::createClient().

@adamwojs As ezplatform-kernel do not use WebTestCase for its tests, it would be hard for me to provide you with a simple and clean detailed reproduction procedure. Sorry :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants