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

AlliBalliBaba · 2026-03-29T12:56:59Z

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 = filepath.ToSlash(strings.TrimPrefix(fc.worker.fileName, fc.documentRoot))


Can you also add a test for when the worker is outside of the public path (match)? Thinking about it again, scriptname should be empty since forwarding an absoule path when expecting a relative one is probably worse than forwarding nothing.

Happy to add the test, but I'm not sure if I agree with passing nothing being the correct choice. We should probably agree on what the correct behaviour is first.

'SCRIPT_NAME'
Contains the current script's path. This is useful for pages which need to point to themselves. The FILE constant contains the full path and filename of the current (i.e. included) file.

If anything, "This is useful for pages which need to point to themselves." would mean that we should perhaps just give the current request uri. But on the other hand, "Contains the current script's path." would mean it should be the absolute path, rather than a uri.

It's still expected to be a relative path, so I'd rather prevent potential redirects like this (and just leave it empty):

/path-the-user-visits -> /app/vendor/some-framework/worker.php

Realistically, most worker mode implementations probably just ignore SCRIPT_NAME.

I don't know, that doesn't feel right to me. I've changed it to what you suggested either way. Nobody should be using these server vars anyway.

…r root

AlliBalliBaba · 2026-04-04T17:52:17Z

cgi.go

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


Hmm I think if a worker is explicitly assigned, we should also ensure that the .php file is the actual worker file before determining pathInfo.

In case of a match *?

Yeah, we probably shouldn't allow pathname from a random script name like:

/random-script.php/pathname

Probably best to just leave it empty if script_name is empty.

It's a bit confusing because in all other cases the script name is guaranteed to be there because of rewrites.

went for a simple if check in order to not introduce runtime another check
it's probably very unlikely to to a different worker file in the public document root

cgi.go

caddy/caddy_test.go

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

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

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

henderkes commented Apr 5, 2026

View reviewed changes

cgi.go 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

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!

AlliBalliBaba Mar 29, 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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

henderkes commented Mar 27, 2026 •

edited

Loading

AlliBalliBaba Mar 29, 2026 •

edited

Loading