Skip to content

Commit 2da08d6

Browse files
henderkesCopilot
henderkes
and
Copilot
authored
fix: set $_SERVER variables: 'SCRIPT_NAME', 'PHP_SELF', and 'PATH_INFO' (#2317)
fixes #2274 (comment) closes #1133 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 ``` --------- Signed-off-by: Marc <m@pyc.ac> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent d5491f1 commit 2da08d6

File tree

7 files changed

+243
-22
lines changed

7 files changed

+243
-22
lines changed

‎.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
/caddy/frankenphp/Build
2+
/caddy/frankenphp/Caddyfile.test
23
/caddy/frankenphp/frankenphp
34
/caddy/frankenphp/frankenphp.exe
5+
/caddy/frankenphp/public
46
/dist
57
/github_conf
68
/internal/testserver/testserver

‎caddy/caddy_test.go‎

Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -486,6 +486,195 @@ func TestPHPServerDirectiveDisableFileServer(t *testing.T) {
486486
tester.AssertGetResponse("http://localhost:"+testPort+"/not-found.txt", http.StatusOK, "I am by birth a Genevese (i not set)")
487487
}
488488

489+
func TestPHPServerGlobals(t *testing.T) {
490+
documentRoot, _ := filepath.Abs("../testdata")
491+
scriptFilename := filepath.Join(documentRoot, "server-globals.php")
492+
493+
tester := caddytest.NewTester(t)
494+
initServer(t, tester, `
495+
{
496+
skip_install_trust
497+
admin localhost:2999
498+
http_port `+testPort+`
499+
https_port 9443
500+
}
501+
502+
localhost:`+testPort+` {
503+
root ../testdata
504+
php_server {
505+
index server-globals.php
506+
}
507+
}
508+
`, "caddyfile")
509+
510+
// Request to /en: no matching file, falls through to server-globals.php worker
511+
// SCRIPT_NAME should be /server-globals.php, PHP_SELF should be /server-globals.php (no /en), PATH_INFO empty
512+
tester.AssertGetResponse(
513+
"http://localhost:"+testPort+"/en",
514+
http.StatusOK,
515+
fmt.Sprintf(`SCRIPT_NAME: /server-globals.php
516+
SCRIPT_FILENAME: %s
517+
PHP_SELF: /server-globals.php
518+
PATH_INFO:
519+
DOCUMENT_ROOT: %s
520+
DOCUMENT_URI: /server-globals.php
521+
REQUEST_URI: /en
522+
`, scriptFilename, documentRoot),
523+
)
524+
525+
// Request to /server-globals.php/en: explicit PHP file with path info
526+
// SCRIPT_NAME should be /server-globals.php, PHP_SELF should be /server-globals.php/en, PATH_INFO should be /en
527+
tester.AssertGetResponse(
528+
"http://localhost:"+testPort+"/server-globals.php/en",
529+
http.StatusOK,
530+
fmt.Sprintf(`SCRIPT_NAME: /server-globals.php
531+
SCRIPT_FILENAME: %s
532+
PHP_SELF: /server-globals.php/en
533+
PATH_INFO: /en
534+
DOCUMENT_ROOT: %s
535+
DOCUMENT_URI: /server-globals.php
536+
REQUEST_URI: /server-globals.php/en
537+
`, scriptFilename, documentRoot),
538+
)
539+
}
540+
541+
func TestWorkerPHPServerGlobals(t *testing.T) {
542+
documentRoot, _ := filepath.Abs("../testdata")
543+
documentRoot2, _ := filepath.Abs("../caddy")
544+
scriptFilename := documentRoot + string(filepath.Separator) + "server-globals.php"
545+
testPortNum, _ := strconv.Atoi(testPort)
546+
testPortTwo := strconv.Itoa(testPortNum + 1)
547+
testPortThree := strconv.Itoa(testPortNum + 2)
548+
549+
tester := caddytest.NewTester(t)
550+
initServer(t, tester, `
551+
{
552+
skip_install_trust
553+
admin localhost:2999
554+
555+
frankenphp {
556+
worker {
557+
file ../testdata/server-globals.php
558+
num 1
559+
}
560+
}
561+
}
562+
563+
http://localhost:`+testPort+` {
564+
php_server {
565+
root ../testdata
566+
index server-globals.php
567+
}
568+
}
569+
570+
http://localhost:`+testPortTwo+` {
571+
php_server {
572+
root ../testdata
573+
index server-globals.php
574+
worker {
575+
file server-globals.php
576+
num 1
577+
}
578+
}
579+
}
580+
581+
http://localhost:`+testPortThree+` {
582+
php_server {
583+
root ./
584+
index server-globals.php
585+
worker {
586+
file ../testdata/server-globals.php
587+
num 1
588+
match *
589+
}
590+
}
591+
}
592+
`, "caddyfile")
593+
594+
// === Site 1: global worker with php_server ===
595+
// because we don't specify a php file, PATH_INFO should be empty
596+
tester.AssertGetResponse(
597+
"http://localhost:"+testPort+"/en",
598+
http.StatusOK,
599+
fmt.Sprintf(`SCRIPT_NAME: /server-globals.php
600+
SCRIPT_FILENAME: %s
601+
PHP_SELF: /server-globals.php
602+
PATH_INFO:
603+
DOCUMENT_ROOT: %s
604+
DOCUMENT_URI: /server-globals.php
605+
REQUEST_URI: /en
606+
`, scriptFilename, documentRoot),
607+
)
608+
609+
tester.AssertGetResponse(
610+
"http://localhost:"+testPort+"/server-globals.php/en",
611+
http.StatusOK,
612+
fmt.Sprintf(`SCRIPT_NAME: /server-globals.php
613+
SCRIPT_FILENAME: %s
614+
PHP_SELF: /server-globals.php/en
615+
PATH_INFO: /en
616+
DOCUMENT_ROOT: %s
617+
DOCUMENT_URI: /server-globals.php
618+
REQUEST_URI: /server-globals.php/en
619+
`, scriptFilename, documentRoot),
620+
)
621+
622+
// === Site 2: php_server with its own worker ===
623+
// because the request does not specify a php file, PATH_INFO should be empty
624+
tester.AssertGetResponse(
625+
"http://localhost:"+testPortTwo+"/en",
626+
http.StatusOK,
627+
fmt.Sprintf(`SCRIPT_NAME: /server-globals.php
628+
SCRIPT_FILENAME: %s
629+
PHP_SELF: /server-globals.php
630+
PATH_INFO:
631+
DOCUMENT_ROOT: %s
632+
DOCUMENT_URI: /server-globals.php
633+
REQUEST_URI: /en
634+
`, scriptFilename, documentRoot),
635+
)
636+
637+
tester.AssertGetResponse(
638+
"http://localhost:"+testPortTwo+"/server-globals.php/en",
639+
http.StatusOK,
640+
fmt.Sprintf(`SCRIPT_NAME: /server-globals.php
641+
SCRIPT_FILENAME: %s
642+
PHP_SELF: /server-globals.php/en
643+
PATH_INFO: /en
644+
DOCUMENT_ROOT: %s
645+
DOCUMENT_URI: /server-globals.php
646+
REQUEST_URI: /server-globals.php/en
647+
`, scriptFilename, documentRoot),
648+
)
649+
650+
// === Site 3: php_server with its own match worker ===
651+
tester.AssertGetResponse(
652+
"http://localhost:"+testPortThree+"/en",
653+
http.StatusOK,
654+
fmt.Sprintf(`SCRIPT_NAME:
655+
SCRIPT_FILENAME: %s
656+
PHP_SELF:
657+
PATH_INFO:
658+
DOCUMENT_ROOT: %s
659+
DOCUMENT_URI:
660+
REQUEST_URI: /en
661+
`, scriptFilename, documentRoot2),
662+
)
663+
664+
tester.AssertGetResponse(
665+
"http://localhost:"+testPortThree+"/server-globals.php/en",
666+
http.StatusOK,
667+
fmt.Sprintf(`SCRIPT_NAME:
668+
SCRIPT_FILENAME: %s
669+
PHP_SELF:
670+
PATH_INFO:
671+
DOCUMENT_ROOT: %s
672+
DOCUMENT_URI:
673+
REQUEST_URI: /server-globals.php/en
674+
`, scriptFilename, documentRoot2),
675+
)
676+
}
677+
489678
func TestMetrics(t *testing.T) {
490679
var wg sync.WaitGroup
491680
tester := caddytest.NewTester(t)

‎caddy/module.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error)
559559
}),
560560
}
561561
rewriteHandler := rewrite.Rewrite{
562-
URI: "{http.matchers.file.relative}",
562+
URI: "{http.matchers.file.relative}{http.matchers.file.remainder}",
563563
}
564564
rewriteRoute := caddyhttp.Route{
565565
MatcherSetsRaw: []caddy.ModuleMap{rewriteMatcherSet},
@@ -573,7 +573,7 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error)
573573
// match only requests that are for PHP files
574574
var pathList []string
575575
for _, ext := range extensions {
576-
pathList = append(pathList, "*"+ext)
576+
pathList = append(pathList, "*"+ext, "*"+ext+"/*")
577577
}
578578
phpMatcherSet := caddy.ModuleMap{
579579
"path": h.JSON(pathList),

‎caddy/php-server.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ func cmdPHPServer(fs caddycmd.Flags) (int, error) {
171171
}, nil),
172172
}
173173
rewriteHandler := rewrite.Rewrite{
174-
URI: "{http.matchers.file.relative}",
174+
URI: "{http.matchers.file.relative}{http.matchers.file.remainder}",
175175
}
176176
rewriteRoute := caddyhttp.Route{
177177
MatcherSetsRaw: []caddy.ModuleMap{rewriteMatcherSet},
@@ -182,7 +182,7 @@ func cmdPHPServer(fs caddycmd.Flags) (int, error) {
182182
// match only requests that are for PHP files
183183
var pathList []string
184184
for _, ext := range extensions {
185-
pathList = append(pathList, "*"+ext)
185+
pathList = append(pathList, "*"+ext, "*"+ext+"/*")
186186
}
187187
phpMatcherSet := caddy.ModuleMap{
188188
"path": caddyconfig.JSON(pathList, nil),

‎cgi.go‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ func addKnownVariablesToServer(fc *frankenPHPContext, trackVarsArray *C.zval) {
111111
requestURI = fc.requestURI
112112
}
113113

114-
requestPath := ensureLeadingSlash(request.URL.Path)
114+
phpSelf := fc.scriptName + fc.pathInfo
115115

116116
C.frankenphp_register_server_vars(trackVarsArray, C.frankenphp_server_vars{
117117
// approximate total length to avoid array re-hashing:
@@ -129,8 +129,8 @@ func addKnownVariablesToServer(fc *frankenPHPContext, trackVarsArray *C.zval) {
129129
document_root_len: C.size_t(len(fc.documentRoot)),
130130
path_info: toUnsafeChar(fc.pathInfo),
131131
path_info_len: C.size_t(len(fc.pathInfo)),
132-
php_self: toUnsafeChar(requestPath),
133-
php_self_len: C.size_t(len(requestPath)),
132+
php_self: toUnsafeChar(phpSelf),
133+
php_self_len: C.size_t(len(phpSelf)),
134134
document_uri: toUnsafeChar(fc.docURI),
135135
document_uri_len: C.size_t(len(fc.docURI)),
136136
script_filename: toUnsafeChar(fc.scriptFilename),
@@ -208,17 +208,26 @@ func splitCgiPath(fc *frankenPHPContext) {
208208
if splitPos := splitPos(path, splitPath); splitPos > -1 {
209209
fc.docURI = path[:splitPos]
210210
fc.pathInfo = path[splitPos:]
211+
}
211212

212-
// Strip PATH_INFO from SCRIPT_NAME
213-
fc.scriptName = strings.TrimSuffix(path, fc.pathInfo)
214-
215-
// Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875
216-
// Info: https://tools.ietf.org/html/rfc3875#section-4.1.13
217-
if fc.scriptName != "" && !strings.HasPrefix(fc.scriptName, "/") {
218-
fc.scriptName = "/" + fc.scriptName
213+
// If a worker is already assigned explicitly, derive SCRIPT_NAME from its filename
214+
if fc.worker != nil {
215+
fc.scriptFilename = fc.worker.fileName
216+
docRootWithSep := fc.documentRoot + string(filepath.Separator)
217+
if strings.HasPrefix(fc.worker.fileName, docRootWithSep) {
218+
fc.scriptName = filepath.ToSlash(strings.TrimPrefix(fc.worker.fileName, fc.documentRoot))
219+
} else {
220+
fc.docURI = ""
221+
fc.pathInfo = ""
219222
}
223+
return
220224
}
221225

226+
// Strip PATH_INFO from SCRIPT_NAME
227+
// Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875
228+
// Info: https://tools.ietf.org/html/rfc3875#section-4.1.13
229+
fc.scriptName = ensureLeadingSlash(strings.TrimSuffix(path, fc.pathInfo))
230+
222231
// TODO: is it possible to delay this and avoid saving everything in the context?
223232
// SCRIPT_FILENAME is the absolute path of SCRIPT_NAME
224233
fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName)

‎context.go‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,14 +86,7 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques
8686
}
8787
}
8888

89-
// If a worker is already assigned explicitly, use its filename and skip parsing path variables
90-
if fc.worker != nil {
91-
fc.scriptFilename = fc.worker.fileName
92-
} else {
93-
// If no worker was assigned, split the path into the "traditional" CGI path variables.
94-
// This needs to already happen here in case a worker script still matches the path.
95-
splitCgiPath(fc)
96-
}
89+
splitCgiPath(fc)
9790

9891
fc.requestURI = r.URL.RequestURI()
9992

‎testdata/server-globals.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
ignore_user_abort(true);
4+
5+
$server_vars = [
6+
'SCRIPT_NAME',
7+
'SCRIPT_FILENAME',
8+
'PHP_SELF',
9+
'PATH_INFO',
10+
'DOCUMENT_ROOT',
11+
'DOCUMENT_URI',
12+
'REQUEST_URI',
13+
];
14+
$handler = static function() use ($server_vars) {
15+
foreach ($server_vars as $var) {
16+
$value = $_SERVER[$var] ?? '(not set)';
17+
echo $value !== '' ? "$var: $value\n" : "$var:\n";
18+
}
19+
};
20+
21+
if (isset($_SERVER['FRANKENPHP_WORKER'])) {
22+
for ($nbRequests = 0, $running = true; $running; ++$nbRequests) {
23+
$running = \frankenphp_handle_request($handler);
24+
gc_collect_cycles();
25+
}
26+
} else {
27+
$handler();
28+
}

0 commit comments

Comments
 (0)