fix(http): Enhance setPsr7Request: parse body #196
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
Introduce a proper bridge between PSR-7 ServerRequestInterface and Yii Request, including request body parsing. The setPsr7Request method now: - extracts the raw Content-Type header (trimming parameters like `charset=UTF-8`); - resolves an appropriate request parser from `$this->parsers` (or the `'*'` fallback, if configured); - parses the PSR-7 request body and injects the result into `parsedBody` via `$request->withParsedBody()`; - wraps the request in `ServerRequestAdapter` while adding the `statelessAppStartTime` header for tracing/debugging; - updates the PSR-7 adapter for `UploadedFile` via `UploadedFile::setPsr7Adapter()`. This change: - aligns PSR-7-based requests with the existing Yii Request behavior regarding body parsing and UploadedFile handling; - enables stateless application startup time tracing at the PSR-7 request level; - keeps compatibility with existing Yii request parsers configured in the application.
WalkthroughsetPsr7Request() now strips parameters from Content-Type, selects a parser from a parsers map (specific type or Changes
Sequence DiagramsequenceDiagram
actor Client
participant setPsr7Request as setPsr7Request()
participant ParsersMap as Parsers Map
participant Parser as Parser\n(RequestParserInterface)
participant Adapter as PSR-7 Adapter / Request
Client->>setPsr7Request: call with incoming request
setPsr7Request->>setPsr7Request: read Content-Type\nstrip parameters (semicolon)
rect rgb(230,240,250)
note over setPsr7Request,ParsersMap: Select & validate parser
setPsr7Request->>ParsersMap: lookup by base type or '*'
ParsersMap-->>setPsr7Request: parser instance or none
setPsr7Request->>Parser: assert implements RequestParserInterface
end
rect rgb(240,250,230)
note over setPsr7Request,Parser: Parse body
alt parser found & returns data
setPsr7Request->>Parser: parse body
Parser-->>setPsr7Request: parsed params
setPsr7Request->>Adapter: create adapter with parsed body
else no parser or no data
setPsr7Request->>Adapter: create adapter with original body
end
end
Adapter-->>Client: PSR-7 request (possibly with parsed body)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20-25 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (14)📓 Common learnings📚 Learning: 2025-08-15T01:04:23.564ZApplied to files:
📚 Learning: 2025-08-15T01:04:41.975ZApplied to files:
📚 Learning: 2025-08-10T13:15:30.396ZApplied to files:
📚 Learning: 2025-08-10T13:17:04.265ZApplied to files:
📚 Learning: 2025-08-13T23:36:57.972ZApplied to files:
📚 Learning: 2025-08-28T22:08:51.812ZApplied to files:
📚 Learning: 2025-08-29T09:44:57.466ZApplied to files:
📚 Learning: 2025-07-22T00:50:26.546ZApplied to files:
📚 Learning: 2025-07-20T16:35:15.341ZApplied to files:
📚 Learning: 2025-07-20T16:35:15.341ZApplied to files:
📚 Learning: 2025-08-24T11:52:50.563ZApplied to files:
📚 Learning: 2025-08-06T22:52:05.608ZApplied to files:
📚 Learning: 2025-07-20T16:33:57.495ZApplied to files:
🔇 Additional comments (4)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/http/Request.php (1)
815-815: Casting to array may discard object structure.
RequestParserInterface::parse()can returnarray|object|null. Casting an object to(array)may not preserve the intended structure if parsers return objects.Consider preserving the original type:
if ($parsedParams !== null) { - $request = $request->withParsedBody((array)$parsedParams); + $request = $request->withParsedBody($parsedParams); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/http/Request.php(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 95
File: src/http/Request.php:844-845
Timestamp: 2025-08-15T01:04:23.564Z
Learning: In yii2-extensions/psr-bridge, the Request::setPsr7Request() method calls getRequestStartTime() to initialize timing at the moment the PSR-7 request is set, which marks the actual start of the request cycle in worker environments. This ensures profiling panels get accurate timing from the true request beginning rather than lazy initialization on first access.
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 95
File: src/http/Request.php:844-845
Timestamp: 2025-08-15T01:04:41.975Z
Learning: In yii2-extensions/psr-bridge Request class, the getRequestStartTime() call in setPsr7Request() is intentionally placed to capture accurate request timing at the moment the PSR-7 request is set, which corresponds to when the request cycle begins. This ensures profiling panels get the correct start time reference rather than when timing is first accessed later.
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.812Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
📚 Learning: 2025-08-15T01:04:23.564Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 95
File: src/http/Request.php:844-845
Timestamp: 2025-08-15T01:04:23.564Z
Learning: In yii2-extensions/psr-bridge, the Request::setPsr7Request() method calls getRequestStartTime() to initialize timing at the moment the PSR-7 request is set, which marks the actual start of the request cycle in worker environments. This ensures profiling panels get accurate timing from the true request beginning rather than lazy initialization on first access.
Applied to files:
src/http/Request.php
🧬 Code graph analysis (1)
src/http/Request.php (1)
src/adapter/ServerRequestAdapter.php (1)
ServerRequestAdapter(47-441)
🔇 Additional comments (1)
src/http/Request.php (1)
818-821: LGTM!The stateless application start time header injection aligns with the timing initialization pattern for worker environments. Based on learnings, this ensures profiling panels get accurate timing from the true request beginning.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/http/Request.php (1)
802-814: Consider extracting parser validation to reduce duplication.The parser validation logic (lines 804-806 and 810-812) is duplicated between the specific content-type and fallback branches. Consider extracting to a small helper method for clarity.
Example refactor:
+ /** + * @throws InvalidConfigException + */ + private function validateParser(mixed $parser, string $context): RequestParserInterface + { + if (!($parser instanceof RequestParserInterface)) { + throw new InvalidConfigException("The '$context' request parser is invalid. It must implement the yii\\web\\RequestParserInterface."); + } + return $parser; + } + public function setPsr7Request(ServerRequestInterface $request): void { // ... content-type extraction ... if (isset($this->parsers[$contentType])) { - $parser = Yii::createObject($this->parsers[$contentType]); - if (!($parser instanceof RequestParserInterface)) { - throw new InvalidConfigException("The '$contentType' request parser is invalid. It must implement the yii\\web\\RequestParserInterface."); - } + $parser = $this->validateParser( + Yii::createObject($this->parsers[$contentType]), + $contentType + ); $parsedParams = $parser->parse((string)$request->getBody(), $rawContentType); } elseif (isset($this->parsers['*'])) { - $parser = Yii::createObject($this->parsers['*']); - if (!($parser instanceof RequestParserInterface)) { - throw new InvalidConfigException('The fallback request parser is invalid. It must implement the yii\\web\\RequestParserInterface.'); - } + $parser = $this->validateParser( + Yii::createObject($this->parsers['*']), + 'fallback' + ); $parsedParams = $parser->parse((string)$request->getBody(), $rawContentType); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/http/Request.php(3 hunks)
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 95
File: src/http/Request.php:844-845
Timestamp: 2025-08-15T01:04:23.564Z
Learning: In yii2-extensions/psr-bridge, the Request::setPsr7Request() method calls getRequestStartTime() to initialize timing at the moment the PSR-7 request is set, which marks the actual start of the request cycle in worker environments. This ensures profiling panels get accurate timing from the true request beginning rather than lazy initialization on first access.
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 172
File: docs/testing.md:17-45
Timestamp: 2025-09-25T20:39:56.402Z
Learning: In the yii2-extensions/psr-bridge repository, documentation formatting in markdown files is handled automatically by prettier, so sentence fragments before code blocks are intentional formatting choices that should not be flagged as readability issues.
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 95
File: src/http/Request.php:844-845
Timestamp: 2025-08-15T01:04:41.975Z
Learning: In yii2-extensions/psr-bridge Request class, the getRequestStartTime() call in setPsr7Request() is intentionally placed to capture accurate request timing at the moment the PSR-7 request is set, which corresponds to when the request cycle begins. This ensures profiling panels get the correct start time reference rather than when timing is first accessed later.
📚 Learning: 2025-08-15T01:04:23.564Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 95
File: src/http/Request.php:844-845
Timestamp: 2025-08-15T01:04:23.564Z
Learning: In yii2-extensions/psr-bridge, the Request::setPsr7Request() method calls getRequestStartTime() to initialize timing at the moment the PSR-7 request is set, which marks the actual start of the request cycle in worker environments. This ensures profiling panels get accurate timing from the true request beginning rather than lazy initialization on first access.
Applied to files:
src/http/Request.php
📚 Learning: 2025-08-15T01:04:41.975Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 95
File: src/http/Request.php:844-845
Timestamp: 2025-08-15T01:04:41.975Z
Learning: In yii2-extensions/psr-bridge Request class, the getRequestStartTime() call in setPsr7Request() is intentionally placed to capture accurate request timing at the moment the PSR-7 request is set, which corresponds to when the request cycle begins. This ensures profiling panels get the correct start time reference rather than when timing is first accessed later.
Applied to files:
src/http/Request.php
📚 Learning: 2025-08-10T13:15:30.396Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 78
File: tests/adapter/ServerParamsPsr7Test.php:8-8
Timestamp: 2025-08-10T13:15:30.396Z
Learning: In the yii2-extensions/psr-bridge project, classes referenced in PHPDoc annotations (such as throws declarations) must be imported even if not used in the actual code, to avoid ghost class references and maintain proper documentation.
Applied to files:
src/http/Request.php
📚 Learning: 2025-08-28T22:08:51.812Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 160
File: tests/http/UploadedFileTest.php:122-179
Timestamp: 2025-08-28T22:08:51.812Z
Learning: In yii2-extensions/psr-bridge, when the PSR-7 adapter is set in UploadedFile, it indicates worker mode (like RoadRunner, Swoole), where $_FILES is not meaningful and should not be checked as a fallback. The PSR-7 adapter becomes the authoritative source for uploaded files in this mode.
Applied to files:
src/http/Request.php
📚 Learning: 2025-08-29T09:44:57.466Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 160
File: src/http/UploadedFile.php:110-115
Timestamp: 2025-08-29T09:44:57.466Z
Learning: In yii2-extensions/psr-bridge, the official Yii2 \yii\web\UploadedFile constructor already handles the 'tempResource' key in the config array and properly maps it to the $_tempResource protected property. Therefore, passing 'tempResource' in the config array to the UploadedFile constructor will not cause UnknownPropertyException.
Applied to files:
src/http/Request.php
📚 Learning: 2025-07-22T00:50:26.546Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 21
File: tests/http/PSR7ResponseTest.php:0-0
Timestamp: 2025-07-22T00:50:26.546Z
Learning: In yii2-extensions/psr-bridge, the ResponseAdapter::formatCookieHeader() method uses `$expire !== 1` to skip validation for Yii2's special deletion cookies, but this should be extended to handle all expired cookies, not just the special case where expire=1.
Applied to files:
src/http/Request.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods, so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
src/http/Request.php
📚 Learning: 2025-07-20T16:35:15.341Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.341Z
Learning: In the yii2-extensions/psr-bridge project, the base TestCase class already handles $_SERVER cleanup in setUp() and tearDown() methods (lines 28 and 32), so individual test methods that extend TestCase don't need manual $_SERVER restoration.
Applied to files:
src/http/Request.php
📚 Learning: 2025-08-24T11:52:50.563Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 141
File: tests/http/stateless/ApplicationRoutingTest.php:1-164
Timestamp: 2025-08-24T11:52:50.563Z
Learning: In yii2-extensions/psr-bridge, tests that manipulate PHP superglobals ($_POST, $_GET, $_SERVER) in the http group do not require process isolation and work fine with the current PHPUnit configuration.
Applied to files:
src/http/Request.php
📚 Learning: 2025-08-06T22:52:05.608Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 64
File: tests/http/StatelessApplicationTest.php:1939-1967
Timestamp: 2025-08-06T22:52:05.608Z
Learning: In yii2-extensions/psr-bridge tests, when testing specific component methods like Request::resolve(), it's necessary to call $app->handle($request) first to initialize all application components before testing the method in isolation. This ensures proper component lifecycle initialization.
Applied to files:
src/http/Request.php
📚 Learning: 2025-08-10T13:17:04.265Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 78
File: src/http/Request.php:0-0
Timestamp: 2025-08-10T13:17:04.265Z
Learning: In the yii2-extensions/psr-bridge project, PHPStan type annotations should be defensive when dealing with external input from PSR-7 sources. For methods like `getUploadedFiles()` and `convertPsr7ToUploadedFiles()`, use `mixed` in the return type annotation to account for untrusted input values, even if the implementation filters them.
Applied to files:
src/http/Request.php
📚 Learning: 2025-07-20T16:33:57.495Z
Learnt from: terabytesoftw
Repo: yii2-extensions/psr-bridge PR: 6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.495Z
Learning: The TestCase class in yii2-extensions/psr-bridge automatically handles $_SERVER superglobal cleanup by saving its original state before each test and restoring it afterward in setUp() and tearDown() methods. Manual $_SERVER cleanup in individual test methods is unnecessary when extending this TestCase.
Applied to files:
src/http/Request.php
🔇 Additional comments (4)
src/http/Request.php (4)
10-10: Import added - previous critical issue resolved.The
RequestParserInterfaceimport correctly addresses the fatal error flagged in the previous review.
792-798: Content-Type extraction fixed - now reads from PSR-7 request.The extraction now correctly uses
$request->getHeaderLine('Content-Type'), resolving the critical issue from the previous review where it was reading from$_SERVERinstead.
820-824: LGTM - adapter creation preserves parsed body.The adapter is correctly instantiated with the potentially updated
$requestobject, ensuring that any parsed body data flows through to the Yii2 bridge. ThestatelessAppStartTimeheader remains in place for profiling, as per established patterns in this codebase.Based on learnings, the timing header ensures profiling panels get accurate request timing in worker environments.
800-814: Verify PSR-7 stream rewindability before parsing request body.The code casts
$request->getBody()to string without verifying the stream is seekable or handling rewind. Per PSR-7 specification, streams are not guaranteed to be rewindable—especially non-seekable sources likephp://input. If the stream has been read or is non-seekable, the cast will return an empty string, causing parsers to receive empty input.Two items to verify:
Stream rewindability: Confirm the ServerRequest implementation provides a rewindable stream, or add
rewind()after parsing. Use Guzzle'sUtils::streamFor()orMessage::rewindBody()helper if needed to safely handle non-seekable streams.Parser property: Confirm
$this->parsersis properly configured in the application config (e.g.,config/web.php) with the expected Content-Type mappings.
Remove the unnecessary (array) cast to support object-based parsed bodies.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Introduce a proper bridge between PSR-7 ServerRequestInterface and Yii Request, including request body parsing.
The setPsr7Request method now:
charset=UTF-8);$this->parsers(or the'*'fallback, if configured);parsedBodyvia$request->withParsedBody();ServerRequestAdapterwhile adding thestatelessAppStartTimeheader for tracing/debugging;UploadedFileviaUploadedFile::setPsr7Adapter().This change:
Pull Request
Summary by CodeRabbit
New Features
Behavior
✏️ Tip: You can customize this high-level summary in your review settings.