[HttpFoundation] Fix cookie to string conversion for raw cookies #20910
Conversation
| if ($this->path) { | ||
| $str .= '; path='.$this->path; | ||
| } | ||
| $str .= '; path='.$this->getPath() ?: '/'; |
There was a problem hiding this comment.
See #20569 (comment) for my reasoning behind this. Im also fine with if ($this->getPath()) { ... } in case the user breaks the contract from a subclass.
There was a problem hiding this comment.
I prefer to keep the condition to keep the payload small (I know that path= / is not that long, but as this is optional, let's do it that way).
There was a problem hiding this comment.
Agree. However source of the problem is https://github.com/symfony/symfony/pull/20910/files#diff-a1e299c557d5ff5fde6fa480eab85d47R70
If it's truly optional (and it is :)) symfony should make no assumptions on empty values. Imo. passing $path=null, $path='', $path='/' should all behave as expected, respectively; <no-path>, path=;, path=/;
Perhaps something to look into later on. For now ill put back the condition 👍
|
Failing test (PHP7/redis) seems unrelated. |
|
Thank you @ro0NL. |
…ookies (ro0NL) This PR was squashed before being merged into the 3.1 branch (closes #20910). Discussion ---------- [HttpFoundation] Fix cookie to string conversion for raw cookies | Q | A | ------------- | --- | Branch? | 3.1 | Bug fix? | yes | New feature? | not really | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20569 (comment) | License | MIT | Doc PR | symfony/symfony-docs#... <!--highly recommended for new features--> Separated from #20569 This mimics PHP's `setrawcookie` behavior. Commits ------- 5e899cd [HttpFoundation] Fix cookie to string conversion for raw cookies
Separated from #20569
This mimics PHP's
setrawcookiebehavior.