Skip to content

Commit 8b547e8

Browse files
committed
Fix session key matching and improve session handling in cache integration
1 parent d30f9d7 commit 8b547e8

File tree

2 files changed

+36
-14
lines changed

2 files changed

+36
-14
lines changed

src/Sentry/Laravel/Features/CacheIntegration.php

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -278,23 +278,27 @@ private function isSessionKey(string $key): bool
278278
$request = $this->container()->make('request');
279279

280280
// Check if the request has a session
281-
if (!$request->hasSession()) {
281+
if (!method_exists($request, 'hasSession') || !$request->hasSession()) {
282282
return false;
283283
}
284284

285285
$session = $request->session();
286286

287287
// Don't start the session if it hasn't been started yet
288-
if (!$session->isStarted()) {
288+
if (!method_exists($session, 'isStarted') || !$session->isStarted()) {
289289
return false;
290290
}
291291

292-
// Get the session ID and check if the cache key matches
292+
// Get the session ID and check if the cache key matches exactly
293293
$sessionId = $session->getId();
294+
295+
// Check for empty session ID
296+
if (empty($sessionId)) {
297+
return false;
298+
}
294299

295-
// Check if the key equals the session ID or contains it
296-
// This handles cases where the cache key might be prefixed
297-
return $key === $sessionId || Str::endsWith($key, $sessionId);
300+
// Check if the key equals the session ID exactly
301+
return $key === $sessionId;
298302
} catch (\Exception $e) {
299303
// If anything goes wrong, we assume it's not a session key
300304
return false;

test/Sentry/Features/CacheIntegrationTest.php

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,8 @@ public function testCacheBreadcrumbIsNotRecordedWhenDisabled(): void
5858
public function testCacheBreadcrumbReplacesSessionKeyWithPlaceholder(): void
5959
{
6060
// Start a session properly in the test environment
61-
$this->withSession(['test' => 'value']);
62-
61+
$this->ensureRequestIsBound();
62+
$this->startSession();
6363
$sessionId = $this->app['session']->getId();
6464

6565
// Use the session ID as a cache key
@@ -183,8 +183,8 @@ public function testCacheSpanReplacesSessionKeyWithPlaceholder(): void
183183
$this->markSkippedIfTracingEventsNotAvailable();
184184

185185
// Start a session properly in the test environment
186-
$this->withSession(['test' => 'value']);
187-
186+
$this->ensureRequestIsBound();
187+
$this->startSession();
188188
$sessionId = $this->app['session']->getId();
189189

190190
$span = $this->executeAndReturnMostRecentSpan(function () use ($sessionId) {
@@ -201,17 +201,17 @@ public function testCacheSpanReplacesMultipleSessionKeysWithPlaceholder(): void
201201
$this->markSkippedIfTracingEventsNotAvailable();
202202

203203
// Start a session properly in the test environment
204-
$this->withSession(['test' => 'value']);
205-
204+
$this->ensureRequestIsBound();
205+
$this->startSession();
206206
$sessionId = $this->app['session']->getId();
207207

208208
$span = $this->executeAndReturnMostRecentSpan(function () use ($sessionId) {
209209
Cache::get([$sessionId, 'regular-key', $sessionId . '_another']);
210210
});
211211

212212
$this->assertEquals('cache.get', $span->getOp());
213-
$this->assertEquals('{sessionKey}, regular-key, {sessionKey}', $span->getDescription());
214-
$this->assertEquals(['{sessionKey}', 'regular-key', '{sessionKey}'], $span->getData()['cache.key']);
213+
$this->assertEquals('{sessionKey}, regular-key, ' . $sessionId . '_another', $span->getDescription());
214+
$this->assertEquals(['{sessionKey}', 'regular-key', $sessionId . '_another'], $span->getData()['cache.key']);
215215
}
216216

217217
public function testCacheOperationDoesNotStartSessionPrematurely(): void
@@ -252,4 +252,22 @@ private function executeAndReturnMostRecentSpan(callable $callable): Span
252252

253253
return array_pop($spans);
254254
}
255+
256+
private function ensureRequestIsBound(): void
257+
{
258+
// Ensure we have a request instance
259+
if (!$this->app->bound('request')) {
260+
$this->app->instance('request', $this->app->make(\Illuminate\Http\Request::class));
261+
}
262+
}
263+
264+
private function startSession(): void
265+
{
266+
// Start the session
267+
$session = $this->app['session'];
268+
$session->start();
269+
270+
// Set the session on the request
271+
$this->app['request']->setLaravelSession($session);
272+
}
255273
}

0 commit comments

Comments
 (0)