Introduce Request bridge implementation with tests.
#6
Conversation
|
Warning Rate limit exceeded@terabytesoftw has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThis update introduces a PSR-7 compatible request adapter and a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SAPI/Web Server
participant PSR7Request
participant ServerRequestAdapter
participant Yii2Request
Client->>SAPI/Web Server: Sends HTTP Request
SAPI/Web Server->>PSR7Request: Creates PSR-7 ServerRequest
PSR7Request->>ServerRequestAdapter: Wraps PSR-7 Request
ServerRequestAdapter->>Yii2Request: Provides request data (body, cookies, headers, etc.)
Yii2Request->>Application: Exposes unified API for request handling
Suggested labels
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6 +/- ##
============================================
Coverage 100.00% 100.00%
- Complexity 29 100 +71
============================================
Files 4 6 +2
Lines 92 257 +165
============================================
+ Hits 92 257 +165 ☔ 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: 11
🧹 Nitpick comments (2)
src/http/Request.php (2)
137-144: Fix comment language and improve documentation.The comment is not in English and lacks detail about the method's purpose.
Apply this diff:
/** - * Reset para workers + * Reset the request state for reuse in worker environments. + * + * This method clears the adapter and cached cookies, allowing the + * request object to be reused for handling multiple requests in + * long-running processes like RoadRunner or Swoole. */ public function reset(): void
146-152: Fix comment language.The comment should be in English.
Apply this diff:
/** - * Establece la request PSR-7 + * Set the PSR-7 request. + * + * @param ServerRequestInterface $request The PSR-7 request to adapt */ public function setPsr7Request(ServerRequestInterface $request): void
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
phpstan.neon(1 hunks)phpunit.xml.dist(1 hunks)src/adapter/ServerRequestAdapter.php(1 hunks)src/emitter/SapiEmitter.php(1 hunks)src/http/Request.php(1 hunks)tests/TestCase.php(1 hunks)tests/bootstrap.php(1 hunks)tests/http/RequestTest.php(1 hunks)tests/provider/RequestProvider.php(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/adapter/ServerRequestAdapter.php (1)
src/http/Request.php (11)
getBodyParams(20-27)getParsedBody(73-76)getCookies(29-40)getHeaders(51-62)getMethod(64-71)getQueryParams(86-94)getQueryString(96-103)getRawBody(105-112)getScriptUrl(114-121)getUploadedFiles(126-130)getUrl(132-135)
🪛 Gitleaks (8.27.2)
tests/TestCase.php
83-83: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Check: phpstan / PHP 8.4-ubuntu-latest
tests/TestCase.php
[failure] 70-70:
Class yii\web\Application referenced with incorrect case: Yii\web\Application.
[failure] 48-48:
Instantiated class Yii\console\Application not found.
🪛 GitHub Check: mutation / PHP 8.4-ubuntu-latest
tests/TestCase.php
[failure] 70-70:
Class yii\web\Application referenced with incorrect case: Yii\web\Application.
[failure] 48-48:
Instantiated class Yii\console\Application not found.
🪛 GitHub Actions: build
tests/TestCase.php
[error] 70-70: PHPUnit error: Class "Yii\web\Application" not found. This error occurs in multiple tests indicating a missing or misconfigured Yii web application class.
[error] 48-48: PHPUnit error: Class "Yii\console\Application" not found during testPreferredLanguage.
tests/http/RequestTest.php
[error] 62-62: PHPUnit error: Class "Yii\web\Application" not found during testCsrfHeaderValidation.
[error] 104-104: PHPUnit error: Class "Yii\web\Application" not found during testCsrfTokenContainsASCIIOnly.
[error] 128-128: PHPUnit error: Class "Yii\web\Application" not found during testCsrfTokenHeader.
[error] 179-179: PHPUnit error: Class "Yii\web\Application" not found during testCsrfTokenPost.
[error] 222-222: PHPUnit error: Class "Yii\web\Application" not found during testCsrfTokenValidation.
[error] 288-288: PHPUnit error: Class "Yii\web\Application" not found during testCustomHeaderCsrfHeaderValidation.
[error] 317-317: PHPUnit error: Class "Yii\web\Application" not found during testCustomSafeMethodsCsrfTokenValidation.
[error] 374-374: PHPUnit error: Class "Yii\web\Application" not found during testCustomUnsafeMethodsCsrfHeaderValidation.
[error] 1021-1021: PHPUnit error: Class "Yii\web\Application" not found during testIssue15317.
[error] 1044-1044: PHPUnit error: Class "Yii\web\Application" not found during testNoCsrfTokenCsrfHeaderValidation.
[error] 1131-1131: PHPUnit error: Class "Yii\console\Application" not found during testPreferredLanguage.
[error] 1252-1252: PHPUnit error: Class "Yii\web\Application" not found during testResolve.
🔇 Additional comments (22)
tests/bootstrap.php (1)
1-16: LGTM! Well-structured test bootstrap setup.The bootstrap file correctly sets up the test environment with strict typing, appropriate error reporting, Yii constants, and autoloader inclusion. The
$_SERVERvariable setup prepares the environment for web request simulation in tests.phpstan.neon (1)
5-7: Good alignment of static analysis with test environment.Adding the bootstrap file ensures PHPStan runs with the same Yii environment setup as the tests, improving analysis accuracy and consistency.
phpunit.xml.dist (1)
5-5: Correct bootstrap alignment with new test infrastructure.The change from vendor autoloader to the custom bootstrap file ensures tests run with proper Yii environment setup, maintaining consistency with the PHPStan configuration.
src/emitter/SapiEmitter.php (1)
173-173: Simplified header normalization logic looks good.The updated approach simplifies the header name normalization while maintaining correct capitalization for standard HTTP headers. The logic is more straightforward and easier to understand.
tests/TestCase.php (3)
83-83: Test cookie validation key is not a security risk.The Gitleaks detection is a false positive - this is just a test configuration value for cookie validation, not a real API key.
26-41: Well-designed superglobal state management.The setUp/tearDown methods properly manage
$_SERVER,$_POST, and$_COOKIEsuperglobals to ensure test isolation, which is excellent practice for preventing test interference.
43-93: Solid test infrastructure foundation.The mock application methods provide a good foundation for testing Yii applications with appropriate default configurations for both console and web contexts. The parameterized configuration merging allows flexibility for specific test needs.
src/adapter/ServerRequestAdapter.php (7)
19-33: LGTM!The method correctly handles body parameters extraction and method parameter removal.
97-106: LGTM!The header collection is properly constructed from PSR-7 headers with correct value joining.
108-111: LGTM!Simple and correct delegation to PSR-7.
158-161: LGTM!Correct extraction of query string from PSR-7 URI.
163-169: LGTM!Properly handles stream rewinding before reading contents.
171-183: LGTM!Good handling of both traditional and PSR-7 worker scenarios with clear documentation.
213-223: LGTM!Correctly constructs URL from URI components.
src/http/Request.php (7)
20-27: LGTM!Proper delegation pattern with fallback to parent implementation.
29-40: LGTM!Well-implemented lazy loading with proper caching and read-only protection.
42-49: LGTM!Correctly retrieves CSRF token from configured header.
51-62: LGTM!Properly applies header filtering after retrieving from adapter.
64-71: LGTM!Correctly delegates to the override-aware method.
86-121: LGTM!All getter methods correctly implement the delegation pattern with proper fallbacks.
154-161: LGTM!Clear error handling for uninitialized adapter.
tests/provider/RequestProvider.php (1)
1-880: LGTM!Comprehensive test data provider with good coverage of edge cases and scenarios.
…o_unused_imports in StyleCI.
…ge into feature-2
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
copilot/general-instructions.md (1)
78-81: Add language specification to fenced code block.The fenced code block should specify the language for proper syntax highlighting and markdown compliance.
- ``` + ```text type(scope): description. - ``` + ```copilot/class-documentation.md (1)
30-38: Fix table formatting for markdown compliance.The table should be surrounded by blank lines for proper markdown formatting.
- Use proper indentation for multi-line descriptions. - Include `@copyright` and `@license` with `{@see LICENSE}`. - Document **only what the code actually does**. + ### Common Tags | Tag | Purpose | |---------------|------------------------------------------| | `@param` | Describes each input parameter | | `@return` | Explains the returned value | | `@throws` | Lists exceptions thrown | | `@template` | Declares generic templates | | `@phpstan-var`| Specifies complex property types | + ## Documentation Flowcopilot/project-documentation.md (1)
100-100: Consider hyphenating compound adjective.The compound adjective "rate limiting" modifying "policies" could be hyphenated for clarity, though this is a minor style preference.
-- Rate limiting policies +- Rate-limiting policies
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
Contents(1 hunks)Page not found(1 hunks)This content should not be emitted(1 hunks)copilot/class-documentation.md(1 hunks)copilot/code-style.md(1 hunks)copilot/general-instructions.md(1 hunks)copilot/project-documentation.md(1 hunks)copilot/static-analysis.md(1 hunks)copilot/unit-test.md(1 hunks)tests/TestCase.php(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- Contents
- This content should not be emitted
- Page not found
- copilot/static-analysis.md
- copilot/code-style.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
copilot/class-documentation.md
31-31: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
copilot/general-instructions.md
78-78: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 LanguageTool
copilot/project-documentation.md
[uncategorized] ~100-~100: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...e format - Error structures and codes - Rate limiting policies - Usage examples per endpoint ...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 Gitleaks (8.27.2)
tests/TestCase.php
85-85: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (7)
tests/TestCase.php (3)
85-85: Static analysis false positive - test configuration key is acceptable.The static analysis tool flagged this as a potential API key, but this is a standard test cookie validation key used in Yii2 test configurations. This is not a security concern as it's only used in test environments.
28-43: Well-implemented test environment management.The setUp/tearDown methods properly isolate test environments by managing PHP superglobals. The approach of saving and restoring $_SERVER while clearing $_POST and $_COOKIE is a good practice for test isolation.
48-95: Application mocking methods follow Yii2 best practices.Both
mockApplicationandmockWebApplicationmethods provide appropriate default configurations for testing. The web application configuration includes necessary components like request configuration with proper security settings.copilot/unit-test.md (1)
1-180: Comprehensive and well-structured unit testing guidelines.This document provides excellent guidance for unit testing practices, covering all essential aspects from naming conventions to Copilot optimization. The AAA pattern emphasis, data provider organization, and practical examples make this a valuable reference for maintaining consistent test quality.
copilot/general-instructions.md (1)
1-98: Excellent comprehensive project guidelines.This document establishes clear, comprehensive standards covering all essential aspects of project development. The requirements for PHP 8.2, 100% test coverage, PHPStan level 5+, and conventional commits provide a solid foundation for maintaining code quality.
copilot/class-documentation.md (1)
1-425: Outstanding comprehensive documentation standards.This document provides exceptional detail for class documentation standards, with practical examples for every type of PHP construct. The inclusion of specific templates, exception documentation patterns, and Copilot optimization tips makes this an invaluable resource for maintaining consistent, high-quality documentation.
copilot/project-documentation.md (1)
1-193: Excellent comprehensive documentation guidelines.This document provides thorough guidance for non-code documentation, covering writing style, technical writing best practices, and various documentation types. The Microsoft Writing Style compliance, practical examples, and Copilot optimization tips create a solid foundation for consistent, high-quality project documentation.
…and improve comments.
…n.neon' configuration..
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/adapter/ServerRequestAdapter.php (1)
184-211: Add cleanup mechanism for temporary files.Temporary files created for in-memory streams are never cleaned up, which could lead to disk space issues in long-running processes.
The previous review comment about temporary file cleanup remains valid. Consider implementing a cleanup mechanism to track and remove temporary files when they're no longer needed.
src/http/Request.php (3)
71-74: Add fallback to parent implementation for consistency.This method throws an exception when no adapter is set, unlike other methods that fall back to parent implementation. This inconsistency could cause issues.
The previous review comment about adding fallback behavior remains valid and should be addressed for consistency with other methods in the class.
123-127: Use adapter's normalized uploaded files for consistency.The method returns raw PSR-7 uploaded files instead of using the adapter's normalized version that converts to PHP's $_FILES structure.
The previous review comment about using the adapter's
getUploadedFiles()method remains valid to ensure consistent data structure across the application.
129-132: Add fallback to parent implementation.Missing fallback when adapter is not set.
The previous review comment about adding fallback behavior for the
getUrl()method remains valid and should be addressed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/adapter/ServerRequestAdapter.php(1 hunks)src/http/Request.php(1 hunks)tests/http/PSR7RequestTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
src/adapter/ServerRequestAdapter.php (1)
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: src/adapter/ServerRequestAdapter.php:15-15
Timestamp: 2025-07-19T18:46:16.556Z
Learning: In Yii2 framework, direct public property access is commonly used instead of getter/setter methods, which differs from strict OOP encapsulation patterns. This is an established framework convention that should be followed in Yii2 extensions.
🧬 Code Graph Analysis (1)
src/adapter/ServerRequestAdapter.php (1)
src/http/Request.php (11)
getBodyParams(20-27)getParsedBody(71-74)getCookies(29-38)getHeaders(49-60)getMethod(62-69)getQueryParams(84-91)getQueryString(93-100)getRawBody(102-109)getScriptUrl(111-118)getUploadedFiles(123-127)getUrl(129-132)
🪛 GitHub Check: mutation / PHP 8.4-ubuntu-latest
tests/http/PSR7RequestTest.php
[failure] 371-371:
Class yii\helpers\Json referenced with incorrect case: Yii\helpers\Json.
🪛 GitHub Check: phpstan / PHP 8.4-ubuntu-latest
tests/http/PSR7RequestTest.php
[failure] 371-371:
Class yii\helpers\Json referenced with incorrect case: Yii\helpers\Json.
🪛 Gitleaks (8.27.2)
tests/http/PSR7RequestTest.php
29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
167-167: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
215-215: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
248-248: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
319-319: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
347-347: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
364-364: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Actions: build
tests/http/PSR7RequestTest.php
[error] 371-371: PHPUnit test error: Class "Yii\helpers\Json" not found in testReturnValidatedCookiesWhenValidationEnabledWithValidCookies.
🔇 Additional comments (6)
src/adapter/ServerRequestAdapter.php (2)
15-15: Public property access aligns with Yii2 conventions.Based on the retrieved learnings, this public property pattern follows Yii2 framework conventions where direct property access is commonly used instead of getter/setter methods.
256-256: JSON decoding implementation looks secure.The replacement of
unserialize()withJson::decode()successfully addresses the previous security concern about object injection vulnerabilities.tests/http/PSR7RequestTest.php (4)
29-29: Test validation keys are not security concerns.The static analysis tool is flagging test validation keys as potential API keys, but these are clearly test fixtures and not actual secrets.
Also applies to: 167-167, 215-215, 248-248, 319-319, 347-347, 364-364
18-56: Well-structured test for reset functionality.This test correctly verifies that the adapter and cookie collections are properly reset and that new PSR-7 requests work correctly after reset.
101-148: Excellent test coverage for method parameter removal.This test ensures that the
_methodparameter is correctly removed from body parameters, which is important for HTTP method override functionality.
360-412: Comprehensive cookie validation test.This test properly validates the cookie validation mechanism using Yii's security component, ensuring both valid and invalid cookies are handled correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/http/PSR7RequestTest.php (1)
7-14: Fix import ordering to comply with coding standards.The imports are not in alphabetical order as required by the project's coding standards.
Apply this diff to fix the import order:
-use PHPUnit\Framework\Attributes\Group; -use Yii; -use yii\base\InvalidConfigException; -use yii\web\CookieCollection; -use yii2\extensions\psrbridge\http\Request; -use yii2\extensions\psrbridge\tests\support\FactoryHelper; -use yii2\extensions\psrbridge\tests\TestCase; -use yii\helpers\Json; +use PHPUnit\Framework\Attributes\Group; +use Yii; +use yii\base\InvalidConfigException; +use yii\helpers\Json; +use yii\web\CookieCollection; +use yii2\extensions\psrbridge\http\Request; +use yii2\extensions\psrbridge\tests\support\FactoryHelper; +use yii2\extensions\psrbridge\tests\TestCase;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/http/PSR7RequestTest.php(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.27.2)
tests/http/PSR7RequestTest.php
30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
168-168: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
216-216: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
249-249: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
320-320: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
348-348: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
365-365: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🪛 GitHub Actions: ecs
tests/http/PSR7RequestTest.php
[warning] 7-17: PhpCsFixer ImportOrderedImportsFixer detected unordered imports. Run 'vendor/bin/ecs check --fix' to automatically fix the import order.
🔇 Additional comments (10)
tests/http/PSR7RequestTest.php (10)
19-57: LGTM! Comprehensive test for request reset behavior.This test effectively verifies that the request reset functionality works correctly, ensuring fresh cookie collections are returned after reset operations.
59-100: LGTM! Well-structured test for body parameter extraction.The test comprehensively validates body parameter extraction from PSR-7 requests with appropriate assertions and descriptive error messages.
102-149: LGTM! Important test for method parameter filtering.This test ensures that HTTP method override parameters are properly filtered from body parameters, which is crucial for security and clean parameter handling.
151-205: LGTM! Thorough test for cookie collection with filtering.The test effectively validates cookie collection behavior, including the important feature of filtering out empty cookies.
207-231: LGTM! Good edge case coverage.This test properly validates the behavior when no cookies are present, ensuring an empty but valid CookieCollection is returned.
233-282: LGTM! Clear test for disabled validation scenario.The test clearly validates that when cookie validation is disabled, all non-empty cookies are properly returned without validation checks.
284-302: LGTM! Essential error handling test.This test ensures proper exception handling when cookie validation is misconfigured, which is crucial for preventing silent failures.
304-335: LGTM! Validates proper handling of invalid cookies.This test ensures that invalid cookies are properly filtered out when validation is enabled, returning an empty collection rather than causing errors.
337-359: LGTM! Important test for preventing caching issues.This test ensures that each call to
getCookies()returns a fresh instance, preventing potential stale data issues from inappropriate caching.
361-413: LGTM! Comprehensive test for cookie validation.This test thoroughly validates the cookie validation functionality, including proper creation of signed cookies and verification that only valid cookies are returned when validation is enabled.
…nhance tests for method overrides.
…when adapter is set,
…rious input scenarios.
…retrieve query string from request URI with various scenarios.
… request handling and add comprehensive tests for URL retrieval and uploaded files.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/http/Request.php (1)
186-199: Consider improving error handling in recursive file conversion.The recursive conversion assumes array structure but doesn't handle potential edge cases where mixed types might occur.
Consider adding validation to handle unexpected data structures:
private function convertPsr7ToUploadedFiles(array $uploadedFiles): array { $converted = []; foreach ($uploadedFiles as $key => $file) { if ($file instanceof UploadedFileInterface) { $converted[$key] = $this->createUploadedFile($file); } elseif (is_array($file)) { $converted[$key] = $this->convertPsr7ToUploadedFiles($file); + } else { + // Log or handle unexpected file types + error_log("Unexpected uploaded file type: " . gettype($file)); } } return $converted; }tests/http/RequestTest.php (1)
873-881: Test assertion could be more specific.The test verifies
getUploadedFiles()returns an empty array, but the assertion message suggests this is specifically for "PSR7 request handling" when this test doesn't actually set up PSR-7 requests.Consider clarifying the test's purpose:
public function testGetUploadedFiles(): void { $request = new Request(); self::assertEmpty( $request->getUploadedFiles(), - "'getUploadedFiles()' should return an empty array when not set up for PSR7 request handling.", + "'getUploadedFiles()' should return an empty array when no adapter is set.", ); }This better reflects what the test is actually verifying - the fallback behavior when no PSR-7 adapter is configured.
tests/http/PSR7RequestTest.php (2)
31-45: Minor typo in comment.There's a small typo in the comment on line 39.
- // kust verify the method executes without throwing exception when adapter is 'null' + // just verify the method executes without throwing exception when adapter is 'null'
981-1084: Consider improving file cleanup in uploaded files test.The test creates files in the runtime directory but doesn't clean them up, which could lead to test pollution.
Add cleanup to remove created files:
public function testReturnUploadedFilesWhenAdapterIsSet(): void { $this->mockWebApplication(); $ds = DIRECTORY_SEPARATOR; $file1 = dirname(__DIR__) . '/support/stub/files/test1.txt'; $file2 = dirname(__DIR__) . '/support/stub/files/test2.php'; $size1 = filesize($file1); $size2 = filesize($file2); + $createdFiles = []; // ... existing code ... foreach ($uploadedFiles as $name => $uploadedFile) { // ... existing assertions ... self::assertTrue( $uploadedFile->saveAs("{$runtimePath}{$ds}{$uploadedFile->name}", false), "Uploaded file '{$uploadedFile->name}' should be saved to the runtime directory successfully.", ); + $createdFiles[] = "{$runtimePath}{$ds}{$uploadedFile->name}"; self::assertFileExists( "{$runtimePath}{$ds}{$uploadedFile->name}", "Uploaded file '{$uploadedFile->name}' should exist in the runtime directory after saving.", ); } + + // Cleanup created test files + foreach ($createdFiles as $file) { + if (file_exists($file)) { + unlink($file); + } + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/http/Request.php(1 hunks)tests/http/PSR7RequestTest.php(1 hunks)tests/http/RequestTest.php(1 hunks)tests/provider/RequestProvider.php(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/provider/RequestProvider.php
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/http/Request.php (3)
tests/provider/RequestProvider.php (4)
getBodyParams(54-82)getMethod(236-251)getQueryString(256-284)getUrl(289-305)src/adapter/ServerRequestAdapter.php (11)
getBodyParams(20-34)getCookies(47-52)getHeaders(54-63)getMethod(65-92)getParsedBody(97-100)getQueryParams(105-108)getQueryString(110-113)getRawBody(115-122)getScriptUrl(124-136)getUploadedFiles(149-152)getUrl(154-164)tests/support/FactoryHelper.php (1)
createUploadedFile(181-189)
⏰ 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). (2)
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
🔇 Additional comments (4)
src/http/Request.php (2)
153-160: Noparent::getUploadedFiles()exists to fall back to
TheRequestclass doesn’t extend a parent that providesgetUploadedFiles(), so callingparent::getUploadedFiles()would be invalid. Returning an empty array when no PSR-7 adapter is set is the intended behavior here—if you need to pull from$_FILES, you should ensure an adapter (e.g.ServerRequestAdapter) is always initialized rather than relying on a non-existent parent method.Likely an incorrect or invalid review comment.
76-83: Inconsistent return type annotation and parent method call.The method has inconsistent behavior compared to its PHPStan annotation and parent fallback:
- PHPStan annotation suggests it returns
array|object|nullbut callsparent::getBodyParams()which typically returnsarray- The method name is
getParsedBody()but it falls back togetBodyParams()instead of a potentialgetParsedBody()methodConsider this fix to align with expected behavior:
/** - * @phpstan-return array<mixed, mixed>|object|null + * @phpstan-return mixed */ -public function getParsedBody(): array|object|null +public function getParsedBody(): mixed { if ($this->adapter !== null) { return $this->adapter->getParsedBody(); } - return parent::getBodyParams(); + return parent::getBodyParams(); }Alternatively, if this should truly mirror PSR-7's
getParsedBody(), consider returningnullwhen no adapter is set, or verify if the parent class has agetParsedBody()method.Likely an incorrect or invalid review comment.
tests/http/PSR7RequestTest.php (2)
358-376: Good test coverage for validation configuration.This test properly verifies that cookie validation fails when enabled but no validation key is provided. The exception handling and message checking are appropriate.
1104-1156: Excellent comprehensive test for cookie validation.This test thoroughly validates the complete cookie validation workflow including:
- Signed cookie creation using Yii's security component
- Proper validation of signed cookies
- Rejection of invalid cookies
- Correct decrypted value retrieval
The test demonstrates good understanding of Yii's cookie validation mechanism and proper integration with PSR-7 requests.
…rove CSRF token validation handling.
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
tests/http/PSR7RequestTest.php (1)
1180-1180: Fix extra spaces in string interpolation.- "Uploaded file '{$name }' should have the expected size.", + "Uploaded file '{$name}' should have the expected size.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
tests/http/PSR7RequestTest.php(1 hunks)tests/http/RequestTest.php(1 hunks)
🧰 Additional context used
🪛 Gitleaks (8.27.2)
tests/http/PSR7RequestTest.php
58-58: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
194-194: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
241-241: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
274-274: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
394-394: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
591-591: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
1215-1215: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
⏰ 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). (2)
- GitHub Check: phpunit / PHP 8.3-windows-latest
- GitHub Check: mutation / PHP 8.4-ubuntu-latest
…ed methods and add exception handling for missing PSR-7 adapter.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/adapter/ServerRequestAdapter.php (1)
15-15: Public property usage aligns with Yii2 conventions.Based on the learnings, the public property access pattern is appropriate for Yii2 framework extensions.
🧹 Nitpick comments (1)
tests/http/PSR7RequestTest.php (1)
1195-1195: Fix typo in assertion message.There's a minor typo in the assertion message - extra space in the variable name.
- "Uploaded file '{$name }' should have the expected size.", + "Uploaded file '{$name}' should have the expected size.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/adapter/ServerRequestAdapter.php(1 hunks)tests/http/PSR7RequestTest.php(1 hunks)tests/http/RequestTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.457Z
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.
src/adapter/ServerRequestAdapter.php (2)
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.457Z
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.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: src/adapter/ServerRequestAdapter.php:15-15
Timestamp: 2025-07-19T18:46:16.556Z
Learning: In Yii2 framework, direct public property access is commonly used instead of getter/setter methods, which differs from strict OOP encapsulation patterns. This is an established framework convention that should be followed in Yii2 extensions.
tests/http/PSR7RequestTest.php (1)
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.457Z
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.
tests/http/RequestTest.php (1)
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.457Z
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.
🧬 Code Graph Analysis (1)
src/adapter/ServerRequestAdapter.php (3)
src/http/Request.php (11)
getBodyParams(22-29)getParsedBody(76-83)getCookies(31-40)getHeaders(51-62)getMethod(64-71)getQueryParams(97-104)getQueryString(123-130)getRawBody(132-139)getScriptUrl(141-148)getUploadedFiles(153-160)getUrl(162-169)tests/provider/RequestProvider.php (4)
getBodyParams(54-82)getMethod(236-251)getQueryString(256-284)getUrl(289-305)tests/support/stub/HTTPFunctions.php (1)
hasHeader(88-91)
🪛 GitHub Check: mutation / PHP 8.4-ubuntu-latest
src/adapter/ServerRequestAdapter.php
[warning] 209-209:
Escaped Mutant for Mutator "ArrayOneItem":
@@ @@
}
}
}
-
return $cookies;
-
}
return count($cookies) > 1 ? array_slice($cookies, 0, 1, true) : $cookies;
}
[warning] 199-199:
Escaped Mutant for Mutator "ArrayItemRemoval":
@@ @@
}
$decodedData = Json::decode($data, true);
if (is_array($decodedData) && isset($decodedData[0], $decodedData[1]) && $decodedData[0] === $name) {
-
$cookies[$name] = new Cookie(['name' => $name, 'value' => $decodedData[1], 'expire' => null]);
-
$cookies[$name] = new Cookie(['value' => $decodedData[1], 'expire' => null]); } } }
[warning] 193-193:
Escaped Mutant for Mutator "TrueValue":
@@ @@
if (is_string($data) === false) {
continue;
}
-
$decodedData = Json::decode($data, true);
-
$decodedData = Json::decode($data, false); if (is_array($decodedData) && isset($decodedData[0], $decodedData[1]) && $decodedData[0] === $name) { $cookies[$name] = new Cookie(['name' => $name, 'value' => $decodedData[1], 'expire' => null]); }
[warning] 190-190:
Escaped Mutant for Mutator "Continue_":
@@ @@
if (is_string($value) && $value !== '') {
$data = Yii::$app->getSecurity()->validateData($value, $validationKey);
if (is_string($data) === false) {
-
continue;
-
break; } $decodedData = Json::decode($data, true); if (is_array($decodedData) && isset($decodedData[0], $decodedData[1]) && $decodedData[0] === $name) {
[warning] 161-161:
Escaped Mutant for Mutator "ArrayItemRemoval":
@@ @@
$cookieParams = $this->psrRequest->getCookieParams();
foreach ($cookieParams as $name => $value) {
if ($value !== '') {
-
$cookies[$name] = new Cookie(['name' => $name, 'value' => $value, 'expire' => null]);
-
$cookies[$name] = new Cookie(['value' => $value, 'expire' => null]); } } return $cookies;
[warning] 79-79:
Escaped Mutant for Mutator "UnwrapStrToUpper":
@@ @@
if ($this->psrRequest->hasHeader('X-Http-Method-Override')) {
$overrideHeader = $this->psrRequest->getHeaderLine('X-Http-Method-Override');
if ($overrideHeader !== '') {
-
return strtoupper($overrideHeader);
-
return $overrideHeader; } } return $this->psrRequest->getMethod();
[warning] 67-67:
Escaped Mutant for Mutator "UnwrapStrToUpper":
@@ @@
$parsedBody = $this->psrRequest->getParsedBody();
// check for method override in body
if (is_array($parsedBody) && isset($parsedBody[$methodParam]) && is_string($parsedBody[$methodParam])) {
-
$methodOverride = strtoupper($parsedBody[$methodParam]);
-
$methodOverride = $parsedBody[$methodParam]; if (in_array($methodOverride, ['GET', 'HEAD', 'OPTIONS'], true) === false) { return $methodOverride; }
[warning] 39-39:
Escaped Mutant for Mutator "FalseValue":
@@ @@
/**
* @phpstan-return array
*/
- public function getCookies(bool $enableValidation = false, string $validationKey = ''): array
- public function getCookies(bool $enableValidation = true, string $validationKey = ''): array
{
return $enableValidation ? $this->getValidatedCookies($validationKey) : $this->getSimpleCookies();
}
🪛 Gitleaks (8.27.2)
tests/http/PSR7RequestTest.php
36-36: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
172-172: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
219-219: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
252-252: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
372-372: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
569-569: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
1230-1230: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (29)
src/adapter/ServerRequestAdapter.php (13)
20-34: LGTM! Proper handling of body parameters.The method correctly handles the parsed body from PSR-7 request and removes the method parameter when present, maintaining compatibility with Yii2's behavior.
39-44: LGTM! Clean delegation pattern for cookie handling.The method appropriately delegates to specialized methods based on the validation flag.
46-55: LGTM! Correct header conversion.The method properly converts PSR-7 headers to Yii2's HeaderCollection format, correctly joining multiple values with commas as per HTTP specification.
57-84: LGTM! Secure method override implementation.The method correctly implements HTTP method override with proper security considerations:
- Prevents downgrading to safe methods (GET, HEAD, OPTIONS) via body parameter
- Properly handles case-insensitive header lookup using PSR-7 methods
- Falls back to the original request method
89-92: LGTM! Direct PSR-7 delegation.
97-100: LGTM! Direct PSR-7 delegation.
102-105: LGTM! Proper URI query extraction.
107-114: LGTM! Proper stream handling with rewind.The method correctly rewinds the stream before reading to ensure all content is retrieved.
116-128: LGTM! Proper handling of worker vs traditional environments.The method correctly differentiates between traditional PHP environments and PSR-7 workers, preventing URL duplication issues.
133-136: LGTM! Direct PSR-7 delegation.
138-148: LGTM! Proper URL construction.The method correctly builds the URL from URI components, only appending query string when present.
153-171: LGTM! Proper cookie conversion without validation.The method correctly converts PSR-7 cookies to Yii2 Cookie objects, appropriately skipping empty values.
176-210: LGTM! Secure cookie validation implementation.The method properly validates signed cookies using Yii's security component with JSON deserialization (avoiding unserialize security risks). The validation logic correctly checks data structure and cookie name matching.
tests/http/RequestTest.php (6)
32-46: Consider if $_SERVER cleanup is needed.Based on the learning, the TestCase class automatically handles $_SERVER cleanup. However, if you want explicit cleanup for clarity, the suggested approach in past comments is valid.
48-85: LGTM! Comprehensive CSRF header validation tests.The test properly validates CSRF header behavior for different HTTP methods.
90-109: LGTM! Good test for ASCII-only CSRF tokens.Important test to ensure CSRF tokens contain only ASCII characters for compatibility.
1092-1117: LGTM! Proper $_COOKIE cleanup implemented.The test correctly saves and restores the $_COOKIE global to prevent test pollution.
1327-1469: LGTM! Comprehensive request resolution tests.The test thoroughly validates URL routing resolution, including interaction with $_GET and internal query parameters.
1500-1520: LGTM! Good exception handling tests.Tests properly validate exception cases when adapter is not set or request URI is missing.
tests/http/PSR7RequestTest.php (10)
1-24: LGTM! Well-structured test class setup.The imports, namespace declaration, and class structure follow PHP and PHPUnit best practices. The use of attributes for grouping and the final class declaration are appropriate.
25-285: Comprehensive cookie handling tests with good coverage.The cookie-related tests thoroughly cover various scenarios including validation, empty cookies, and reset functionality. The test validation keys triggering static analysis warnings are appropriate test fixtures, not security concerns.
287-634: Well-designed CSRF token tests covering various scenarios.The CSRF token tests properly cover header-based token retrieval, custom headers, empty values, and fallback behavior. The test logic is sound with appropriate assertions.
65-153: Solid body parameter handling tests.The body parameter tests effectively verify form data parsing and the proper removal of method override parameters. The assertions are comprehensive and the test setup is appropriate.
389-930: Thorough query parameter and string handling tests.The query-related tests provide excellent coverage using data providers and test various scenarios including empty queries and fallback behavior. The implementation is solid.
451-556: Comprehensive HTTP method override testing.The HTTP method tests thoroughly cover various override mechanisms including headers, body parameters, and custom method parameters. The test coverage is excellent with proper assertions.
996-1206: Excellent uploaded files testing with comprehensive coverage.The uploaded files tests are exceptionally thorough, covering nested arrays, file property verification, and actual file operations. The test implementation properly validates the PSR-7 to Yii UploadedFile conversion process.
932-969: Proper raw body content testing.The raw body tests effectively verify stream-based content retrieval with appropriate test setup and assertions. Both empty and populated body scenarios are covered.
751-850: Solid parsed body handling tests.The parsed body tests provide good coverage for different data types (arrays, objects, null) with proper assertions and realistic test scenarios.
418-749: Comprehensive URL and script URL testing.The URL-related tests properly cover different execution modes (worker vs traditional) and fallback scenarios. The $_SERVER modifications are appropriate for testing, and cleanup is handled automatically by the TestCase base class.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/http/PSR7RequestTest.php (1)
667-667: Fix typo in comment.There's a typo in the comment: "kust" should be "just".
- // kust verify the method executes without throwing exception when adapter is 'null' + // just verify the method executes without throwing exception when adapter is 'null'
🧹 Nitpick comments (1)
tests/http/PSR7RequestTest.php (1)
1237-1237: Fix extra whitespace in string interpolation.There's an extra space in the variable interpolation.
- "Uploaded file '{$name }' should have the expected size.", + "Uploaded file '{$name}' should have the expected size.",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tests/http/PSR7RequestTest.php(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.300Z
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.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.300Z
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.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.457Z
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.
tests/http/PSR7RequestTest.php (3)
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.300Z
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.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1536-1552
Timestamp: 2025-07-20T16:35:15.300Z
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.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#6
File: tests/http/RequestTest.php:1564-1578
Timestamp: 2025-07-20T16:33:57.457Z
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.
🪛 Gitleaks (8.27.2)
tests/http/PSR7RequestTest.php
38-38: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
174-174: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
221-221: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
254-254: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
374-374: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
571-571: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
989-989: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
1272-1272: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (1)
tests/http/PSR7RequestTest.php (1)
24-1321: Excellent comprehensive test coverage for PSR-7 integration.This test class provides thorough coverage of the
Requestclass PSR-7 integration scenarios including:
- Cookie handling with and without validation
- HTTP method determination with various override mechanisms
- CSRF token retrieval from headers
- Query parameter and string parsing
- Raw body content handling
- Uploaded files conversion and saving
- Parsed body data handling
- Proper fallback to parent behavior when adapter is null
The tests are well-structured, use descriptive assertions, and follow PHPUnit best practices.
Summary by CodeRabbit
New Features
Tests
Chores
Style