Skip to content
Open
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
8 changes: 4 additions & 4 deletions .github/workflows/sanitizers.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -102,13 +102,13 @@ jobs:
run: echo "$(pwd)/php/target/bin" >> "$GITHUB_PATH"
- name: Install e-dant/watcher
uses: ./.github/actions/watcher
- name: Set Set CGO flags
- name: Install gotestsum
run: go install gotest.tools/gotestsum@latest
- name: Set CGO flags
run: |
{
echo "CGO_CFLAGS=$CFLAGS -I${PWD}/watcher/target/include $(php-config --includes)"
echo "CGO_LDFLAGS=$LDFLAGS $(php-config --ldflags) $(php-config --libs)"
} >> "$GITHUB_ENV"
- name: Compile tests
run: go test ${{ matrix.sanitizer == 'msan' && '-tags=nowatcher' || '' }} -${{ matrix.sanitizer }} -v -x -c
- name: Run tests
run: ./frankenphp.test -test.v
run: gotestsum -- ${{ matrix.sanitizer == 'msan' && '-tags=nowatcher' || '' }} -${{ matrix.sanitizer }} ./...
20 changes: 12 additions & 8 deletions .github/workflows/tests.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,13 +64,13 @@ jobs:
- name: Build testcli binary
working-directory: internal/testcli/
run: go build
- name: Compile library tests
run: go test -race -v -x -c
- name: Install gotestsum
run: go install gotest.tools/gotestsum@latest
- name: Run library tests
run: ./frankenphp.test -test.v
run: gotestsum -- -race ./...
- name: Run Caddy module tests
working-directory: caddy/
run: go test -race -v ./...
run: gotestsum -- -race ./...
- name: Run Fuzzing Tests
working-directory: caddy/
run: go test -fuzz FuzzRequest -fuzztime 20s
Expand Down Expand Up @@ -136,9 +136,11 @@ jobs:
run: |
echo "CGO_CFLAGS=$(php-config --includes)" >> "${GITHUB_ENV}"
echo "CGO_LDFLAGS=$(php-config --ldflags) $(php-config --libs)" >> "${GITHUB_ENV}"
- name: Install gotestsum
run: go install gotest.tools/gotestsum@latest
- name: Run integration tests
working-directory: internal/extgen/
run: go test -tags integration -v -timeout 30m
run: gotestsum -- -tags integration -timeout 30m
tests-mac:
name: Tests (macOS, PHP 8.5)
runs-on: macos-latest
Expand All @@ -164,7 +166,9 @@ jobs:
env:
phpts: ts
debug: true
- name: Set Set CGO flags
- name: Install gotestsum
run: go install gotest.tools/gotestsum@latest
- name: Set CGO flags
run: |
{
echo "CGO_CFLAGS=-I/opt/homebrew/include/ $(php-config --includes)"
Expand All @@ -173,7 +177,7 @@ jobs:
- name: Build
run: go build -tags nowatcher
- name: Run library tests
run: go test -tags nowatcher -race -v ./...
run: gotestsum -- -tags nowatcher -race ./...
- name: Run Caddy module tests
working-directory: caddy/
run: go test -race -v ./...
run: gotestsum -- -race ./...
5 changes: 3 additions & 2 deletions .github/workflows/windows.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,10 @@ jobs:
"opcache.enable=0`r`nopcache.enable_cli=0" | Out-File php.ini
$env:PHPRC = Get-Location

go test -race ./...
go install gotest.tools/gotestsum@latest
gotestsum -- -race ./...
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
cd caddy
go test -race ./...
gotestsum -- -race ./...
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
working-directory: ${{ github.workspace }}\frankenphp
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/caddy/frankenphp/Build
Copy link
Copy Markdown
Member

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

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
173 changes: 173 additions & 0 deletions caddy/caddy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,179 @@ func TestPHPServerDirectiveDisableFileServer(t *testing.T) {
tester.AssertGetResponse("http://localhost:"+testPort+"/not-found.txt", http.StatusOK, "I am by birth a Genevese (i not set)")
}

func TestPHPServerGlobals(t *testing.T) {
documentRoot, _ := filepath.Abs("../testdata")
scriptFilename := filepath.Join(documentRoot, "server-globals.php")

tester := caddytest.NewTester(t)
initServer(t, tester, `
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`
https_port 9443
}

localhost:`+testPort+` {
root ../testdata
php_server {
index server-globals.php
}
}
`, "caddyfile")

// Request to /en: no matching file, falls through to server-globals.php worker
// SCRIPT_NAME should be /server-globals.php, PHP_SELF should be /server-globals.php (no /en), PATH_INFO empty
tester.AssertGetResponse(
"http://localhost:"+testPort+"/en",
http.StatusOK,
"SCRIPT_NAME: /server-globals.php<br>"+
"SCRIPT_FILENAME: "+scriptFilename+"<br>"+
"PHP_SELF: /server-globals.php<br>"+
"PATH_INFO: <br>"+
"DOCUMENT_ROOT: "+documentRoot+"<br>"+
"REQUEST_URI: /en<br>",
)

// Request to /server-globals.php/en: explicit PHP file with path info
// SCRIPT_NAME should be /server-globals.php, PHP_SELF should be /server-globals.php/en, PATH_INFO should be /en
tester.AssertGetResponse(
"http://localhost:"+testPort+"/server-globals.php/en",
http.StatusOK,
"SCRIPT_NAME: /server-globals.php<br>"+
"SCRIPT_FILENAME: "+scriptFilename+"<br>"+
"PHP_SELF: /server-globals.php/en<br>"+
"PATH_INFO: /en<br>"+
"DOCUMENT_ROOT: "+documentRoot+"<br>"+
"REQUEST_URI: /server-globals.php/en<br>",
)
}

func TestWorkerPHPServerGlobals(t *testing.T) {
documentRoot, _ := filepath.Abs("../testdata")
documentRoot2, _ := filepath.Abs("../caddy")
scriptFilename := documentRoot + string(filepath.Separator) + "server-globals.php"
testPortNum, _ := strconv.Atoi(testPort)
testPortTwo := strconv.Itoa(testPortNum + 1)
testPortThree := strconv.Itoa(testPortNum + 2)

tester := caddytest.NewTester(t)
initServer(t, tester, `
{
skip_install_trust
admin localhost:2999

frankenphp {
worker {
file ../testdata/server-globals.php
num 1
}
}
}

http://localhost:`+testPort+` {
php_server {
root ../testdata
index server-globals.php
}
}

http://localhost:`+testPortTwo+` {
php_server {
root ../testdata
index server-globals.php
worker {
file server-globals.php
num 1
}
}
}

