Skip to content

Commit e0f01d1

Browse files
withinboredomhenderkesdunglas
authored
Handle symlinking edge cases (#1660)
This one is interesting — though I’m not sure the best way to provide a test. I will have to look into maybe an integration test because it is a careful dance between how we resolve paths in the Caddy module vs. workers. I looked into making a proper change (literally using the same logic everywhere), but I think it is best to wait until #1646 is merged. But anyway, this change deals with some interesting edge cases. I will use gherkin to describe them: ```gherkin Feature: FrankenPHP symlinked edge cases Background: Given a `test` folder And a `public` folder linked to `test` And a worker script located at `test/index.php` And a `test/nested` folder And a worker script located at `test/nested/index.php` 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 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 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 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 ``` Trying to write that out in regular English would be more complex IMHO. These scenarios should all pass now with this PR. --------- Signed-off-by: Marc <m@pyc.ac> Co-authored-by: henderkes <m@pyc.ac> Co-authored-by: Kévin Dunglas <kevin@dunglas.fr>
1 parent a6b0577 commit e0f01d1

8 files changed

Lines changed: 315 additions & 5 deletions

File tree

‎caddy/caddy_test.go‎

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1500,3 +1500,253 @@ func TestLog(t *testing.T) {
15001500
"",
15011501
)
15021502
}
1503+
1504+
// TestSymlinkWorkerPaths tests different ways to reference worker scripts in symlinked directories
1505+
func TestSymlinkWorkerPaths(t *testing.T) {
1506+
cwd, _ := os.Getwd()
1507+
publicDir := filepath.Join(cwd, "..", "testdata", "symlinks", "public")
1508+
1509+
t.Run("NeighboringWorkerScript", func(t *testing.T) {
1510+
// Scenario: neighboring worker script
1511+
// Given frankenphp located in the test folder
1512+
// When I execute `frankenphp php-server --listen localhost:8080 -w index.php` from `public`
1513+
// Then I expect to see the worker script executed successfully
1514+
tester := caddytest.NewTester(t)
1515+
tester.InitServer(`
1516+
{
1517+
skip_install_trust
1518+
admin localhost:2999
1519+
http_port `+testPort+`
1520+
1521+
frankenphp {
1522+
worker `+publicDir+`/index.php 1
1523+
}
1524+
}
1525+
1526+
localhost:`+testPort+` {
1527+
route {
1528+
php {
1529+
root `+publicDir+`
1530+
resolve_root_symlink true
1531+
}
1532+
}
1533+
}
1534+
`, "caddyfile")
1535+
1536+
tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "Request: 0\n")
1537+
})
1538+
1539+
t.Run("NestedWorkerScript", func(t *testing.T) {
1540+
// Scenario: nested worker script
1541+
// Given frankenphp located in the test folder
1542+
// When I execute `frankenphp --listen localhost:8080 -w nested/index.php` from `public`
1543+
// Then I expect to see the worker script executed successfully
1544+
tester := caddytest.NewTester(t)
1545+
tester.InitServer(`
1546+
{
1547+
skip_install_trust
1548+
admin localhost:2999
1549+
http_port `+testPort+`
1550+
1551+
frankenphp {
1552+
worker `+publicDir+`/nested/index.php 1
1553+
}
1554+
}
1555+
1556+
localhost:`+testPort+` {
1557+
route {
1558+
php {
1559+
root `+publicDir+`
1560+
resolve_root_symlink true
1561+
}
1562+
}
1563+
}
1564+
`, "caddyfile")
1565+
1566+
tester.AssertGetResponse("http://localhost:"+testPort+"/nested/index.php", http.StatusOK, "Nested request: 0\n")
1567+
})
1568+
1569+
t.Run("OutsideSymlinkedFolder", func(t *testing.T) {
1570+
// Scenario: outside the symlinked folder
1571+
// Given frankenphp located in the root folder
1572+
// When I execute `frankenphp --listen localhost:8080 -w public/index.php` from the root folder
1573+
// Then I expect to see the worker script executed successfully
1574+
tester := caddytest.NewTester(t)
1575+
tester.InitServer(`
1576+
{
1577+
skip_install_trust
1578+
admin localhost:2999
1579+
http_port `+testPort+`
1580+
1581+
frankenphp {
1582+
worker {
1583+
name outside_worker
1584+
file `+publicDir+`/index.php
1585+
num 1
1586+
}
1587+
}
1588+
}
1589+
1590+
localhost:`+testPort+` {
1591+
route {
1592+
php {
1593+
root `+publicDir+`
1594+
resolve_root_symlink true
1595+
}
1596+
}
1597+
}
1598+
`, "caddyfile")
1599+
1600+
tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "Request: 0\n")
1601+
})
1602+
1603+
t.Run("SpecifiedRootDirectory", func(t *testing.T) {
1604+
// Scenario: specified root directory
1605+
// Given frankenphp located in the root folder
1606+
// When I execute `frankenphp --listen localhost:8080 -w public/index.php -r public` from the root folder
1607+
// Then I expect to see the worker script executed successfully
1608+
tester := caddytest.NewTester(t)
1609+
tester.InitServer(`
1610+
{
1611+
skip_install_trust
1612+
admin localhost:2999
1613+
http_port `+testPort+`
1614+
1615+
frankenphp {
1616+
worker {
1617+
name specified_root_worker
1618+
file `+publicDir+`/index.php
1619+
num 1
1620+
}
1621+
}
1622+
}
1623+
1624+
localhost:`+testPort+` {
1625+
route {
1626+
php {
1627+
root `+publicDir+`
1628+
resolve_root_symlink true
1629+
}
1630+
}
1631+
}
1632+
`, "caddyfile")
1633+
1634+
tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, "Request: 0\n")
1635+
})
1636+
}
1637+
1638+
// TestSymlinkResolveRoot tests the resolve_root_symlink directive behavior
1639+
func TestSymlinkResolveRoot(t *testing.T) {
1640+
cwd, _ := os.Getwd()
1641+
testDir := filepath.Join(cwd, "..", "testdata", "symlinks", "test")
1642+
publicDir := filepath.Join(cwd, "..", "testdata", "symlinks", "public")
1643+
1644+
t.Run("ResolveRootSymlink", func(t *testing.T) {
1645+
// Tests that resolve_root_symlink directive works correctly
1646+
tester := caddytest.NewTester(t)
1647+
tester.InitServer(`
1648+
{
1649+
skip_install_trust
1650+
admin localhost:2999
1651+
http_port `+testPort+`
1652+
1653+
frankenphp {
1654+
worker `+publicDir+`/document-root.php 1
1655+
}
1656+
}
1657+
1658+
localhost:`+testPort+` {
1659+
route {
1660+
php {
1661+
root `+publicDir+`
1662+
resolve_root_symlink true
1663+
}
1664+
}
1665+
}
1666+
`, "caddyfile")
1667+
1668+
// DOCUMENT_ROOT should be the resolved path (testDir)
1669+
tester.AssertGetResponse("http://localhost:"+testPort+"/document-root.php", http.StatusOK, "DOCUMENT_ROOT="+testDir+"\n")
1670+
})
1671+
1672+
t.Run("NoResolveRootSymlink", func(t *testing.T) {
1673+
// Tests that symlinks are preserved when resolve_root_symlink is false (non-worker mode)
1674+
tester := caddytest.NewTester(t)
1675+
tester.InitServer(`
1676+
{
1677+
skip_install_trust
1678+
admin localhost:2999
1679+
http_port `+testPort+`
1680+
}
1681+
1682+
localhost:`+testPort+` {
1683+
route {
1684+
php {
1685+
root `+publicDir+`
1686+
resolve_root_symlink false
1687+
}
1688+
}
1689+
}
1690+
`, "caddyfile")
1691+
1692+
// DOCUMENT_ROOT should be the symlink path (publicDir) when resolve_root_symlink is false
1693+
tester.AssertGetResponse("http://localhost:"+testPort+"/document-root.php", http.StatusOK, "DOCUMENT_ROOT="+publicDir+"\n")
1694+
})
1695+
}
1696+
1697+
// TestSymlinkWorkerBehavior tests worker behavior with symlinked directories
1698+
func TestSymlinkWorkerBehavior(t *testing.T) {
1699+
cwd, _ := os.Getwd()
1700+
publicDir := filepath.Join(cwd, "..", "testdata", "symlinks", "public")
1701+
1702+
t.Run("WorkerScriptFailsWithoutWorkerMode", func(t *testing.T) {
1703+
// Tests that accessing a worker-only script without configuring it as a worker actually results in an error
1704+
tester := caddytest.NewTester(t)
1705+
tester.InitServer(`
1706+
{
1707+
skip_install_trust
1708+
admin localhost:2999
1709+
http_port `+testPort+`
1710+
}
1711+
1712+
localhost:`+testPort+` {
1713+
route {
1714+
php {
1715+
root `+publicDir+`
1716+
}
1717+
}
1718+
}
1719+
`, "caddyfile")
1720+
1721+
// Accessing the worker script without worker configuration MUST fail
1722+
// The script checks $_SERVER['FRANKENPHP_WORKER'] and dies if not set
1723+
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")
1724+
})
1725+
1726+
t.Run("MultipleRequests", func(t *testing.T) {
1727+
// Tests that symlinked workers handle multiple requests correctly
1728+
tester := caddytest.NewTester(t)
1729+
tester.InitServer(`
1730+
{
1731+
skip_install_trust
1732+
admin localhost:2999
1733+
http_port `+testPort+`
1734+
}
1735+
1736+
localhost:`+testPort+` {
1737+
route {
1738+
php {
1739+
root `+publicDir+`
1740+
resolve_root_symlink true
1741+
worker index.php 1
1742+
}
1743+
}
1744+
}
1745+
`, "caddyfile")
1746+
1747+
// Make multiple requests - each should increment the counter
1748+
for i := 0; i < 5; i++ {
1749+
tester.AssertGetResponse("http://localhost:"+testPort+"/index.php", http.StatusOK, fmt.Sprintf("Request: %d\n", i))
1750+
}
1751+
})
1752+
}

