[HttpFoundation] Deprecate Request::getSession() when Request::hasSession() is false #9625
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
xabbuh
reviewed
Apr 20, 2018
security/form_login.rst
Outdated
| @@ -444,7 +444,9 @@ remove this variable, it's better to use the | |||
| // ... | |||
| use Symfony\Component\Security\Http\Util\TargetPathTrait; | |||
|
|
|||
| $targetPath = $this->getTargetPath($request->getSession(), $providerKey); | |||
| $session = $request->hasSession() ? $request->getSession() : throw_no_session_exception(); | |||
There was a problem hiding this comment.
I am not convinced we should throw an exception here. Not having a session looks valid to me and we should then just use the empty value as the target path IMO.
xabbuh
reviewed
Apr 20, 2018
components/http_foundation.rst
Outdated
| @@ -235,6 +235,13 @@ the | |||
| method tells you if the request contains a session which was started in one of | |||
| the previous requests. | |||
|
|
|||
| .. versionadded:: 4.1 | |||
|
|
|||
xabbuh
reviewed
Apr 20, 2018
components/http_foundation.rst
Outdated
| Using :method:`Symfony\\Component\\HttpFoundation\\Request::getSession()` when no session | ||
| has been set, was deprecated in Symfony 4.1. It will throw an exception in | ||
| Symfony 5.0 when session is null. | ||
| Use :method:`Symfony\\Component\\HttpFoundation\\Request::hasSession()` instead. |
There was a problem hiding this comment.
I would this reword to say something like "Check for an existing session first by calling Request::hasSession()."
|
never mind, that part would have failed previously too if there is no session |
xabbuh
approved these changes
Apr 20, 2018
xabbuh
reviewed
Apr 20, 2018
components/http_foundation.rst
Outdated
| @@ -235,6 +235,12 @@ the | |||
| method tells you if the request contains a session which was started in one of | |||
| the previous requests. | |||
|
|
|||
| .. versionadded:: 4.1 | |||
| Using :method:`Symfony\\Component\\HttpFoundation\\Request::getSession()` | |||
| when no session has been set, was deprecated in Symfony 4.1. It will throw | |||
xabbuh
reviewed
Apr 20, 2018
components/http_foundation.rst
Outdated
| .. versionadded:: 4.1 | ||
| Using :method:`Symfony\\Component\\HttpFoundation\\Request::getSession()` | ||
| when no session has been set, was deprecated in Symfony 4.1. It will throw | ||
| an exception in Symfony 5.0 when session is null. Check for an existing session |
There was a problem hiding this comment.
and "null" should be enclosed by double backticks
|
A nice deprecation message! Thanks for contributing it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This fixes #9457.