http://localhost:`+testPortThree+` {
php_server {
root ./
index server-globals.php
worker {
file ../testdata/server-globals.php
num 1
match *
}
}
}
`, "caddyfile")

// === Site 1: global worker with php_server ===
// because we don't specify a php file, PATH_INFO should be empty
tester.AssertGetResponse(
"http://localhost:"+testPort+"/en",
http.StatusOK,
"SCRIPT_NAME: /server-globals.php<br>"+
"SCRIPT_FILENAME: "+scriptFilename+"<br>"+
"PHP_SELF: /server-globals.php<br>"+
"PATH_INFO: <br>"+
"DOCUMENT_ROOT: "+documentRoot+"<br>"+
"REQUEST_URI: /en<br>",
)

tester.AssertGetResponse(
"http://localhost:"+testPort+"/server-globals.php/en",
http.StatusOK,
"SCRIPT_NAME: /server-globals.php<br>"+
"SCRIPT_FILENAME: "+scriptFilename+"<br>"+
"PHP_SELF: /server-globals.php/en<br>"+
"PATH_INFO: /en<br>"+
"DOCUMENT_ROOT: "+documentRoot+"<br>"+
"REQUEST_URI: /server-globals.php/en<br>",
)

// === Site 2: php_server with its own worker ===
// because the request does not specify a php file, PATH_INFO should be empty
tester.AssertGetResponse(
"http://localhost:"+testPortTwo+"/en",
http.StatusOK,
"SCRIPT_NAME: /server-globals.php<br>"+
"SCRIPT_FILENAME: "+scriptFilename+"<br>"+
"PHP_SELF: /server-globals.php<br>"+
"PATH_INFO: <br>"+
"DOCUMENT_ROOT: "+documentRoot+"<br>"+
"REQUEST_URI: /en<br>",
)

tester.AssertGetResponse(
"http://localhost:"+testPortTwo+"/server-globals.php/en",
http.StatusOK,
"SCRIPT_NAME: /server-globals.php<br>"+
"SCRIPT_FILENAME: "+scriptFilename+"<br>"+
"PHP_SELF: /server-globals.php/en<br>"+
"PATH_INFO: /en<br>"+
"DOCUMENT_ROOT: "+documentRoot+"<br>"+
"REQUEST_URI: /server-globals.php/en<br>",
)

// === Site 3: php_server with its own match worker ===
tester.AssertGetResponse(
"http://localhost:"+testPortThree+"/en",
http.StatusOK,
"SCRIPT_NAME: <br>"+
"SCRIPT_FILENAME: "+scriptFilename+"<br>"+
"PHP_SELF: <br>"+
"PATH_INFO: <br>"+
"DOCUMENT_ROOT: "+documentRoot2+"<br>"+
"REQUEST_URI: /en<br>",
)

tester.AssertGetResponse(
"http://localhost:"+testPortThree+"/server-globals.php/en",
http.StatusOK,
"SCRIPT_NAME: <br>"+
"SCRIPT_FILENAME: "+scriptFilename+"<br>"+
"PHP_SELF: <br>"+
"PATH_INFO: <br>"+
"DOCUMENT_ROOT: "+documentRoot2+"<br>"+
"REQUEST_URI: /server-globals.php/en<br>",
)
}

func TestMetrics(t *testing.T) {
var wg sync.WaitGroup
tester := caddytest.NewTester(t)
Expand Down
4 changes: 2 additions & 2 deletions caddy/module.go
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}",
}
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
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
28 changes: 18 additions & 10 deletions cgi.go
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,17 +208,25 @@ func splitCgiPath(fc *frankenPHPContext) {
if splitPos := splitPos(path, splitPath); splitPos > -1 {
fc.docURI = path[:splitPos]
Copy link
Copy Markdown
Contributor

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

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:]
}

// 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
// If a worker is already assigned explicitly, derive SCRIPT_NAME from its filename
if fc.worker != nil {
fc.scriptFilename = fc.worker.fileName
docRootWithSep := fc.documentRoot + string(filepath.Separator)
if strings.HasPrefix(fc.worker.fileName, docRootWithSep) {
fc.scriptName = filepath.ToSlash(strings.TrimPrefix(fc.worker.fileName, fc.documentRoot))
} else {
fc.pathInfo = ""
}
return
}

// Strip PATH_INFO from SCRIPT_NAME
// Ensure the SCRIPT_NAME has a leading slash for compliance with RFC3875
// Info: https://tools.ietf.org/html/rfc3875#section-4.1.13
fc.scriptName = ensureLeadingSlash(strings.TrimSuffix(path, fc.pathInfo))

// TODO: is it possible to delay this and avoid saving everything in the context?
// SCRIPT_FILENAME is the absolute path of SCRIPT_NAME
fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName)
Expand Down
9 changes: 1 addition & 8 deletions context.go
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
21 changes: 21 additions & 0 deletions testdata/server-globals.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

ignore_user_abort(true);

$handler = static function() {
echo "SCRIPT_NAME: " . ($_SERVER['SCRIPT_NAME'] ?? '(not set)') . "<br>";
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.

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

echo "SCRIPT_FILENAME: " . ($_SERVER['SCRIPT_FILENAME'] ?? '(not set)') . "<br>";
echo "PHP_SELF: " . ($_SERVER['PHP_SELF'] ?? '(not set)') . "<br>";
echo "PATH_INFO: " . ($_SERVER['PATH_INFO'] ?? '(not set)') . "<br>";
echo "DOCUMENT_ROOT: " . ($_SERVER['DOCUMENT_ROOT'] ?? '(not set)') . "<br>";
echo "REQUEST_URI: " . ($_SERVER['REQUEST_URI'] ?? '(not set)') . "<br>";
};

if (isset($_SERVER['FRANKENPHP_WORKER'])) {
for ($nbRequests = 0, $running = true; $running; ++$nbRequests) {
$running = \frankenphp_handle_request($handler);
gc_collect_cycles();
}
} else {
$handler();
}
Loading