fix(webserver-config): scope nginx php handler per app for correct SCRIPT_FILENAME#18
Draft
TDannhauer wants to merge 1 commit into
Draft
fix(webserver-config): scope nginx php handler per app for correct SCRIPT_FILENAME#18TDannhauer wants to merge 1 commit into
TDannhauer wants to merge 1 commit into
Conversation
…RIPT_FILENAME The nginx flavor emitted a single server-scope `location ~ \.php$` block using `SCRIPT_FILENAME $document_root$fastcgi_script_name`. That only resolves for a root-anchored app served from the server-level `root`; for an aliased subpath app the expression points at the server root rather than the app fileroot, so php-fpm cannot find the script. When no app is root-anchored there is no `root` at all and every subpath app is unservable without manual edits. Wrap each app in a `^~` prefix location and nest its php handler so SCRIPT_FILENAME reflects the app's anchoring: $document_root$fastcgi_script_name for root-anchored apps, $request_filename (which honours the alias) for subpath apps. Forbid blocks and the front-controller try_files fallback move inside the same location, preserving dispatch order. Add unit tests for the previously untested WebserverConfig services: NginxEmitter, AppMapBuilder, GenerationContext and WebserverConfigOptions.
Contributor
Author
|
@ralflang basically my review |
Member
|
I'm sceptical of this analysis. I think I'd better park this one until we actually do have the nginx CI pipeline. |
ralflang
marked this pull request as draft
July 8, 2026 21:20
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
2 participants
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
SCRIPT_FILENAMEcorrectly for aliased locations.WebserverConfigservices introduced in feat(webserver-config): registry-driven vhost / htaccess / nginx generator #17.Motivation
#17 emitted a single server-scope
location ~ \.php$usingSCRIPT_FILENAME $document_root$fastcgi_script_name. For an aliasedsubpath app (e.g.
/imp/)$document_rootpoints at the server root,not the app fileroot, so php-fpm receives a wrong path and cannot run the
front controller. With no root-anchored app there is no
rootdirectiveat all, leaving every subpath app unservable. Apache was unaffected; CI
only served the Apache flavor, so this went uncaught.
Changes
NginxEmitter: wrap each app in a^~prefix location; nest the PHPhandler per app with
$document_root$fastcgi_script_name(root-anchored)or
$request_filename(subpath, honours the alias). Forbid blocks andthe
try_filesfallback move inside the same location; the redundantserver-scope handler is removed. Banner documents the PATH_INFO caveat.
NginxEmitterTest,AppMapBuilderTest,GenerationContextTest,WebserverConfigOptionsTest.Test plan
vendor/bin/phpunitfull suite green (195 tests)$request_filenamefor subpath,$document_rootfor root-anchored, nested forbid blocks, and no server-scope handler
php-fpm serves the front controller