Skip to content

Commit 2d24733

Browse files
smarcetsmarcet
committed
fix(cache): prevent non-200 response poisoning and thundering herd in CacheMiddleware
1 parent 4392a9d commit 2d24733

1 file changed

Lines changed: 87 additions & 57 deletions

File tree

‎app/Http/Middleware/CacheMiddleware.php‎

Lines changed: 87 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,20 @@
11
<?php namespace App\Http\Middleware;
2+
/**
3+
* Copyright 2026 OpenStack Foundation
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
**/
214

315
use Closure;
416
use Illuminate\Http\JsonResponse;
517
use Illuminate\Support\Facades\Cache;
6-
use Illuminate\Support\Facades\Config;
718
use Illuminate\Support\Facades\Log;
819
use libs\utils\CacheRegions;
920
use models\oauth2\IResourceServerContext;
@@ -17,9 +28,11 @@ public function __construct(IResourceServerContext $context)
1728
$this->context = $context;
1829
}
1930

20-
private const ENC_DF = 'DF1:'; // gzdeflate/gzinflate
21-
private const ENC_P0 = 'P0:'; // without compression
22-
private int $gzipLevel = 9; //
31+
private const ENC_DF = 'DF1:'; // gzdeflate/gzinflate
32+
private const ENC_P0 = 'P0:'; // without compression
33+
private const LOCK_TTL = 10; // lock auto-expires after 10s (safety net if holder crashes)
34+
private const LOCK_WAIT = 5; // losers wait up to 5s for the winner to finish
35+
private int $gzipLevel = 9;
2336

2437
private function encode(array $payload):string{
2538
$json = json_encode($payload, JSON_UNESCAPED_UNICODE);
@@ -86,71 +99,88 @@ public function handle($request, Closure $next, $cache_lifetime, $cache_region =
8699
$regionTag = CacheRegions::getCacheRegionFor($cache_region, $id);
87100
}
88101
}
89-
$status = 200;
90-
$wasHit = false;
91-
if ($regionTag) {
92-
Log::debug("CacheMiddleware: using region tag {$regionTag} ip {$ip} agent {$agent}");
93-
$wasHit = Cache::tags($regionTag)->has($key);
94-
Log::debug($wasHit ? "CacheMiddleware: cache HIT (tagged)" : "CacheMiddleware: cache MISS (tagged)", [
95-
'tag' => $regionTag,
96-
'ip' => $ip,
97-
'agent' => $agent,
98-
'key' => $key,
99-
]);
100-
101-
$encoded = Cache::tags($regionTag)
102-
->remember($key, $cache_lifetime, function() use ($next, $request, $regionTag, $key, $cache_lifetime, &$status,$ip, $agent) {
103-
$resp = $next($request);
104-
if ($resp instanceof JsonResponse) {
105-
$status = $resp->getStatusCode();
106-
if($status === 200) {
107-
return $this->encode($resp->getData(true));
108-
}
109-
}
110-
// don’t cache non-200 or non-JSON
111-
return Cache::get($key);
112-
});
102+
$cache = $regionTag ? Cache::tags($regionTag) : Cache::store();
103+
$logCtx = array_filter([
104+
'tag' => $regionTag,
105+
'ip' => $ip,
106+
'agent' => $agent,
107+
'key' => $key,
108+
]);
109+
110+
// Phase 1: optimistic read — no lock needed on hit
111+
$encoded = $cache->get($key);
112+
113+
if ($encoded !== null) {
114+
Log::debug("CacheMiddleware: cache HIT", $logCtx);
115+
113116
$data = $this->decode($encoded);
117+
if ($data === null) $data = is_array($encoded) ? $encoded : [];
118+
119+
$response = new JsonResponse($data, 200, ['Content-Type' => 'application/json']);
120+
$wasHit = true;
114121
} else {
115-
$wasHit = Cache::has($key);
116-
117-
Log::debug($wasHit ? "CacheMiddleware: cache HIT" : "CacheMiddleware: cache MISS", [
118-
'ip' => $ip,
119-
'agent' => $agent,
120-
'key' => $key,
121-
]);
122-
123-
$encoded = Cache::remember($key, $cache_lifetime, function() use ($next, $request, $key, &$status, $ip, $agent) {
124-
$resp = $next($request);
125-
if ($resp instanceof JsonResponse) {
126-
$status = $resp->getStatusCode();
127-
if($status === 200)
128-
return $this->encode($resp->getData(true));
122+
// Phase 2: cache miss — acquire lock so only one request executes the handler
123+
$lockKey = $regionTag ? "cache_lock:{$regionTag}:{$key}" : "cache_lock:{$key}";
124+
$lock = Cache::lock($lockKey, self::LOCK_TTL);
125+
$wasHit = false;
126+
127+
try {
128+
if ($lock->block(self::LOCK_WAIT)) {
129+
// Won the lock — double-check: another request may have populated the cache
130+
$encoded = $cache->get($key);
131+
132+
if ($encoded !== null) {
133+
Log::debug("CacheMiddleware: cache HIT (after lock)", $logCtx);
134+
135+
$data = $this->decode($encoded);
136+
if ($data === null) $data = is_array($encoded) ? $encoded : [];
137+
138+
$response = new JsonResponse($data, 200, ['Content-Type' => 'application/json']);
139+
$wasHit = true;
140+
} else {
141+
Log::debug("CacheMiddleware: cache MISS (executing handler)", $logCtx);
142+
143+
$resp = $next($request);
144+
145+
// Only cache 200 JSON responses; let everything else pass through as-is
146+
if ($resp instanceof JsonResponse && $resp->getStatusCode() === 200) {
147+
$cache->put($key, $this->encode($resp->getData(true)), $cache_lifetime);
148+
} else {
149+
return $resp;
150+
}
151+
152+
$response = $resp;
153+
}
154+
} else {
155+
// Could not acquire lock within LOCK_WAIT seconds — fall through without lock
156+
Log::warning("CacheMiddleware: lock timeout, executing handler without lock", $logCtx);
157+
158+
$resp = $next($request);
159+
160+
if ($resp instanceof JsonResponse && $resp->getStatusCode() === 200) {
161+
$cache->put($key, $this->encode($resp->getData(true)), $cache_lifetime);
162+
} else {
163+
return $resp;
164+
}
165+
166+
$response = $resp;
129167
}
130-
return Cache::get($key);
131-
});
132-
$data = $this->decode($encoded);
168+
} finally {
169+
$lock->release();
170+
}
133171
}
134-
// safe guard
135-
if ($data === null) $data = is_array($encoded) ? $encoded : [];
136172

137-
// Build the JsonResponse (either from cache or fresh)
138-
$response = new JsonResponse($data, $status, ['Content-Type' => 'application/json']);
139-
140-
// Mark for revalidation so your ETag middleware can return 304 when unchanged
173+
// Mark for revalidation so ETag middleware can return 304 when unchanged
141174
$response->setPublic();
142175
$response->setMaxAge(0);
143176
$response->headers->addCacheControlDirective('must-revalidate', true);
144177
$response->headers->addCacheControlDirective('proxy-revalidate', true);
145178
$response->headers->add([
146-
'X-Cache-Result' => $wasHit ? 'HIT':'MISS',
147-
]);
148-
Log::debug( "CacheMiddleware: returning response", [
149-
'ip' => $ip,
150-
'agent' => $agent,
151-
'key' => $key,
179+
'X-Cache-Result' => $wasHit ? 'HIT' : 'MISS',
152180
]);
153181

182+
Log::debug("CacheMiddleware: returning response", $logCtx);
183+
154184
return $response;
155185
}
156186

0 commit comments

Comments
 (0)