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

Ensure $request->hasSession() is always checked before calling getSession()#32703

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

nicolas-grekas merged 1 commit intosymfony:4.4fromArman-Hosseini:patch-6

Jul 30, 2019

Merged

Ensure $request->hasSession() is always checked before calling getSession()#32703

nicolas-grekas merged 1 commit intosymfony:4.4fromArman-Hosseini:patch-6

Jul 30, 2019

Conversation

Copy link

Contributor

Arman-Hosseini commentedJul 24, 2019•
edited by chalasr
Loading

Q	A
Branch?	4.4
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	-
License	MIT
Doc PR	-

carsonbot added Status: Needs Review Security Deprecation labels

Jul 24, 2019

nicolas-grekas added this to the3.4 milestone

Jul 24, 2019

Copy link

Member

nicolas-grekas commentedJul 24, 2019•
edited
Loading

Any reason why you're proposing this?

nicolas-grekas removed the Deprecation label

Jul 24, 2019

Copy link

ContributorAuthor

Arman-Hosseini commentedJul 24, 2019

@nicolas-grekas
Just improving the codes.

Copy link

Member

fabpot commentedJul 27, 2019

We do not change code that works. We did such refactoring in the past and that introduced some bugs. So, we either refactor to fix a bug or we keep the current code.

nicolas-grekas changed the base branch from3.4 to4.4

July 27, 2019 09:07

nicolas-grekas approved these changes

Jul 27, 2019

View reviewed changes

Copy link

Member

nicolas-grekas left a comment

Choose a reason for hiding this comment

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

I rebased this PR for 4.4 and reviewed the full code base to ensure we don't call getSession without checking hasSession before, as it is required now (getSession throws in 5.0 instead of returning null)

carsonbot added Status: Reviewed and removed Status: Needs Review labels

Jul 27, 2019

nicolas-grekas changed the title~~[Security] Update some of the codes in AuthenticationUtils~~Ensure $request->getSession() always checks hasSession() before

Jul 27, 2019

nicolas-grekas changed the title~~Ensure $request->getSession() always checks hasSession() before~~Ensure $request->hasSession() is always checked before calling getSession()

Jul 27, 2019

nicolas-grekas modified the milestones:3.4,next

Jul 27, 2019

xabbuh requested changes

Jul 27, 2019

View reviewed changes

src/Symfony/Bridge/Twig/AppVariable.php OutdatedShow resolvedHide resolved

carsonbot added Status: Needs Work and removed Status: Reviewed labels

Jul 27, 2019

Copy link

ContributorAuthor

Arman-Hosseini commentedJul 27, 2019

@nicolas-grekas thank you.

chalasr approved these changes

Jul 27, 2019

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Work labels

Jul 27, 2019

xabbuh previously requested changes

Jul 27, 2019

View reviewed changes

Copy link

Member

xabbuh left a comment

Choose a reason for hiding this comment

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

looks like some tests are failing

carsonbot added Status: Needs Work and removed Status: Reviewed labels

Jul 27, 2019

Ensure $request->hasSession() is always checked before calling getSes…

7b2c326

…sion()

Copy link

Member

chalasr commentedJul 28, 2019

@xabbuh fixed, thanks

nicolas-grekas added Status: Reviewed and removed Status: Needs Work labels

Jul 28, 2019

nicolas-grekas dismissedxabbuh’sstale review

July 28, 2019 17:50

tests are now fixed

xabbuh approved these changes

Jul 28, 2019

View reviewed changes

Copy link

Member

nicolas-grekas commentedJul 30, 2019

Thank you@Arman-Hosseini.

nicolas-grekas merged commit7b2c326 intosymfony:4.4

Jul 30, 2019

nicolas-grekas added a commit that referenced this pull request

Jul 30, 2019

bug#32703Ensure $request->hasSession() is always checked before cal…

5824ab8

…ling getSession() (Arman-Hosseini)This PR was merged into the 4.4 branch.Discussion----------Ensure $request->hasSession() is always checked before calling getSession()| Q             | A| ------------- | ---| Branch?       | 4.4| Bug fix?      | no| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -Commits-------7b2c326 Ensure $request->hasSession() is always checked before calling getSession()