Skip to content

Commit

Permalink
Removes the caddy configuration and checks php.ini for filter.default
Browse files Browse the repository at this point in the history
  • Loading branch information
a.stecher authored and dunglas committed Oct 24, 2024
1 parent 2a3fb29 commit 8d60a22
Show file tree
Hide file tree
Showing 9 changed files with 41 additions and 123 deletions.
15 changes: 0 additions & 15 deletions caddy/caddy.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@ type FrankenPHPApp struct {
NumThreads int `json:"num_threads,omitempty"`
// Workers configures the worker scripts to start.
Workers []workerConfig `json:"workers,omitempty"`
// Whether to support fringe features like filter_input(INPUT_SERVER, $name)
DeprecatedMode bool `json:"deprecated_mode,omitempty"`
}

// CaddyModule returns the Caddy module information.
Expand All @@ -91,9 +89,6 @@ func (f *FrankenPHPApp) Start() error {
for _, w := range f.Workers {
opts = append(opts, frankenphp.WithWorkers(repl.ReplaceKnown(w.FileName, ""), w.Num, w.Env, w.Watch))
}
if f.DeprecatedMode {
opts = append(opts, frankenphp.WithDeprecatedMode())
}

_, loaded, err := phpInterpreter.LoadOrNew(mainPHPInterpreterKey, func() (caddy.Destructor, error) {
if err := frankenphp.Init(opts...); err != nil {
Expand Down Expand Up @@ -141,16 +136,6 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error {
}

f.NumThreads = v
case "deprecated_mode":
if !d.NextArg() {
f.DeprecatedMode = true
continue
}
v, err := strconv.ParseBool(d.Val())
if err != nil {
return err
}
f.DeprecatedMode = v
case "worker":
wc := workerConfig{}
if d.NextArg() {
Expand Down
58 changes: 11 additions & 47 deletions caddy/caddy_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"fmt"
"net/http"
"os"
"path/filepath"
"strings"
"sync"
"testing"
Expand Down Expand Up @@ -577,50 +578,13 @@ func TestAutoWorkerConfig(t *testing.T) {
))
}

func TestAllServerVarsWithInputFilterInDeprecatedMode(t *testing.T) {
absPath, _ := filepath.Abs("./testdata/")
expectedBody, _ := os.ReadFile("../testdata/server-filter-var.txt")
expectedBody = bytes.ReplaceAll(expectedBody, "{withFilterVar}", "true")
expectedBody = bytes.ReplaceAll(expectedBody, "{absPath}", absPath)
expectedBody = bytes.ReplaceAll(expectedBody, "{testPort}", testPort)
tester := caddytest.NewTester(t)
tester.InitServer(`
{
skip_install_trust
admin localhost:2999
http_port `+testPort+`
frankenphp {
deprecated_mode true
}
}
localhost:`+testPort+` {
route {
root ../testdata
php
}
}
`, "caddyfile")

tester.AssertPostResponseBody(
"http://user@localhost:"+testPort+"/server-filter-var.php/path?withFilterVar=true&specialChars=%3C\\x00</>",
[]string{
"Content-Type: application/x-www-form-urlencoded",
"Content-Length: 14", // maliciously set to 14
"Special-Chars: <\\x00>",
"Host: Malicous Host",
},
bytes.NewBufferString("foo=bar"),
http.StatusOK,
string(expectedBody),
)
}

func TestAllServerVarsWithoutInputFilter(t *testing.T) {
absPath, _ := filepath.Abs("./testdata/")
expectedBody, _ := os.ReadFile("../testdata/server-filter-var.txt")
expectedBody = bytes.ReplaceAll(expectedBody, "{withFilterVar}", "false")
expectedBody = bytes.ReplaceAll(expectedBody, "{absPath}", absPath)
expectedBody = bytes.ReplaceAll(expectedBody, "{testPort}", testPort)
func TestAllDefinedServerVars(t *testing.T) {
documentRoot, _ := filepath.Abs("../testdata/")
expectedBodyFile, _ := os.ReadFile("../testdata/server-all-vars-ordered.txt")
expectedBody := string(expectedBodyFile)
expectedBody = strings.ReplaceAll(expectedBody, "{documentRoot}", documentRoot)
expectedBody = strings.ReplaceAll(expectedBody, "\r\n", "\n")
expectedBody = strings.ReplaceAll(expectedBody, "{testPort}", testPort)
tester := caddytest.NewTester(t)
tester.InitServer(`
{
Expand All @@ -637,15 +601,15 @@ func TestAllServerVarsWithoutInputFilter(t *testing.T) {
}
`, "caddyfile")
tester.AssertPostResponseBody(
"http://user@localhost:"+testPort+"/server-filter-var.php/path?withFilterVar=false&specialChars=%3C%3E\\x00</>",
"http://user@localhost:"+testPort+"/server-all-vars-ordered.php/path?specialChars=%3E\\x00%00</>",
[]string{
"Content-Type: application/x-www-form-urlencoded",
"Content-Length: 14", // maliciously set to 14
"Special-Chars: <\\x00>",
"Special-Chars: <%00>",
"Host: Malicous Host",
},
bytes.NewBufferString("foo=bar"),
http.StatusOK,
string(expectedBody),
expectedBody,
)
}
17 changes: 0 additions & 17 deletions docs/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,20 +231,3 @@ docker run -v $PWD:/app/public \
-p 80:80 -p 443:443 -p 443:443/udp \
dunglas/frankenphp
```

## Enable the Deprecated Mode

Some PHP features are disabled by default since they will negatively impact performance. You can still enable these
features by setting `deprecated_mode` to true in the global options.

```caddyfile
{
frankenphp {
deprecated_mode true
}
}
```

Currently, 'deprecated' features include:

- Using `filter_input(INPUT_SERVER, $variable)` instead of `$_SERVER[$variable]`
17 changes: 12 additions & 5 deletions frankenphp.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ typedef struct frankenphp_server_context {
bool finished;
} frankenphp_server_context;

static bool should_filter_var = 0;
__thread bool should_filter_var = 0;
__thread frankenphp_server_context *local_ctx = NULL;
__thread uintptr_t thread_index;
__thread zval *os_environment = NULL;
Expand Down Expand Up @@ -669,7 +669,7 @@ void frankenphp_release_zend_string(zend_string *z_string) {

static void
frankenphp_register_variable_from_request_info(zend_string *zKey, char *value,
bool must_be_present,
bool must_be_present,
zval *track_vars_array) {
if (value != NULL) {
frankenphp_register_trusted_var(zKey, value, strlen(value),
Expand Down Expand Up @@ -701,7 +701,8 @@ void frankenphp_register_variables_from_request_info(
request_uri, SG(request_info).request_uri, true, track_vars_array);
}

/* variables with user defined keys must be registered safely to avoid globals takepvers */
/* variables with user defined keys must be registered safely to avoid globals
* takepvers */
void frankenphp_register_variable_safe(char *key, char *val, size_t val_len,
zval *track_vars_array) {
if (val == NULL || key == NULL) {
Expand Down Expand Up @@ -814,6 +815,12 @@ static void *php_thread(void *arg) {

local_ctx = malloc(sizeof(frankenphp_server_context));

/* check if a default filter (deprecated) is set in php.ini and only filter if
* it is */
char *default_filter;
cfg_get_string("filter.default", &default_filter);
should_filter_var = default_filter != NULL;

while (go_handle_request(thread_index)) {
}

Expand Down Expand Up @@ -917,9 +924,9 @@ static void *php_main(void *arg) {
return NULL;
}

int frankenphp_init(int num_threads, bool deprecated_mode) {
int frankenphp_init(int num_threads) {
pthread_t thread;
should_filter_var = deprecated_mode;
should_filter_var = false;

if (pthread_create(&thread, NULL, &php_main, (void *)(intptr_t)num_threads) !=
0) {
Expand Down
2 changes: 1 addition & 1 deletion frankenphp.go
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ func Init(options ...Option) error {
requestChan = make(chan *http.Request)
initPHPThreads(opt.numThreads)

if C.frankenphp_init(C.int(opt.numThreads), C.bool(opt.deprecatedMode)) != 0 {
if C.frankenphp_init(C.int(opt.numThreads)) != 0 {
return MainThreadCreationError
}

Expand Down
2 changes: 1 addition & 1 deletion frankenphp.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ typedef struct frankenphp_config {
} frankenphp_config;
frankenphp_config frankenphp_get_config();

int frankenphp_init(int num_threads, bool deprecated_mode);
int frankenphp_init(int num_threads);

int frankenphp_update_server_context(
bool create, bool has_main_request, bool has_active_request,
Expand Down
18 changes: 4 additions & 14 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,10 @@ type Option func(h *opt) error
//
// If you change this, also update the Caddy module and the documentation.
type opt struct {
numThreads int
deprecatedMode bool
workers []workerOpt
logger *zap.Logger
metrics Metrics
numThreads int
workers []workerOpt
logger *zap.Logger
metrics Metrics
}

type workerOpt struct {
Expand Down Expand Up @@ -59,12 +58,3 @@ func WithLogger(l *zap.Logger) Option {
return nil
}
}

// WithFringeMode configures whether to enable fringe features like filter_input(INPUT_SERVER, $name).
func WithDeprecatedMode() Option {
return func(o *opt) error {
o.deprecatedMode = true

return nil
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
<?php
/**
* This file tests the filter_input(INPUT_SERVER, $name) feature
* Specifically when it's enabled via deprecated_mode
* If it's not enabled, just echo out the variables directly from $_SERVER
* @see TestAllServerVarsWithoutInputFilter
* @see TestAllServerVarsWithInputFilterInFringeMode
*/

echo "<pre>\n";
foreach ([
'CONTENT_LENGTH',
Expand Down Expand Up @@ -40,11 +34,6 @@
'REQUEST_METHOD',
'REQUEST_URI',
] as $name) {

if ($_GET['withFilterVar'] === 'true') {
echo "$name:" . filter_input(INPUT_SERVER, $name) . "\n";
} else {
echo "$name:" . $_SERVER[$name] . "\n";
}
echo "$name:" . $_SERVER[$name] . "\n";
}
echo "</pre>";
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
<pre>
CONTENT_LENGTH:7
HTTP_CONTENT_LENGTH:7
HTTP_SPECIAL_CHARS:<\x00>
DOCUMENT_ROOT:{absPath}/testdata
DOCUMENT_URI:/server-filter-var.php
HTTP_SPECIAL_CHARS:<%00>
DOCUMENT_ROOT:{documentRoot}
DOCUMENT_URI:/server-all-vars-ordered.php
GATEWAY_INTERFACE:CGI/1.1
HTTP_HOST:localhost:{testPort}
HTTPS:
PATH_INFO:/path
CONTENT_TYPE:application/x-www-form-urlencoded
DOCUMENT_ROOT:{absPath}/testdata
DOCUMENT_ROOT:{documentRoot}
REMOTE_ADDR:127.0.0.1
CONTENT_LENGTH:7
PHP_SELF:/server-filter-var.php/path
PHP_SELF:/server-all-vars-ordered.php/path
REMOTE_HOST:127.0.0.1
REQUEST_SCHEME:http
SCRIPT_FILENAME:{absPath}/testdata/server-filter-var.php
SCRIPT_NAME:/server-filter-var.php
SCRIPT_FILENAME:{documentRoot}/server-all-vars-ordered.php
SCRIPT_NAME:/server-all-vars-ordered.php
SERVER_NAME:localhost
SERVER_PORT:{testPort}
SERVER_PROTOCOL:HTTP/1.1
Expand All @@ -25,9 +25,9 @@ SSL_PROTOCOL:
AUTH_TYPE:
REMOTE_IDENT:
CONTENT_TYPE:application/x-www-form-urlencoded
PATH_TRANSLATED:{absPath}/testdata/path
QUERY_STRING:withFilterVar={withFilterVar}&specialChars=%3C\x00</>
PATH_TRANSLATED:{documentRoot}/path
QUERY_STRING:specialChars=%3E\x00%00</>
REMOTE_USER:user
REQUEST_METHOD:POST
REQUEST_URI:/server-filter-var.php/path?withFilterVar={withFilterVar}&specialChars=%3C\x00</>
REQUEST_URI:/server-all-vars-ordered.php/path?specialChars=%3E\x00%00</>
</pre>

0 comments on commit 8d60a22

Please sign in to comment.