8000 ResponseHeaderBag::computeCacheValue() doesn't handle cloning well · Issue #16171 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

ResponseHeaderBag::computeCacheValue() doesn't handle cloning well #16171

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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
neclimdul opened this issue Oct 7, 2015 · 1 comment
Closed

ResponseHeaderBag::computeCacheValue() doesn't handle cloning well #16171

neclimdul opened this issue Oct 7, 2015 · 1 comment
Labels
Bug HttpFoundation Status: Needs Review

Comments

@neclimdul
Copy link
Contributor

Consider the following test:

<?php
class HeaderTest extends UnitTestCase {
  public function testHeaderBag() {
    $headers = ['test'];
    $bag1 = new ResponseHeaderBag($headers);
    $bag2 = new ResponseHeaderBag($bag1->allPreserveCase());
    $this->assertEquals($bag1->allPreserveCase(), $bag2->allPreserveCase());
  } 
}
?>

It was my assumption that this would pass but it ends up not.

Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array (
     0 => Array (...)
     'Cache-Control' => Array (
-        0 => 'no-cache'
+        0 => 'no-cache, private'
     )
 )

I'm not sure the solution but these aren't technically the same thing so its seems buggy.
@ewgRa ewgRa mentioned this issue Oct 21, 2015
Closed
@ewgRa
Copy link
Contributor
ewgRa commented Oct 21, 2015

PR created, must fix this case.

@jakzal jakzal mentioned this issue Dec 30, 2015
Closed
@fabpot fabpot mentioned this issue Jun 22, 2016
Merged
@fabpot fabpot closed this as completed Jun 23, 2016
fabpot added a commit that referenced this issue Jun 23, 2016
@fabpot
feature #19143 Response headers fix (fabpot)
e949d34 
This PR was merged into the 3.2-dev branch.

Discussion
----------

Response headers fix

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes/no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #16171, #16307
| License       | MIT
| Doc PR        | n/a

To fix the inconsistency mentioned in #16171, I think the "best" solution would be to add `private` when cache-control is not set, which was the intention but was forgotten.

I propose to make the fix in 3.2 only as it might be a BC break.

Commits
-------

66afa01 [HttpFoundation] added private by default when setting Cache-Control to no-cache
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpFoundation Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @javiereguiluz @neclimdul @ewgRa @xabbuh @carsonbot
0