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.

Sign up for GitHub

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

Jump to bottom

IBX-6207: Requests containing front controller script causes session-not-found exception #377

Merged
merged 4 commits into from
Nov 3, 2023
Merged

IBX-6207: Requests containing front controller script causes session-not-found exception #377

merged 4 commits into from
Nov 3, 2023

Conversation

vidarl
Copy link
Member

@vidarl vidarl commented Jul 20, 2023 •
edited
Loading

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

Provide the front controller script in the url ( ie http://localhost/index.php or http://localhost/foobar/index.php ) will cause SessionNotFoundException exception.

Expected behavior : RejectExplicitFrontControllerRequestsListener should kick in ensure a 404 is returned without filling up logs.

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).

@vidarl vidarl requested a review from a team July 20, 2023 14:16
@vidarl vidarl requested a review from Steveb-p July 26, 2023 07:29
alongosz
alongosz reviewed Jul 28, 2023
View reviewed changes
Copy link
Member

@alongosz alongosz left a comment

Choose a reason for hiding this comment

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

@vidarl Please add test coverage for that use case to \eZ\Bundle\EzPublishCoreBundle\Tests\EventListener\SessionInitByPostListenerTest

eZ/Bundle/EzPublishCoreBundle/EventListener/SessionInitByPostListener.php Outdated Show resolved Hide resolved
@vidarl
Copy link
Member Author

vidarl commented Aug 3, 2023 •
edited
Loading

@vidarl Please add test coverage for that use case to \eZ\Bundle\EzPublishCoreBundle\Tests\EventListener\SessionInitByPostListenerTest

@alongosz : Added simple test in 5c91a54. Without the fix, Symfony\Component\HttpFoundation\Exception\SessionNotFoundException will be thrown and test will fail

edit : wrong hash in initial comment....

@vidarl vidarl requested a review from alongosz August 3, 2023 09:50
@vidarl
Copy link
Member Author

vidarl commented Sep 5, 2023

@alongosz , @Steveb-p : Review ping

@sonarcloud
Copy link

sonarcloud bot commented Sep 6, 2023

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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@Steveb-p Steveb-p added Bug Something isn't working Ready for review labels Oct 13, 2023
@konradoboza konradoboza requested a review from a team October 13, 2023 10:27
@alongosz alongosz requested a review from a team October 13, 2023 14:49
@micszo micszo self-assigned this Oct 25, 2023
@micszo micszo mentioned this pull request Oct 25, 2023
Closed
@micszo
Copy link
Member

micszo commented Oct 25, 2023

Hi @vidarl ! Could you please rebase this PR? 😊

vidarl and others added 4 commits October 26, 2023 11:27
@vidarl
IBX-6207: Requests containing front controller script causes session-…
1f679c0 
…not-found exception
@vidarl @Steveb-p
Update eZ/Bundle/EzPublishCoreBundle/EventListener/SessionInitByPostL…
79ff02d 
…istener.php

Co-authored-by: Paweł Niedzielski <pawel.niedzielski@ibexa.co>
@vidarl
fixup! IBX-6207: Requests containing front controller script causes s…
0c9343f 
…ession-not-found exception
@vidarl @alongosz
Apply suggestions from code review
5d5a580 
Co-authored-by: Andrew Longosz <alongosz@users.noreply.github.com>
@vidarl vidarl force-pushed the ibx-6207_requests_containing_frontcontroller_script_causes_session_not_found_exception branch from 4e1746f to 5d5a580 Compare October 26, 2023 09:28
@vidarl
Copy link
Member Author

vidarl commented Oct 26, 2023

@micszo : Just rebased to latest 1.3

@sonarcloud
Copy link

sonarcloud bot commented Oct 26, 2023

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 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@micszo
Copy link
Member

micszo commented Oct 26, 2023

@vidarl when the url with index.php in it is opened in the browser should this error be printed in log every second?

