fix: set $_SERVER variables: 'SCRIPT_NAME', 'PHP_SELF', and 'PATH_INFO' by henderkes · Pull Request #2317 · php/frankenphp

henderkes · 2026-03-27T15:49:37Z

apache/nginx/caddy pass PHP_SELF as SCRIPT_NAME + PATH_INFO, but our PATH_INFO wasn't working because our matcher stripped the rest of the path.

request url: localhost/index.php/en

# was non-worker:
SCRIPT_NAME: /index.php
PATH_INFO: 
PHP_SELF: /index.php
REQUEST_URL: /en

# was fastcgi:
SCRIPT_NAME: /index.php
PATH_INFO:  /en
PHP_SELF: /index.php/en
REQUEST_URL: /en

# was php_server worker
SCRIPT_NAME:
PATH_INFO:
PHP_SELF: /en
REQUEST_URL: /en

# now is always:
SCRIPT_NAME: /index.php
PATH_INFO: /en
PHP_SELF: /index.php/en
REQUEST_URL: /en

…rkers apache passes PHP_SELF as SCRIPT_NAME + REQUEST_URL, but the docs say it's the same as SCRIPT_NAME and that's how Caddy+fpm behave too

AlliBalliBaba · 2026-03-28T11:08:52Z

cgi.go