‎caddy/module.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,15 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error {
138138
}
139139

140140
f.resolvedDocumentRoot = root
141+
142+
// Also resolve symlinks in worker file paths when resolve_root_symlink is true
143+
for i, wc := range f.Workers {
144+
if !filepath.IsAbs(wc.FileName) {
145+
continue
146+
}
147+
resolvedPath, _ := filepath.EvalSymlinks(wc.FileName)
148+
f.Workers[i].FileName = resolvedPath
149+
}
141150
}
142151
}
143152

@@ -181,7 +190,10 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c
181190
if documentRoot == "" && frankenphp.EmbeddedAppPath != "" {
182191
documentRoot = frankenphp.EmbeddedAppPath
183192
}
184-
documentRootOption = frankenphp.WithRequestDocumentRoot(documentRoot, *f.ResolveRootSymlink)
193+
// If we do not have a resolved document root, then we cannot resolve the symlink of our cwd because it may
194+
// resolve to a different directory than the one we are currently in.
195+
// This is especially important if there are workers running.
196+
documentRootOption = frankenphp.WithRequestDocumentRoot(documentRoot, false)
185197
} else {
186198
documentRoot = f.resolvedDocumentRoot
187199
documentRootOption = frankenphp.WithRequestResolvedDocumentRoot(documentRoot)

‎internal/testext/exttest.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,13 +11,14 @@ package testext
1111
// #include "extension.h"
1212
import "C"
1313
import (
14-
"github.com/dunglas/frankenphp"
15-
"github.com/stretchr/testify/assert"
16-
"github.com/stretchr/testify/require"
1714
"io"
1815
"net/http/httptest"
1916
"testing"
2017
"unsafe"
18+
19+
"github.com/dunglas/frankenphp"
20+
"github.com/stretchr/testify/assert"
21+
"github.com/stretchr/testify/require"
2122
)
2223

2324
func testRegisterExtension(t *testing.T) {

‎testdata/symlinks/public‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test

‎testdata/symlinks/test/document-root.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
if (isset($_SERVER['FRANKENPHP_WORKER'])) {
4+
$i = 0;
5+
do {
6+
$ok = frankenphp_handle_request(function () use ($i): void {
7+
echo "DOCUMENT_ROOT=" . $_SERVER['DOCUMENT_ROOT'] . "\n";
8+
});
9+
$i++;
10+
} while ($ok);
11+
} else {
12+
echo "DOCUMENT_ROOT=" . $_SERVER['DOCUMENT_ROOT'] . "\n";
13+
}

‎testdata/symlinks/test/index.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
if (!isset($_SERVER['FRANKENPHP_WORKER'])) {
4+
die("Error: This script must be run in worker mode (FRANKENPHP_WORKER not set to '1')\n");
5+
}
6+
7+
$i = 0;
8+
do {
9+
$ok = frankenphp_handle_request(function () use ($i): void {
10+
echo sprintf("Request: %d\n", $i);
11+
});
12+
$i++;
13+
} while ($ok);

‎testdata/symlinks/test/nested/index.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
<?php
2+
3+
if (!isset($_SERVER['FRANKENPHP_WORKER'])) {
4+
die("Error: This script must be run in worker mode (FRANKENPHP_WORKER not set to '1')\n");
5+
}
6+
7+
$i = 0;
8+
do {
9+
$ok = frankenphp_handle_request(function () use ($i): void {
10+
echo sprintf("Nested request: %d\n", $i);
11+
});
12+
$i++;
13+
} while ($ok);

‎worker.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,14 @@ func getWorkerByPath(path string) *worker {
111111
}
112112

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

0 commit comments

Comments
 (0)