[2023-10-26T13:11:38.322836+02:00] request.ERROR: Uncaught PHP Exception Symfony\Component\HttpKernel\Exception\NotFoundHttpException: "" at /Users/michalszoltysek/Projects/workspace/ibexa_website_4/vendor/ezsystems/ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/EventListener/RejectExplicitFrontControllerRequestsListener.php line 44 {"exception":"[object] (Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException(code: 0):  at /Users/michalszoltysek/Projects/workspace/ibexa_website_4/vendor/ezsystems/ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/EventListener/RejectExplicitFrontControllerRequestsListener.php:44)"} []
[2023-10-26T13:11:39.721775+02:00] request.ERROR: Uncaught PHP Exception Symfony\Component\HttpKernel\Exception\NotFoundHttpException: "" at /Users/michalszoltysek/Projects/workspace/ibexa_website_4/vendor/ezsystems/ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/EventListener/RejectExplicitFrontControllerRequestsListener.php line 44 {"exception":"[object] (Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException(code: 0):  at /Users/michalszoltysek/Projects/workspace/ibexa_website_4/vendor/ezsystems/ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/EventListener/RejectExplicitFrontControllerRequestsListener.php:44)"} []
[2023-10-26T13:11:41.139665+02:00] request.ERROR: Uncaught PHP Exception Symfony\Component\HttpKernel\Exception\NotFoundHttpException: "" at /Users/michalszoltysek/Projects/workspace/ibexa_website_4/vendor/ezsystems/ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/EventListener/RejectExplicitFrontControllerRequestsListener.php line 44 {"exception":"[object] (Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException(code: 0):  at /Users/michalszoltysek/Projects/workspace/ibexa_website_4/vendor/ezsystems/ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/EventListener/RejectExplicitFrontControllerRequestsListener.php:44)"} []
[2023-10-26T13:11:42.538055+02:00] request.ERROR: Uncaught PHP Exception Symfony\Component\HttpKernel\Exception\NotFoundHttpException: "" at /Users/michalszoltysek/Projects/workspace/ibexa_website_4/vendor/ezsystems/ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/EventListener/RejectExplicitFrontControllerRequestsListener.php line 44 {"exception":"[object] (Symfony\\Component\\HttpKernel\\Exception\\NotFoundHttpException(code: 0):  at /Users/michalszoltysek/Projects/workspace/ibexa_website_4/vendor/ezsystems/ezplatform-kernel/eZ/Bundle/EzPublishCoreBundle/EventListener/RejectExplicitFrontControllerRequestsListener.php:44)"} []

@vidarl
Copy link
Member Author

vidarl commented Oct 27, 2023 •
edited
Loading

@vidarl when the url with index.php in it is opened in the browser should this error be printed in log every second?

@micszo : yes, in dev you'll get one exception in the logs ( half a dozen without patch )
In production you'll now get no log entry (besides the 404 of course )

micszo
micszo approved these changes Oct 27, 2023
View reviewed changes
Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Reproduced and retested. 400 response is replaced by 404. Log entries are reduced.

Tested on Ibexa Commerce 3.3.36-dev.

(CI has canceled and skipped jobs)

@micszo micszo removed their assignment Oct 27, 2023
@adamwojs adamwojs merged commit 0f4d6ab into 1.3 Nov 3, 2023
18 of 19 checks passed
@adamwojs adamwojs deleted the ibx-6207_requests_containing_frontcontroller_script_causes_session_not_found_exception branch November 3, 2023 07:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@konradoboza konradoboza konradoboza approved these changes

@micszo micszo micszo approved these changes

@alongosz alongosz alongosz approved these changes

@Steveb-p Steveb-p Steveb-p approved these changes

Assignees

@kisztof kisztof

@mikadamczyk mikadamczyk

@alongosz alongosz

Labels
Bug Something isn't working QA approved
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@vidarl @micszo @Steveb-p @alongosz @konradoboza @adamwojs @kisztof @mikadamczyk