+	// If a worker is already assigned explicitly, derive SCRIPT_NAME from its filename
+	if fc.worker != nil {
+		fc.scriptFilename = fc.worker.fileName
+		fc.scriptName = strings.TrimPrefix(fc.worker.fileName, fc.documentRoot)


Hmm probably would be better to leave scriptName empty if the worker is in the public path:

root /some/path worker /other/path { match * }

I'm not sure about that. Shouldn't it show /other/path/index.php then?

Not sure either TBH. In CGI spec it's the path relative to the root, so /other/path/index.php might be misleading.

Technically that's a path relative to the root, haha.

I don't see a better solution. Emptying it out would be even more of a violation.

caddy/module.go

… would activate for the non-worker case too

cgi.go

…r root

cgi.go

caddy/caddy_test.go

Co-authored-by: Marc <m@pyc.ac> Signed-off-by: Marc <m@pyc.ac>

dunglas

LGTM, but I didn't carefully review the CGI spec nor existing Apache/FPM behaviors. I trust you on this

.github/workflows/tests.yaml

dunglas · 2026-04-08T13:02:26Z

testdata/server-globals.php

+ignore_user_abort(true);
+
+$handler = static function() {
+    echo "SCRIPT_NAME: " . ($_SERVER['SCRIPT_NAME'] ?? '(not set)') . "<br>";


Nit: you could add \n at the end of each lines, so you could use backticks for better readability in tests

dunglas · 2026-04-08T13:02:46Z

.gitignore

@@ -1,6 +1,8 @@
 /caddy/frankenphp/Build


Changes to revert?

added these because I often times find myself verifying bug reports there

I'm thinking they're fine in the .gitignore, but if you want I can revert

Copilot

Pull request overview

This PR fixes inconsistencies in $_SERVER CGI variables (notably SCRIPT_NAME, PHP_SELF, and PATH_INFO) in worker mode by ensuring path splitting happens consistently and by preserving “remainder” path segments through the Caddy rewrite/matching pipeline.

Changes:

Always split CGI path variables during request context creation, including in worker mode.
Set PHP_SELF from SCRIPT_NAME + PATH_INFO, and update CGI path splitting logic for worker-assigned requests.
Update Caddy php_server rewrite/match patterns to preserve remainder path info and add regression tests for server globals.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`context.go`	Always calls `splitCgiPath(fc)` to compute CGI path vars consistently.
`cgi.go`	Updates `PHP_SELF` derivation and refactors `splitCgiPath` to better support worker mode.
`caddy/php-server.go`	Preserves remainder in rewrite and matches `.php/` paths to support PATH_INFO.
`caddy/module.go`	Same Caddy rewrite/matcher adjustments for the directive parser output.
`caddy/caddy_test.go`	Adds coverage asserting expected globals for both worker and non-worker php_server setups.
`testdata/server-globals.php`	New test helper script that prints key `$_SERVER` variables.
`.github/workflows/tests.yaml`	Adds `-failfast` to speed up CI failure feedback.
`.gitignore`	Ignores additional generated Caddy test artifacts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

cgi.go

caddy/caddy_test.go

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Marc <m@pyc.ac>

alexandre-daubois

LGTM once Kevin's comments are addressed

AlliBalliBaba · 2026-04-08T18:01:30Z

cgi.go

@@ -208,17 +208,25 @@ func splitCgiPath(fc *frankenPHPContext) {
 	if splitPos := splitPos(path, splitPath); splitPos > -1 {
 		fc.docURI = path[:splitPos]


Just realized the same is true for documentUri (should be empty with match), probably makes sense to move the worker check before this block.

I didn't even know that's a server variable. Will move the check up later.

fix $_SERVER['SCRIPT_NAME'] and $_SERVER['PHP_SELF'] in php_server wo…

708b233

…rkers apache passes PHP_SELF as SCRIPT_NAME + REQUEST_URL, but the docs say it's the same as SCRIPT_NAME and that's how Caddy+fpm behave too

henderkes requested a review from AlliBalliBaba March 27, 2026 15:49

henderkes marked this pull request as ready for review March 27, 2026 15:49

-failfast for tests, so we can actually see what failed

1935dc9

henderkes marked this pull request as draft March 27, 2026 16:26

fix pathInfo being empty

1eb691d

henderkes changed the title ~~fix $_SERVER['SCRIPT_NAME'] and $_SERVER['PHP_SELF'] in php_server workers~~ fix $_SERVER['SCRIPT_NAME'], 'PHP_SELF', and 'PATH_INFO' Mar 27, 2026

henderkes marked this pull request as ready for review March 27, 2026 17:05

AlliBalliBaba reviewed Mar 28, 2026

View reviewed changes

caddy/module.go Show resolved Hide resolved

henderkes added 2 commits March 28, 2026 21:11

add tests (worker and non-worker separately because the global worker…

3708b41

… would activate for the non-worker case too

fix on windows

2a32861

henderkes commented Mar 28, 2026

View reviewed changes

cgi.go Outdated Show resolved Hide resolved

use existing ensureLeadingSlash function to

652322b

henderkes requested review from AlliBalliBaba and dunglas March 28, 2026 15:36

AlliBalliBaba reviewed Mar 29, 2026

View reviewed changes

cgi.go Outdated Show resolved Hide resolved

henderkes added 2 commits March 29, 2026 21:57

test for match * worker

fd6595e

@AlliBalliBaba suggestion, empty out script_name if it's not in serve…

b8a981d

…r root

henderkes changed the title ~~fix $_SERVER['SCRIPT_NAME'], 'PHP_SELF', and 'PATH_INFO'~~ fix: set $_SERVER variables: 'SCRIPT_NAME', 'PHP_SELF', and 'PATH_INFO' Mar 31, 2026

AlliBalliBaba reviewed Apr 4, 2026

View reviewed changes

cgi.go Show resolved Hide resolved

henderkes commented Apr 5, 2026

View reviewed changes

cgi.go Outdated Show resolved Hide resolved

henderkes commented Apr 5, 2026

View reviewed changes

caddy/caddy_test.go Outdated Show resolved Hide resolved

henderkes added 2 commits April 6, 2026 00:20

Apply suggestions from code review

91a6968

Co-authored-by: Marc <m@pyc.ac> Signed-off-by: Marc <m@pyc.ac>

Merge branch 'main' into fix/script_name

0a294ac

dunglas approved these changes Apr 8, 2026

View reviewed changes

dunglas requested a review from Copilot April 8, 2026 13:04

Copilot started reviewing on behalf of dunglas April 8, 2026 13:05 View session

Copilot AI reviewed Apr 8, 2026

View reviewed changes

cgi.go Outdated Show resolved Hide resolved

caddy/caddy_test.go Show resolved Hide resolved

caddy/caddy_test.go Outdated Show resolved Hide resolved

caddy/caddy_test.go Outdated Show resolved Hide resolved

safety check for separator, in case someone uses matchers

b632a5e

henderkes and others added 2 commits April 8, 2026 21:00

Update caddy/caddy_test.go

c5481d0

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Marc <m@pyc.ac>

Update caddy/caddy_test.go

6fc5c15

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Marc <m@pyc.ac>

alexandre-daubois reviewed Apr 8, 2026

View reviewed changes

alexandre-daubois approved these changes Apr 8, 2026

View reviewed changes

henderkes added 5 commits April 8, 2026 22:12

use gotestsum for tests (shows all failed test outputs only)

7375dce

use gotestsum for other tests too (except fuzz test)

2dc7270

Merge branch 'main' into fix/script_name

44519f5

fix "Set Set CGO flags"

5f9c77f

reset CI test changes, refactor out to next PR

786b6be

AlliBalliBaba reviewed Apr 8, 2026

View reviewed changes

		@@ -208,17 +208,25 @@ func splitCgiPath(fc *frankenPHPContext) {
		if splitPos := splitPos(path, splitPath); splitPos > -1 {
		fc.docURI = path[:splitPos]

Conversation

henderkes commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dunglas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dunglas Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexandre-daubois left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

henderkes commented Mar 27, 2026 •

edited

Loading

dunglas Apr 8, 2026 •

edited

Loading