Skip to content

fix: set $_SERVER variables: 'SCRIPT_NAME', 'PHP_SELF', and 'PATH_INFO' #2317

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
henderkes merged 20 commits into from
Apr 20, 2026
Merged

fix: set $_SERVER variables: 'SCRIPT_NAME', 'PHP_SELF', and 'PATH_INFO' #2317

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
708b233
fix $_SERVER['SCRIPT_NAME'] and $_SERVER['PHP_SELF'] in php_server wo…
henderkes Mar 27, 2026
1935dc9
-failfast for tests, so we can actually see what failed
henderkes Mar 27, 2026
1eb691d
fix pathInfo being empty
henderkes Mar 27, 2026
3708b41
add tests (worker and non-worker separately because the global worker…
henderkes Mar 28, 2026
2a32861
fix on windows
henderkes Mar 28, 2026
652322b
use existing ensureLeadingSlash function to
henderkes Mar 28, 2026
fd6595e
test for match * worker
henderkes Mar 29, 2026
b8a981d
@AlliBalliBaba suggestion, empty out script_name if it's not in serve…
henderkes Mar 29, 2026
91a6968
Apply suggestions from code review
henderkes Apr 5, 2026
0a294ac
Merge branch 'main' into fix/script_name
henderkes Apr 5, 2026
b632a5e
safety check for separator, in case someone uses matchers
henderkes Apr 8, 2026
c5481d0
Update caddy/caddy_test.go
henderkes Apr 8, 2026
6fc5c15
Update caddy/caddy_test.go
henderkes Apr 8, 2026
7375dce
use gotestsum for tests (shows all failed test outputs only)
henderkes Apr 8, 2026
2dc7270
use gotestsum for other tests too (except fuzz test)
henderkes Apr 8, 2026
44519f5
Merge branch 'main' into fix/script_name
henderkes Apr 8, 2026
5f9c77f
fix "Set Set CGO flags"
henderkes Apr 8, 2026
786b6be
reset CI test changes, refactor out to next PR
henderkes Apr 8, 2026
5fb29c7
suggestions
henderkes Apr 10, 2026
10dcc5a
Merge branch 'main' into fix/script_name
henderkes Apr 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions .github/workflows/tests.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ jobs:
working-directory: internal/testcli/
run: go build
- name: Compile library tests
run: go test -race -v -x -c
run: go test -race -failfast -v -x -c
Comment thread
henderkes marked this conversation as resolved.
Outdated
- name: Run library tests
run: ./frankenphp.test -test.v
run: ./frankenphp.test -test.v -test.failfast
- name: Run Caddy module tests
working-directory: caddy/
run: go test -race -v ./...
run: go test -race -failfast -v ./...
- name: Run Fuzzing Tests
working-directory: caddy/
run: go test -fuzz FuzzRequest -fuzztime 20s
Expand Down Expand Up @@ -173,7 +173,7 @@ jobs:
- name: Build
run: go build -tags nowatcher
- name: Run library tests
run: go test -tags nowatcher -race -v ./...
run: go test -tags nowatcher -race -failfast -v ./...
- name: Run Caddy module tests
working-directory: caddy/
run: go test -race -v ./...
run: go test -race -failfast -v ./...
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/caddy/frankenphp/Build
Copy link
Copy Markdown
Member

@dunglas dunglas Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes to revert?

Copy link
Copy Markdown
Contributor Author

@henderkes henderkes Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

/caddy/frankenphp/Caddyfile.test
/caddy/frankenphp/frankenphp
/caddy/frankenphp/frankenphp.exe
/caddy/frankenphp/public
/dist
/github_conf
/internal/testserver/testserver
Expand Down
4 changes: 2 additions & 2 deletions caddy/module.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -559,7 +559,7 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error)
}),
}
rewriteHandler := rewrite.Rewrite{
URI: "{http.matchers.file.relative}",
URI: "{http.matchers.file.relative}{http.matchers.file.remainder}",
Comment thread
henderkes marked this conversation as resolved.
}
rewriteRoute := caddyhttp.Route{
MatcherSetsRaw: []caddy.ModuleMap{rewriteMatcherSet},
Expand All @@ -573,7 +573,7 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error)
// match only requests that are for PHP files
var pathList []string
for _, ext := range extensions {
pathList = append(pathList, "*"+ext)
pathList = append(pathList, "*"+ext, "*"+ext+"/*")
}
phpMatcherSet := caddy.ModuleMap{
"path": h.JSON(pathList),
Expand Down
4 changes: 2 additions & 2 deletions caddy/php-server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func cmdPHPServer(fs caddycmd.Flags) (int, error) {
}, nil),
}
rewriteHandler := rewrite.Rewrite{
URI: "{http.matchers.file.relative}",
URI: "{http.matchers.file.relative}{http.matchers.file.remainder}",
}
rewriteRoute := caddyhttp.Route{
MatcherSetsRaw: []caddy.ModuleMap{rewriteMatcherSet},
Expand All @@ -182,7 +182,7 @@ func cmdPHPServer(fs caddycmd.Flags) (int, error) {
// match only requests that are for PHP files
var pathList []string
for _, ext := range extensions {
pathList = append(pathList, "*"+ext)
pathList = append(pathList, "*"+ext, "*"+ext+"/*")
}
phpMatcherSet := caddy.ModuleMap{
"path": caddyconfig.JSON(pathList, nil),
Expand Down
27 changes: 17 additions & 10 deletions cgi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ func addKnownVariablesToServer(fc *frankenPHPContext, trackVarsArray *C.zval) {
requestURI = fc.requestURI
}

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

C.frankenphp_register_server_vars(trackVarsArray, C.frankenphp_server_vars{
// approximate total length to avoid array re-hashing:
Expand All @@ -129,8 +129,8 @@ func addKnownVariablesToServer(fc *frankenPHPContext, trackVarsArray *C.zval) {
document_root_len: C.size_t(len(fc.documentRoot)),
path_info: toUnsafeChar(fc.pathInfo),
path_info_len: C.size_t(len(fc.pathInfo)),
php_self: toUnsafeChar(requestPath),
php_self_len: C.size_t(len(requestPath)),
php_self: toUnsafeChar(phpSelf),
php_self_len: C.size_t(len(phpSelf)),
document_uri: toUnsafeChar(fc.docURI),
document_uri_len: C.size_t(len(fc.docURI)),
script_filename: toUnsafeChar(fc.scriptFilename),
Expand Down Expand Up @@ -208,15 +208,22 @@ func splitCgiPath(fc *frankenPHPContext) {
if splitPos := splitPos(path, splitPath); splitPos > -1 {
fc.docURI = path[:splitPos]
Copy link
Copy Markdown
Contributor

@AlliBalliBaba AlliBalliBaba Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

@henderkes henderkes Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

fc.pathInfo = path[splitPos:]
Comment thread
AlliBalliBaba marked this conversation as resolved.
}

// 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)
Copy link
Copy Markdown
Contributor

@AlliBalliBaba AlliBalliBaba Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Copy Markdown
Contributor Author

@henderkes henderkes Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor

@AlliBalliBaba AlliBalliBaba Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Copy Markdown
Contributor Author

@henderkes henderkes Mar 28, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return
}

// Strip PATH_INFO from SCRIPT_NAME
fc.scriptName = strings.TrimSuffix(path, fc.pathInfo)
// Strip PATH_INFO from SCRIPT_NAME
fc.scriptName = strings.TrimSuffix(path, fc.pathInfo)

// Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875
// Info: https://tools.ietf.org/html/rfc3875#section-4.1.13
if fc.scriptName != "" && !strings.HasPrefix(fc.scriptName, "/") {
fc.scriptName = "/" + fc.scriptName
}
// Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875
// Info: https://tools.ietf.org/html/rfc3875#section-4.1.13
if fc.scriptName != "" && !strings.HasPrefix(fc.scriptName, "/") {
fc.scriptName = "/" + fc.scriptName
}
Comment thread
henderkes marked this conversation as resolved.
Outdated

// TODO: is it possible to delay this and avoid saving everything in the context?
Expand Down
9 changes: 1 addition & 8 deletions context.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,14 +86,7 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques
}
}

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

fc.requestURI = r.URL.RequestURI()

Expand Down
Loading