Skip to content

Commit f03e580

Browse files
akrabatakrabat
authored
Merge pull request #55 from JimTools/bug/cache-hit-still-sends-body
Truncate response body on cache hits
2 parents f608f39 + 5dd9ddd commit f03e580

File tree

2 files changed

+79
-17
lines changed

2 files changed

+79
-17
lines changed

‎src/Cache.php‎

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
use Psr\Http\Message\ResponseInterface;
1313
use Psr\Http\Message\ServerRequestInterface;
14+
use Psr\Http\Message\StreamFactoryInterface;
1415
use Psr\Http\Server\MiddlewareInterface;
1516
use Psr\Http\Server\RequestHandlerInterface;
1617

@@ -45,18 +46,29 @@ class Cache implements MiddlewareInterface
4546
*/
4647
protected $mustRevalidate;
4748

49+
/**
50+
* @var StreamFactoryInterface|null
51+
*/
52+
protected $streamFactory;
53+
4854
/**
4955
* Create new HTTP cache
5056
*
51-
* @param string $type The cache type: "public" or "private"
52-
* @param int $maxAge The maximum age of client-side cache
53-
* @param bool $mustRevalidate must-revalidate
57+
* @param string $type The cache type: "public" or "private"
58+
* @param int $maxAge The maximum age of client-side cache
59+
* @param bool $mustRevalidate must-revalidate
60+
* @param StreamFactoryInterface|null $streamFactory Will clear body of a 304 if provided
5461
*/
55-
public function __construct(string $type = 'private', int $maxAge = 86400, bool $mustRevalidate = false)
56-
{
62+
public function __construct(
63+
string $type = 'private',
64+
int $maxAge = 86400,
65+
bool $mustRevalidate = false,
66+
?StreamFactoryInterface $streamFactory = null
67+
) {
5768
$this->type = $type;
5869
$this->maxAge = $maxAge;
5970
$this->mustRevalidate = $mustRevalidate;
71+
$this->streamFactory = $streamFactory;
6072
}
6173

6274
/**
@@ -101,7 +113,11 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
101113
if ($ifNoneMatch) {
102114
$etagList = preg_split('@\s*,\s*@', $ifNoneMatch);
103115
if (is_array($etagList) && (in_array($etag, $etagList) || in_array('*', $etagList))) {
104-
return $response->withStatus(304);
116+
$response = $response->withStatus(304);
117+
if ($this->streamFactory) {
118+
$response = $response->withBody($this->streamFactory->createStream(''));
119+
}
120+
return $response;
105121
}
106122
}
107123
}
@@ -118,7 +134,11 @@ public function process(ServerRequestInterface $request, RequestHandlerInterface
118134
$ifModifiedSince = $request->getHeaderLine('If-Modified-Since');
119135

120136
if ($ifModifiedSince && $lastModified <= strtotime($ifModifiedSince)) {
121-
return $response->withStatus(304);
137+
$response = $response->withStatus(304);
138+
if ($this->streamFactory) {
139+
$response = $response->withBody($this->streamFactory->createStream(''));
140+
}
141+
return $response;
122142
}
123143
}
124144

‎tests/CacheTest.php‎

Lines changed: 52 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Slim\HttpCache\Cache;
1616
use Slim\Psr7\Factory\ResponseFactory;
1717
use Slim\Psr7\Factory\ServerRequestFactory;
18+
use Slim\Psr7\Factory\StreamFactory;
1819

1920
use function gmdate;
2021
use function time;
@@ -64,7 +65,7 @@ public function handle(ServerRequestInterface $request): ResponseInterface
6465

6566
public function testCacheControlHeader()
6667
{
67-
$cache = new Cache('public', 86400);
68+
$cache = $this->createCache('public', 86400, false, false);
6869
$req = $this->requestFactory();
6970

7071
$res = $cache->process($req, $this->createRequestHandler(null));
@@ -76,7 +77,7 @@ public function testCacheControlHeader()
7677

7778
public function testCacheControlHeaderWithMustRevalidate()
7879
{
79-
$cache = new Cache('private', 86400, true);
80+
$cache = $this->createCache('private', 86400, true, false);
8081
$req = $this->requestFactory();
8182

8283
$res = $cache->process($req, $this->createRequestHandler(null));
@@ -88,7 +89,7 @@ public function testCacheControlHeaderWithMustRevalidate()
8889

8990
public function testCacheControlHeaderWithZeroMaxAge()
9091
{
91-
$cache = new Cache('private', 0, false);
92+
$cache = $this->createCache('private', 0, false, false);
9293
$req = $this->requestFactory();
9394

9495
$res = $cache->process($req, $this->createRequestHandler(null));
@@ -100,7 +101,7 @@ public function testCacheControlHeaderWithZeroMaxAge()
100101

101102
public function testCacheControlHeaderDoesNotOverrideExistingHeader()
102103
{
103-
$cache = new Cache('public', 86400);
104+
$cache = $this->createCache('public', 86400, false, false);
104105
$req = $this->requestFactory();
105106

106107
$res = $this->createResponse()->withHeader('Cache-Control', 'no-cache,no-store');
@@ -116,22 +117,24 @@ public function testLastModifiedWithCacheHit()
116117
$now = time();
117118
$lastModified = gmdate('D, d M Y H:i:s T', $now + 86400);
118119
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now + 86400);
119-
$cache = new Cache('public', 86400);
120+
$cache = $this->createCache('public', 86400, false, false);
120121

121122
$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);
122123

123124
$res = $this->createResponse()->withHeader('Last-Modified', $lastModified);
125+
$res->getBody()->write('payload data');
124126
$res = $cache->process($req, $this->createRequestHandler($res));
125127

126128
$this->assertEquals(304, $res->getStatusCode());
129+
self::assertSame('payload data', (string) $res->getBody());
127130
}
128131

129132
public function testLastModifiedWithCacheHitAndNewerDate()
130133
{
131134
$now = time();
132135
$lastModified = gmdate('D, d M Y H:i:s T', $now + 86400);
133136
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now + 172800); // <-- Newer date
134-
$cache = new Cache('public', 86400);
137+
$cache = $this->createCache('public', 86400, false, false);
135138
$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);
136139

137140
$res = $this->createResponse()->withHeader('Last-Modified', $lastModified);
@@ -145,7 +148,7 @@ public function testLastModifiedWithCacheHitAndOlderDate()
145148
$now = time();
146149
$lastModified = gmdate('D, d M Y H:i:s T', $now + 86400);
147150
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now); // <-- Older date
148-
$cache = new Cache('public', 86400);
151+
$cache = $this->createCache('public', 86400, false, false);
149152
$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);
150153

151154
$res = $this->createResponse()->withHeader('Last-Modified', $lastModified);
@@ -159,7 +162,7 @@ public function testLastModifiedWithCacheMiss()
159162
$now = time();
160163
$lastModified = gmdate('D, d M Y H:i:s T', $now + 86400);
161164
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now - 86400);
162-
$cache = new Cache('public', 86400);
165+
$cache = $this->createCache('public', 86400, false, false);
163166
$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);
164167

165168
$res = $this->createResponse()->withHeader('Last-Modified', $lastModified);
@@ -172,25 +175,64 @@ public function testETagWithCacheHit()
172175
{
173176
$etag = 'abc';
174177
$ifNoneMatch = 'abc';
175-
$cache = new Cache('public', 86400);
178+
$cache = $this->createCache('public', 86400, false, false);
176179
$req = $this->requestFactory()->withHeader('If-None-Match', $ifNoneMatch);
177180

178181
$res = $this->createResponse()->withHeader('Etag', $etag);
182+
$res->getBody()->write('payload data');
179183
$res = $cache->process($req, $this->createRequestHandler($res));
180184

181185
$this->assertEquals(304, $res->getStatusCode());
186+
self::assertSame('payload data', (string) $res->getBody());
182187
}
183188

184189
public function testETagWithCacheMiss()
185190
{
186191
$etag = 'abc';
187192
$ifNoneMatch = 'xyz';
188-
$cache = new Cache('public', 86400);
193+
$cache = $this->createCache('public', 86400, false, false);
189194
$req = $this->requestFactory()->withHeader('If-None-Match', $ifNoneMatch);
190195

191196
$res = $this->createResponse()->withHeader('Etag', $etag);
192197
$res = $cache->process($req, $this->createRequestHandler($res));
193198

194199
$this->assertEquals(200, $res->getStatusCode());
195200
}
201+
202+
public function testETagReturnsNoBodyOnCacheHitWhenAStreamFactoryIsProvided(): void
203+
{
204+
$etag = 'abc';
205+
$cache = $this->createCache('private', 86400, false, true);
206+
$req = $this->requestFactory()->withHeader('If-None-Match', $etag);
207+
208+
$res = $this->createResponse()->withHeader('Etag', $etag);
209+
$res->getBody()->write('payload data');
210+
$res = $cache->process($req, $this->createRequestHandler($res));
211+
212+
self::assertSame(304, $res->getStatusCode());
213+
self::assertSame('', (string) $res->getBody());
214+
}
215+
216+
public function testLastModifiedReturnsNoBodyOnCacheHitWhenAStreamFactoryIsProvided(): void
217+
{
218+
$now = time() + 86400;
219+
$lastModified = gmdate('D, d M Y H:i:s T', $now);
220+
$ifModifiedSince = gmdate('D, d M Y H:i:s T', $now);
221+
$cache = $this->createCache('private', 86400, false, true);
222+
223+
$req = $this->requestFactory()->withHeader('If-Modified-Since', $ifModifiedSince);
224+
$res = $this->createResponse()->withHeader('Last-Modified', $lastModified);
225+
$res->getBody()->write('payload data');
226+
227+
$res = $cache->process($req, $this->createRequestHandler($res));
228+
229+
self::assertEquals(304, $res->getStatusCode());
230+
self::assertSame('', (string) $res->getBody());
231+
}
232+
233+
private function createCache(string $type, int $maxAge, bool $mustRevalidate, bool $withStreamFactory): Cache
234+
{
235+
$streamFactory = $withStreamFactory ? new StreamFactory() : null;
236+
return new Cache($type, $maxAge, $mustRevalidate, $streamFactory);
237+
}
196238
}

0 commit comments

Comments
 (0)