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

Appearance settings

Commit f12b74b

Browse filesBrowse files
committed
Merge pull request FriendsOfSymfony#274 from bastnic/feature/hash-sanity-check
check sanity of user context hash, if not, prevent setting any cache
2 parents 26e44bd + 9543119 commit f12b74b
Copy full SHA for f12b74b

File tree

Expand file treeCollapse file tree

3 files changed

+95
-1
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+95
-1
lines changed

‎CHANGELOG.md

Copy file name to clipboardExpand all lines: CHANGELOG.md
+13Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,19 @@
11
Changelog
22
=========
33

4+
5+
1.3.7
6+
-----
7+
8+
* Add a sanity check on UserContextHash to avoid invalid content being cached
9+
(example: anonymous cache set for authenticated user). This scenario occures
10+
when the UserContextHash is cached by Varnish via
11+
`fos_http_cache.user_context.hash_cache_ttl` > 0 and the session is lost via
12+
garbage collector. The data given is the anonymous one despite having a hash
13+
for authenticated, all authenticated users will then have the anonymous version.
14+
Same problem could occurs with users having is role changed or anything else
15+
that can modify the hash.
16+
417
1.3.2
518
-----
619

‎EventListener/UserContextSubscriber.php

Copy file name to clipboardExpand all lines: EventListener/UserContextSubscriber.php
+20-1Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,11 @@ class UserContextSubscriber implements EventSubscriberInterface
4646
*/
4747
private $userIdentifierHeaders;
4848

49+
/**
50+
* @var string
51+
*/
52+
private $hash;
53+
4954
/**
5055
* @var string
5156
*/
@@ -88,6 +93,10 @@ public function onKernelRequest(GetResponseEvent $event)
8893
}
8994

9095
if (!$this->requestMatcher->matches($event->getRequest())) {
96+
if ($event->getRequest()->headers->has($this->hashHeader)) {
97+
$this->hash = $this->hashGenerator->generateHash();
98+
}
99+
91100
return;
92101
}
93102

@@ -124,9 +133,19 @@ public function onKernelResponse(FilterResponseEvent $event)
124133
}
125134

126135
$response = $event->getResponse();
136+
$request = $event->getRequest();
137+
127138
$vary = $response->getVary();
128139

129-
if ($event->getRequest()->headers->has($this->hashHeader)) {
140+
if ($request->headers->has($this->hashHeader)) {
141+
// hash has changed, session has most certainly changed, prevent setting incorrect cache
142+
if (!is_null($this->hash) && $this->hash !== $request->headers->get($this->hashHeader)) {
143+
$response->setClientTtl(0);
144+
$response->headers->addCacheControlDirective('no-cache');
145+
146+
return;
147+
}
148+
130149
if (!in_array($this->hashHeader, $vary)) {
131150
$vary[] = $this->hashHeader;
132151
}

‎Tests/Unit/EventListener/UserContextSubscriberTest.php

Copy file name to clipboardExpand all lines: Tests/Unit/EventListener/UserContextSubscriberTest.php
+62Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ public function testOnKernelRequestNotMatched()
104104

105105
$requestMatcher = $this->getRequestMatcher($request, false);
106106
$hashGenerator = \Mockery::mock('\FOS\HttpCache\UserContext\HashGenerator');
107+
$hashGenerator->shouldReceive('generateHash')->andReturn('hash');
107108

108109
$userContextSubscriber = new UserContextSubscriber($requestMatcher, $hashGenerator, array('X-SessionId'), 'X-Hash');
109110
$event = $this->getKernelRequestEvent($request);
@@ -168,6 +169,67 @@ public function testOnKernelResponseNotCached()
168169
$this->assertEquals('X-SessionId', $event->getResponse()->headers->get('Vary'));
169170
}
170171

172+
/**
173+
* If there is no hash in the request, vary on the user identifier.
174+
*/
175+
public function testFullRequestHashOk()
176+
{
177+
$request = new Request();
178+
$request->setMethod('GET');
179+
$request->headers->set('X-Hash', 'hash');
180+
181+
$requestMatcher = $this->getRequestMatcher($request, false);
182+
$hashGenerator = \Mockery::mock('\FOS\HttpCache\UserContext\HashGenerator');
183+
$hashGenerator->shouldReceive('generateHash')->andReturn('hash');
184+
185+
// onKernelRequest
186+
$userContextSubscriber = new UserContextSubscriber($requestMatcher, $hashGenerator, array('X-SessionId'), 'X-Hash');
187+
$event = $this->getKernelRequestEvent($request);
188+
189+
$userContextSubscriber->onKernelRequest($event);
190+
191+
$response = $event->getResponse();
192+
193+
$this->assertNull($response);
194+
195+
// onKernelResponse
196+
$event = $this->getKernelResponseEvent($request);
197+
$userContextSubscriber->onKernelResponse($event);
198+
199+
$this->assertContains('X-Hash', $event->getResponse()->headers->get('Vary'));
200+
}
201+
202+
/**
203+
* If there is no hash in the requests but session changed, prevent setting bad cache
204+
*/
205+
public function testFullRequestHashChanged()
206+
{
207+
$request = new Request();
208+
$request->setMethod('GET');
209+
$request->headers->set('X-Hash', 'hash');
210+
211+
$requestMatcher = $this->getRequestMatcher($request, false);
212+
$hashGenerator = \Mockery::mock('\FOS\HttpCache\UserContext\HashGenerator');
213+
$hashGenerator->shouldReceive('generateHash')->andReturn('hash-changed');
214+
215+
// onKernelRequest
216+
$userContextSubscriber = new UserContextSubscriber($requestMatcher, $hashGenerator, array('X-SessionId'), 'X-Hash');
217+
$event = $this->getKernelRequestEvent($request);
218+
219+
$userContextSubscriber->onKernelRequest($event);
220+
221+
$response = $event->getResponse();
222+
223+
$this->assertNull($response);
224+
225+
// onKernelResponse
226+
$event = $this->getKernelResponseEvent($request);
227+
$userContextSubscriber->onKernelResponse($event);
228+
229+
$this->assertFalse($event->getResponse()->headers->has('Vary'));
230+
$this->assertEquals('max-age=0, no-cache, private', $event->getResponse()->headers->get('Cache-Control'));
231+
}
232+
171233
protected function getKernelRequestEvent(Request $request, $type = HttpKernelInterface::MASTER_REQUEST)
172234
{
173235
return new GetResponseEvent(

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.