fix(http): Adjust session handling in StatelessApplication to set session ID from cookies and ensure clean state management.
#158
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ession ID from cookies and ensure clean state management.
WalkthroughUpdated StatelessApplication::reset to derive the session ID from the request cookie (by session name), set it on the session, run bootstrap, and finally close the session. Removed the earlier explicit session close/open cycle before bootstrap. No public API signatures changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant R as Request
participant A as StatelessApplication.reset()
participant S as Session
participant B as Bootstrap
R->>A: Invoke reset()
A->>R: Read cookie by session name
A->>S: setId(sessionId)
A->>B: bootstrap()
A->>S: close()
note right of A: Pre-bootstrap close/open removed
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #158 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 317 317
===========================================
Files 12 12
Lines 803 801 -2
===========================================
- Hits 803 801 -2 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/http/StatelessApplication.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
src/http/StatelessApplication.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: phpunit / PHP 8.1-ubuntu-latest
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: composer-require-checker / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
Comment on lines
+423
to
426
| // if a session cookie is present, set the session ID accordingly | ||
| $sessionId = $this->request->getCookies()->get($this->session->getName())->value ?? ''; | ||
| $this->session->setId($sessionId); | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Fix null dereference and avoid Yii cookie-validation mismatch when reading session ID
->get(...)->value ?? ''will fatal when the cookie is absent because->valueis accessed onnull.- Reading from
$this->request->getCookies()depends on Yii cookie validation; the PHP session cookie (e.g.,PHPSESSID) is typically not signed and may be filtered out when validation is enabled. Use PSR-7$request->getCookieParams()instead and only set the ID when present and non-empty.
Apply this diff:
- // if a session cookie is present, set the session ID accordingly
- $sessionId = $this->request->getCookies()->get($this->session->getName())->value ?? '';
- $this->session->setId($sessionId);
+ // if a session cookie is present, set the session ID accordingly
+ $sessionName = $this->session->getName();
+ $sessionId = $request->getCookieParams()[$sessionName] ?? null;
+ if (is_string($sessionId) && $sessionId !== '') {
+ $this->session->setId($sessionId);
+ }Optionally, add a lightweight unit test to cover:
- no session cookie present → setId() is not called
- valid session cookie present → setId() receives that value
- empty session cookie value → setId() is not called
I can draft the tests if helpful.
🏁 Script executed:
#!/bin/bash
# Locate the StatelessApplication.php file
file=$(rg -l "class StatelessApplication" --glob "*.php")
echo "StatelessApplication file: $file"
# Show lines around the snippet (lines ~423)
sed -n '400,460p' "$file"
# Show occurrences of getCookies in the file
rg -n "getCookies" -C2 "$file"
# Search for any use of getCookieParams in the repo
rg -n "getCookieParams" -C3 --glob "*.php"Length of output: 3942
🏁 Script executed:
#!/bin/bash
# Display the session‐cookie logic in StatelessApplication
sed -n '400,460p' src/http/StatelessApplication.php
# Check if getCookieParams is already used in this file
rg -n 'getCookieParams' -C3 src/http/StatelessApplication.phpLength of output: 2383
Prevent null dereference and bypass Yii cookie validation for session ID
In src/http/StatelessApplication.php inside the reset() method (around line 423), the current code…
// if a session cookie is present, set the session ID accordingly
$sessionId = $this->request->getCookies()->get($this->session->getName())->value ?? '';
$this->session->setId($sessionId);…can
- fatally dereference
->valuewhen the named cookie is missing (it returnsnull), - and silently overwrite the session ID with an empty string even if no cookie was provided,
- while also relying on Yii’s cookie-validation (which drops unsigned PHPSESSID cookies).
Replace with PSR-7’s raw cookie params and guard both null and empty values:
- // if a session cookie is present, set the session ID accordingly
- $sessionId = $this->request->getCookies()->get($this->session->getName())->value ?? '';
- $this->session->setId($sessionId);
+ // if a session cookie is present, set the session ID accordingly
+ $sessionName = $this->session->getName();
+ $sessionId = $request->getCookieParams()[$sessionName] ?? null;
+ if (is_string($sessionId) && $sessionId !== '') {
+ $this->session->setId($sessionId);
+ }To fully cover this behavior, add a small unit test for reset():
- no session cookie →
session->setId()is never invoked - valid non-empty cookie →
session->setId()is called with that value - cookie present but empty →
session->setId()is not invoked
🤖 Prompt for AI Agents
In src/http/StatelessApplication.php around lines 423-426, the code currently
dereferences ->value on a potentially null cookie and unconditionally calls
$this->session->setId('') when no cookie exists; change it to read the raw PSR-7
cookie params (e.g. $this->request->getCookieParams()), check for the cookie
name existence and that its value is non-null and non-empty before calling
$this->session->setId($value) so you never dereference null or overwrite the
session with an empty string; additionally add unit tests for reset(): (1) no
session cookie => assert setId() is never called, (2) valid non-empty cookie =>
assert setId() called with that value, (3) cookie present but empty => assert
setId() is not called.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit