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

[HttpKernel] Dont store response cookies with HttpCache#24190

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

Closed
ro0NL wants to merge1 commit intosymfony:3.3fromro0NL:httpcache/no-cookies
Closed

[HttpKernel] Dont store response cookies with HttpCache#24190

ro0NL wants to merge1 commit intosymfony:3.3fromro0NL:httpcache/no-cookies

Conversation

@ro0NL
Copy link
Contributor

@ro0NLro0NL commentedSep 13, 2017•
edited
Loading

QA
Branch?3.3
Bug fix?yes
New feature?no
BC breaks?no
Deprecations?no
Tests pass?yes
Fixed tickets#...
LicenseMIT
Doc PRsymfony/symfony-docs#...

Before#20569 response cookies, created using theobject API, were not preserved inResponseHeaderBag::all(), thus not stored withHttpCache. Yet, cookies created with the string API did.

That difference is now eliminated, but because of it we now always include all cookies 😅 Causing side effects here (mentioned on slack today). Sorry, i did not anticipatedthat.

I think the preferred behavior is to not store any cookies, as i tend to believe object api is used a lot more.

So this is a hotfix, whereas 3.4 might further discuss features about cookie policy.

Reporter notified, lets wait for confirmation a bit. But i think this should do.

@ro0NL
Copy link
ContributorAuthor

ro0NL commentedSep 13, 2017•
edited
Loading

TBH im just as scared of the opposite behavior really :) im not too familiar with HttpCache, so dont merge blindly.

Status: Needs work

@nicolas-grekas
Copy link
Member

How does Varnish behave here? What does the HTTP spec say about cookies?

@ro0NL
Copy link
ContributorAuthor

Fromhttps://varnish-cache.org/docs/5.1/users-guide/increasing-your-hitrate.html#cookies

Varnish will, in the default configuration, not cache an object coming from the backend with a 'Set-Cookie' header present. Also, if the client sends a Cookie header, Varnish will bypass the cache and go directly to the backend.

Also thinkhttps://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php#L562 implies to preserve actually.

Goal of this PR is to add a test in 3.3 for this :)

@nicolas-grekasnicolas-grekas added this to the3.3 milestoneSep 24, 2017
@ro0NL
Copy link
ContributorAuthor

ro0NL commentedSep 25, 2017•
edited
Loading

Closing for now. Side effect is real; yet current behavior is the one expected IMHO. Tend to believe there be'd more issues if it was really bad :)

@ro0NLro0NL closed thisSep 25, 2017
@ro0NLro0NL deleted the httpcache/no-cookies branchSeptember 25, 2017 06:30
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Projects

None yet

Milestone

3.3

Development

Successfully merging this pull request may close these issues.

3 participants

@ro0NL@nicolas-grekas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp