Skip to content
250 changes: 250 additions & 0 deletions caddy/caddy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1500,3 +1500,253 @@ func TestLog(t *testing.T) {
"",
)
}

// TestSymlinkWorkerPaths tests different ways to reference worker scripts in symlinked directories
func TestSymlinkWorkerPaths(t *testing.T) {
cwd, _ := os.Getwd()
publicDir := filepath.Join(cwd, "..", "testdata", "symlinks", "public")

t.Run("NeighboringWorkerScript", func(t *testing.T) {
// Scenario: neighboring worker script
// Given frankenphp located in the test folder
// When I execute `frankenphp php-server --listen localhost:8080 -w index.php` from `public`
// Then I expect to see the worker script executed successfully
tester := caddytest.NewTester(t)
tester.InitServer(`
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`

frankenphp {
worker `+publicDir+`/index.php 1
}
}

localhost:`+testPort+` {
route {
php {
root `+publicDir+`
resolve_root_symlink true
}
}
}
`, "caddyfile")

tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "Request: 0\n")
})

t.Run("NestedWorkerScript", func(t *testing.T) {
// Scenario: nested worker script
// Given frankenphp located in the test folder
// When I execute `frankenphp --listen localhost:8080 -w nested/index.php` from `public`
// Then I expect to see the worker script executed successfully
tester := caddytest.NewTester(t)
tester.InitServer(`
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`

frankenphp {
worker `+publicDir+`/nested/index.php 1
}
}

localhost:`+testPort+` {
route {
php {
root `+publicDir+`
resolve_root_symlink true
}
}
}
`, "caddyfile")

tester.AssertGetResponse("http://localhost:"+testPort+"/nested/index.php", http.StatusOK, "Nested request: 0\n")
})

t.Run("OutsideSymlinkedFolder", func(t *testing.T) {
// Scenario: outside the symlinked folder
// Given frankenphp located in the root folder
// When I execute `frankenphp --listen localhost:8080 -w public/index.php` from the root folder
// Then I expect to see the worker script executed successfully
tester := caddytest.NewTester(t)
tester.InitServer(`
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`

frankenphp {
worker {
name outside_worker
file `+publicDir+`/index.php
num 1
}
}
}

localhost:`+testPort+` {
route {
php {
root `+publicDir+`
resolve_root_symlink true
}
}
}
`, "caddyfile")

tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "Request: 0\n")
})

t.Run("SpecifiedRootDirectory", func(t *testing.T) {
// Scenario: specified root directory
// Given frankenphp located in the root folder
// When I execute `frankenphp --listen localhost:8080 -w public/index.php -r public` from the root folder
// Then I expect to see the worker script executed successfully
tester := caddytest.NewTester(t)
tester.InitServer(`
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`

frankenphp {
worker {
name specified_root_worker
file `+publicDir+`/index.php
num 1
}
}
}

localhost:`+testPort+` {
route {
php {
root `+publicDir+`
resolve_root_symlink true
}
}
}
`, "caddyfile")

tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "Request: 0\n")
})
}

// TestSymlinkResolveRoot tests the resolve_root_symlink directive behavior
func TestSymlinkResolveRoot(t *testing.T) {
cwd, _ := os.Getwd()
testDir := filepath.Join(cwd, "..", "testdata", "symlinks", "test")
publicDir := filepath.Join(cwd, "..", "testdata", "symlinks", "public")

t.Run("ResolveRootSymlink", func(t *testing.T) {
// Tests that resolve_root_symlink directive works correctly
tester := caddytest.NewTester(t)
tester.InitServer(`
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`

frankenphp {
worker `+publicDir+`/document-root.php 1
}
}

localhost:`+testPort+` {
route {
php {
root `+publicDir+`
resolve_root_symlink true
}
}
}
`, "caddyfile")

// DOCUMENT_ROOT should be the resolved path (testDir)
tester.AssertGetResponse("http://localhost:"+testPort+"/document-root.php", http.StatusOK, "DOCUMENT_ROOT="+testDir+"\n")
})

t.Run("NoResolveRootSymlink", func(t *testing.T) {
// Tests that symlinks are preserved when resolve_root_symlink is false (non-worker mode)
tester := caddytest.NewTester(t)
tester.InitServer(`
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`
}

localhost:`+testPort+` {
route {
php {
root `+publicDir+`
resolve_root_symlink false
}
}
}
`, "caddyfile")

// DOCUMENT_ROOT should be the symlink path (publicDir) when resolve_root_symlink is false
tester.AssertGetResponse("http://localhost:"+testPort+"/document-root.php", http.StatusOK, "DOCUMENT_ROOT="+publicDir+"\n")
})
}

// TestSymlinkWorkerBehavior tests worker behavior with symlinked directories
func TestSymlinkWorkerBehavior(t *testing.T) {
cwd, _ := os.Getwd()
publicDir := filepath.Join(cwd, "..", "testdata", "symlinks", "public")

t.Run("WorkerScriptFailsWithoutWorkerMode", func(t *testing.T) {
// Tests that accessing a worker-only script without configuring it as a worker actually results in an error
tester := caddytest.NewTester(t)
tester.InitServer(`
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`
}

localhost:`+testPort+` {
route {
php {
root `+publicDir+`
}
}
}
`, "caddyfile")

// Accessing the worker script without worker configuration MUST fail
// The script checks $_SERVER['FRANKENPHP_WORKER'] and dies if not set
tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "Error: This script must be run in worker mode (FRANKENPHP_WORKER not set to '1')\n")
})

t.Run("MultipleRequests", func(t *testing.T) {
// Tests that symlinked workers handle multiple requests correctly
tester := caddytest.NewTester(t)
tester.InitServer(`
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`
}

localhost:`+testPort+` {
route {
php {
root `+publicDir+`
resolve_root_symlink true
worker index.php 1
}
}
}
`, "caddyfile")

// Make multiple requests - each should increment the counter
for i := 0; i < 5; i++ {
tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, fmt.Sprintf("Request: %d\n", i))
}
})
}
14 changes: 13 additions & 1 deletion caddy/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,15 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
}

