Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[FrameworkBundle][HttpFoundation] make session service resettable#30620

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

Merged
fabpot merged 1 commit intosymfony:3.4fromdmaicher:issue-30619
Mar 23, 2019

Conversation

@dmaicher
Copy link
Contributor

QA
Branch?3.4
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#30619
LicenseMIT
Doc PR-

Thisfixes#30619 by making the session service "resettable" viaServicesResetter.

@nicolas-grekas
Copy link
Member

Before merging: the way tests are written is not compatible with phpunit 4.8.

@dmaicher
Copy link
ContributorAuthor

Before merging: the way tests are written is not compatible with phpunit 4.8.

@nicolas-grekas because of$this->createMock(...)?

I find quite a lot of usages of that in the 3.4 branch.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

(failure unrelated)

@fabpot
Copy link
Member

Thank you@dmaicher.

dmaicher reacted with thumbs up emoji

@fabpotfabpot merged commite46ef76 intosymfony:3.4Mar 23, 2019
fabpot added a commit that referenced this pull requestMar 23, 2019
…ettable (dmaicher)This PR was merged into the 3.4 branch.Discussion----------[FrameworkBundle][HttpFoundation] make session service resettable| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets |#30619| License       | MIT| Doc PR        | -Thisfixes#30619 by making the session service "resettable"  via `ServicesResetter`.Commits-------e46ef76 [FrameworkBundle][HttpFoundation] make session service resettable
This was referencedApr 2, 2019
@Chi-teck
Copy link
Contributor

This caused a critical issue in Drupal 8.
https://www.drupal.org/project/drupal/issues/3045349

@dmaicherdmaicher deleted the issue-30619 branchApril 6, 2019 10:24
@dmaicher
Copy link
ContributorAuthor

@Chi-teck sorry to hear that. Please open an issue if you think we should discuss/track this.

@mdlutz24
Copy link
Contributor

The issue in drupal was that we use a custom session storage such that if a user is not logged into the website then we don't start the session UNTIL something is saved to prevent setting cookies unnecessarily. Since our storage save function isn't called anymore until the session is started, we don't save the session at all anymore.

Our initial workaround was to adjust our isStarted method to return true after bootstrap, but we are finding that is causing problems in other places that depend on isStarted() to tell us whether the session is actually started or not.

Might it make sense to move the responsibility for checking if the session is started from Session::save and into SessionStorageInterface::save? This would avoid the BC issues for custom storage that depended on managing whether they should save or not on their own, but still allow for the bug fix for the bundled storage classes.

nicolas-grekas added a commit to nicolas-grekas/symfony that referenced this pull requestMay 1, 2019
…ion service resettable (dmaicher)"This reverts commit029fb2e, reversingchanges made to9dad29d.
@nicolas-grekasnicolas-grekas added this to the3.4 milestoneMay 1, 2019
fabpot added a commit that referenced this pull requestMay 1, 2019
…session service resettable (dmaicher)" (nicolas-grekas)This PR was merged into the 3.4 branch.Discussion----------Revert "bug#30620 [FrameworkBundle][HttpFoundation] make session service resettable (dmaicher)"| Q             | A| ------------- | ---| Branch?       | 3.4| Bug fix?      | yes| New feature?  | no| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| Fixed tickets | -| License       | MIT| Doc PR        | -This reverts commit029fb2e, reversingchanges made to9dad29d.Reverts#30620Replaces#31215We don't need to solve this in 3.4Making the session resettable should be done on master, by implementing `ResetInterface`.On 3.4 apps, one should write a dedicated `SessionResetter` that would implement the reverted logic.Commits-------4177331 Revert "bug#30620 [FrameworkBundle][HttpFoundation] make session service resettable (dmaicher)"
fabpot added a commit that referenced this pull requestMay 1, 2019
* 3.4:  Revert "bug#30620 [FrameworkBundle][HttpFoundation] make session service resettable (dmaicher)"  [Workflow] Fixed dumping when many transition with same name exist  fix ConsoleFormatter - call to a member function format() on string
fabpot added a commit that referenced this pull requestMay 1, 2019
* 4.2:  Revert "bug#30620 [FrameworkBundle][HttpFoundation] make session service resettable (dmaicher)"  [Workflow] Fixed dumping when many transition with same name exist  relax assertions in tests  fix ConsoleFormatter - call to a member function format() on string  Made `debug:container` and `debug:autowiring` ignore starting backslash in service id  [Validator] Translate messages into Japanese  Fix Thai translation in validators.th.xlf  [FramworkBundle] mark any env vars found in the ide setting as used
This was referencedMay 1, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+1 more reviewer

@vudaltsovvudaltsovvudaltsov approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

3.4

Development

Successfully merging this pull request may close these issues.

7 participants

@dmaicher@nicolas-grekas@fabpot@Chi-teck@mdlutz24@vudaltsov@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp