fix: strip shebang from mock server router for php -S#15
Merged
Conversation
The dependency's bin/openapi-mock-server is a CLI entrypoint prefixed with a `#!/usr/bin/env php` shebang. It was passed directly to `php -S` as the router script. PHP strips shebangs for CLI scripts but not reliably for built-in-server router scripts across all PHP builds (CI's PHP 8.3 does not). The unstripped shebang leaks into the response body, so the following `declare(strict_types=1)` is no longer the first statement and triggers a fatal error. Responses become HTML error pages instead of JSON, failing every acceptance test via seeResponseIsJson. public/index.php cannot be used instead: its autoload path is standalone-only and breaks when installed as a dependency. Emit a shebang-less sibling router next to the bin (preserving __DIR__ so relative autoload/config paths keep resolving), serve that, and remove it in _afterSuite. No-op when the bin has no shebang.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughOpenApiServerMock now resolves a shebang-free router script for PHP’s built-in server, cleans it up after the suite, and composer dependency constraints are bumped. ChangesRouter Script Generation and Cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Suite as Codeception Suite
participant Mock as OpenApiServerMock
participant FS as Filesystem
participant PHP as PHP Built-in Server
Suite->>Mock: startMockServer()
Mock->>Mock: resolveRouterScript()
Mock->>FS: read bin file
Mock->>Mock: strip leading shebang
Mock->>FS: write sibling router script
Mock->>PHP: php -S with resolved router script
Suite->>Mock: _afterSuite()
Mock->>FS: unlink router script
Mock->>Mock: clear stored path
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Bump 26 locked packages including webproject-xyz/php-openapi-mock-server 1.3.5 => 1.3.6 and symfony/* 7.4.14, and raise composer.json constraints to match.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/OpenApiServerMock.php (1)
186-193: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard the write of the generated router script.
file_put_contents()can fail (unwritablevendorbin dir, disk full) and returnsfalse. Currently a failed write still stores$routerPathand returns it, sophp -Sis launched against a missing/empty router and only surfaces later as an opaque "failed to start" error fromwaitForServer(). Fail fast with a clear message instead.♻️ Proposed change
$stripped = (string) preg_replace('/^#![^\n]*\n/', '', $source, 1); $routerPath = dirname($binPath) . '/.openapi-mock-server.router.php'; - file_put_contents($routerPath, $stripped); - $this->routerScript = $routerPath; + if (false === file_put_contents($routerPath, $stripped)) { + throw new RuntimeException("Failed to write mock server router script to '{$routerPath}'."); + } + $this->routerScript = $routerPath; return $routerPath;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/OpenApiServerMock.php` around lines 186 - 193, Guard the generated router write in OpenApiServerMock::getRouterPath() so a failed file_put_contents() does not leave a bogus router path behind. Check the return value when writing $routerPath, and if it is false, throw a clear exception with context instead of returning the path; keep the logic around $stripped, $routerPath, and the shebang-removal flow unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/OpenApiServerMock.php`:
- Around line 186-193: Guard the generated router write in
OpenApiServerMock::getRouterPath() so a failed file_put_contents() does not
leave a bogus router path behind. Check the return value when writing
$routerPath, and if it is false, throw a clear exception with context instead of
returning the path; keep the logic around $stripped, $routerPath, and the
shebang-removal flow unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f19b089a-c7b4-43de-9c12-1cc9d1f74776
📒 Files selected for processing (1)
src/OpenApiServerMock.php
Member
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
🎉 This PR is included in version 1.0.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
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.
Problem
CI failed on 4 acceptance tests (run 28681355179) with:
Root cause
startMockServer()passed the dependency'sbin/openapi-mock-serverdirectly tophp -Sas the router script. That bin is a CLI entrypoint prefixed with#!/usr/bin/env php.PHP strips shebangs for CLI scripts but not reliably for built-in-server router scripts across all PHP builds — CI's PHP 8.3 does not strip it (local 8.3.31 does, which is why it passed locally). The leaked shebang becomes body output, so the following
declare(strict_types=1)is no longer the first statement → fatal error. Responses became HTML error pages instead of JSON, failing everyseeResponseIsJsonassertion.Regression surfaced after the recent dep bump (
ec025b2) tophp-openapi-mock-server1.3.5, which added the shebang.public/index.phpcannot be used instead: its autoload path (__DIR__/../vendor/autoload.php) is standalone-only and breaks when installed as a dependency.Fix
resolveRouterScript()writes a shebang-less sibling router (.openapi-mock-server.router.php) next to the bin — preserving__DIR__so the bin's relative autoload/config paths keep resolving — serves that viaphp -S, and removes it in_afterSuite. No-op when the bin has no shebang.Verification
<?php/declareas first statements and serves/users→ 200.Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.