f.resolvedDocumentRoot = root

// Also resolve symlinks in worker file paths when resolve_root_symlink is true
for i, wc := range f.Workers {
if !filepath.IsAbs(wc.FileName) {
continue
}
resolvedPath, _ := filepath.EvalSymlinks(wc.FileName)
f.Workers[i].FileName = resolvedPath
}
}
}

Expand Down Expand Up @@ -181,7 +190,10 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
if documentRoot == "" && frankenphp.EmbeddedAppPath != "" {
documentRoot = frankenphp.EmbeddedAppPath
}
documentRootOption = frankenphp.WithRequestDocumentRoot(documentRoot, *f.ResolveRootSymlink)
// If we do not have a resolved document root, then we cannot resolve the symlink of our cwd because it may
// resolve to a different directory than the one we are currently in.
// This is especially important if there are workers running.
documentRootOption = frankenphp.WithRequestDocumentRoot(documentRoot, false)
Copy link
Contributor

Choose a reason for hiding this comment

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

Not checking for symlinks here seems like it might break some setups. Definitely would be nice to test all the edge cases somehow (should be possible with caddy_tests?).

Tbh though, I don't know what the current edge cases with symlinks are

Copy link
Member Author

Choose a reason for hiding this comment

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

I’m pretty sure we don’t want to evaluate symlinks here. In the case where a document root was expressly given (such as prod setups), we will never even enter this branch. In the case where a document root isn’t given, we need to be compatible with how we resolve workers. There is an edge case here that I didn’t cover, and I’m working out how to handle it: chained symlinks.

I suspect I’ll have to go back to the drawing board.

} else {
documentRoot = f.resolvedDocumentRoot
documentRootOption = frankenphp.WithRequestResolvedDocumentRoot(documentRoot)
Expand Down
7 changes: 4 additions & 3 deletions internal/testext/exttest.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,14 @@ package testext
// #include "extension.h"
import "C"
import (
"github.com/dunglas/frankenphp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"io"
"net/http/httptest"
"testing"
"unsafe"

"github.com/dunglas/frankenphp"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func testRegisterExtension(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions testdata/symlinks/public
13 changes: 13 additions & 0 deletions testdata/symlinks/test/document-root.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

if (isset($_SERVER['FRANKENPHP_WORKER'])) {
$i = 0;
do {
$ok = frankenphp_handle_request(function () use ($i): void {
echo "DOCUMENT_ROOT=" . $_SERVER['DOCUMENT_ROOT'] . "\n";
});
$i++;
} while ($ok);
} else {
echo "DOCUMENT_ROOT=" . $_SERVER['DOCUMENT_ROOT'] . "\n";
}
13 changes: 13 additions & 0 deletions testdata/symlinks/test/index.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

if (!isset($_SERVER['FRANKENPHP_WORKER'])) {
die("Error: This script must be run in worker mode (FRANKENPHP_WORKER not set to '1')\n");
}

$i = 0;
do {
$ok = frankenphp_handle_request(function () use ($i): void {
echo sprintf("Request: %d\n", $i);
});
$i++;
} while ($ok);
13 changes: 13 additions & 0 deletions testdata/symlinks/test/nested/index.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?php

if (!isset($_SERVER['FRANKENPHP_WORKER'])) {
die("Error: This script must be run in worker mode (FRANKENPHP_WORKER not set to '1')\n");
}

$i = 0;
do {
$ok = frankenphp_handle_request(function () use ($i): void {
echo sprintf("Nested request: %d\n", $i);
});
$i++;
} while ($ok);
9 changes: 8 additions & 1 deletion worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,14 @@ func getWorkerByPath(path string) *worker {
}

func newWorker(o workerOpt) (*worker, error) {
absFileName, err := fastabs.FastAbs(o.fileName)
// Order is important!
// This order ensures that FrankenPHP started from inside a symlinked directory will properly resolve any paths.
// If it is started from outside a symlinked directory, it is resolved to the same path that we use in the Caddy module.
absFileName, err := filepath.EvalSymlinks(o.fileName)
if err != nil {
return nil, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err)
}
Comment on lines +117 to +120
Copy link
Contributor

Choose a reason for hiding this comment

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

I like it 👍 explicitly checking if a worker exists is something I actually wanted to do for a long time

Copy link
Contributor

Choose a reason for hiding this comment

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

Or wait, evalSymlinks will just evaluate symlinks. In that case it would also be nice to error right away if there's no file there. Can also be done in another PR though

Copy link
Member Author

Choose a reason for hiding this comment

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

it errors if the file doesn't exist (I had to update a failing test -- haven't pushed the commit yet)

absFileName, err = fastabs.FastAbs(absFileName)
if err != nil {
return nil, fmt.Errorf("worker filename is invalid %q: %w", o.fileName, err)
}
Expand Down