From 92f305d87a0b6dcbe009c8816195d71108c468e9 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 27 Jul 2025 14:31:46 +0200 Subject: [PATCH 01/16] Removes NewRequestWithContext. --- caddy/module.go | 7 +++---- context.go | 27 +++++++++----------------- frankenphp.go | 12 +++++------- frankenphp_test.go | 38 ++++++++----------------------------- internal/testext/exttest.go | 5 +---- internal/testserver/main.go | 7 +------ phpmainthread_test.go | 4 +--- worker_test.go | 7 +------ 8 files changed, 29 insertions(+), 78 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 50056ead4d..1cf0e7b9bc 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -184,16 +184,15 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c } } - fr, err := frankenphp.NewRequestWithContext( + if err := frankenphp.ServeHTTP( + w, r, documentRootOption, frankenphp.WithRequestSplitPath(f.SplitPath), frankenphp.WithRequestPreparedEnv(env), frankenphp.WithOriginalRequest(&origReq), frankenphp.WithWorkerName(workerName), - ) - - if err = frankenphp.ServeHTTP(w, fr); err != nil { + ); err != nil { return caddyhttp.Error(http.StatusInternalServerError, err) } diff --git a/context.go b/context.go index 6de140da7a..ede1babd5f 100644 --- a/context.go +++ b/context.go @@ -1,7 +1,6 @@ package frankenphp import ( - "context" "log/slog" "net/http" "os" @@ -33,18 +32,13 @@ type frankenPHPContext struct { startedAt time.Time } -// fromContext extracts the frankenPHPContext from a context. -func fromContext(ctx context.Context) (fctx *frankenPHPContext, ok bool) { - fctx, ok = ctx.Value(contextKey).(*frankenPHPContext) - return -} - -// NewRequestWithContext creates a new FrankenPHP request context. -func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) { +// newFrankenPHPContext creates a new FrankenPHP request context. +func newFrankenPHPContext(rw http.ResponseWriter, r *http.Request, opts ...RequestOption) (*frankenPHPContext, error) { fc := &frankenPHPContext{ - done: make(chan interface{}), - startedAt: time.Now(), - request: r, + done: make(chan interface{}), + startedAt: time.Now(), + request: r, + responseWriter: rw, } for _, o := range opts { if err := o(fc); err != nil { @@ -99,24 +93,21 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques fc.worker = getWorkerByPath(fc.scriptFilename) } - c := context.WithValue(r.Context(), contextKey, fc) - - return r.WithContext(c), nil + return fc, nil } +// newDummyContext creates a fake context from just a request path. func newDummyContext(requestPath string, opts ...RequestOption) (*frankenPHPContext, error) { r, err := http.NewRequest(http.MethodGet, requestPath, nil) if err != nil { return nil, err } - fr, err := NewRequestWithContext(r, opts...) + fc, err := newFrankenPHPContext(nil, r, opts...) if err != nil { return nil, err } - fc, _ := fromContext(fr.Context()) - return fc, nil } diff --git a/frankenphp.go b/frankenphp.go index d2437dbf06..961521c1b9 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -377,19 +377,17 @@ func updateServerContext(thread *phpThread, fc *frankenPHPContext, isWorkerReque return nil } -// ServeHTTP executes a PHP script according to the given context. -func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error { +// ServeHTTP executes a PHP script according to the given context and options. +func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { if !isRunning { return ErrNotRunning } - fc, ok := fromContext(request.Context()) - if !ok { - return ErrInvalidRequest + fc, err := newFrankenPHPContext(responseWriter, request, opts...) + if err != nil { + return fmt.Errorf("%w: %s", ErrRequestContextCreation, err.Error()) } - fc.responseWriter = responseWriter - if !fc.validate() { return nil } diff --git a/frankenphp_test.go b/frankenphp_test.go index 24241e058b..5a768a62e4 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -82,10 +82,7 @@ func runTest(t *testing.T, test func(func(http.ResponseWriter, *http.Request), * defer frankenphp.Shutdown() handler := func(w http.ResponseWriter, r *http.Request) { - req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(testDataDir, false)) - assert.NoError(t, err) - - err = frankenphp.ServeHTTP(w, req) + err = frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot(testDataDir, false)) assert.NoError(t, err) } @@ -200,14 +197,13 @@ func testPathInfo(t *testing.T, opts *testOptions) { requestURI := r.URL.RequestURI() r.URL.Path = path - rewriteRequest, err := frankenphp.NewRequestWithContext(r, + err := frankenphp.ServeHTTP( + w, + r, frankenphp.WithRequestDocumentRoot(testDataDir, false), frankenphp.WithRequestEnv(map[string]string{"REQUEST_URI": requestURI}), ) assert.NoError(t, err) - - err = frankenphp.ServeHTTP(w, rewriteRequest) - assert.NoError(t, err) } req := httptest.NewRequest("GET", fmt.Sprintf("http://example.com/pathinfo/%d", i), nil) @@ -816,12 +812,7 @@ func ExampleServeHTTP() { defer frankenphp.Shutdown() http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot("/path/to/document/root", false)) - if err != nil { - panic(err) - } - - if err := frankenphp.ServeHTTP(w, req); err != nil { + if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot("/path/to/document/root", false)); err != nil { panic(err) } }) @@ -846,12 +837,7 @@ func BenchmarkHelloWorld(b *testing.B) { testDataDir := cwd + "/testdata/" handler := func(w http.ResponseWriter, r *http.Request) { - req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(testDataDir, false)) - if err != nil { - panic(err) - } - - if err := frankenphp.ServeHTTP(w, req); err != nil { + if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot(testDataDir, false)); err != nil { panic(err) } } @@ -874,11 +860,7 @@ func BenchmarkEcho(b *testing.B) { testDataDir := cwd + "/testdata/" handler := func(w http.ResponseWriter, r *http.Request) { - req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(testDataDir, false)) - if err != nil { - panic(err) - } - if err := frankenphp.ServeHTTP(w, req); err != nil { + if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot(testDataDir, false)); err != nil { panic(err) } } @@ -982,13 +964,9 @@ func BenchmarkServerSuperGlobal(b *testing.B) { preparedEnv := frankenphp.PrepareEnv(env) handler := func(w http.ResponseWriter, r *http.Request) { - req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(testDataDir, false), frankenphp.WithRequestPreparedEnv(preparedEnv)) - if err != nil { - panic(err) - } r.Header = headers - if err := frankenphp.ServeHTTP(w, req); err != nil { + if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot(testDataDir, false), frankenphp.WithRequestPreparedEnv(preparedEnv)); err != nil { panic(err) } } diff --git a/internal/testext/exttest.go b/internal/testext/exttest.go index abebee4c1d..862b6a09b1 100644 --- a/internal/testext/exttest.go +++ b/internal/testext/exttest.go @@ -31,10 +31,7 @@ func testRegisterExtension(t *testing.T) { req := httptest.NewRequest("GET", "http://example.com/index.php", nil) w := httptest.NewRecorder() - req, err = frankenphp.NewRequestWithContext(req, frankenphp.WithRequestDocumentRoot("./testdata", false)) - assert.NoError(t, err) - - err = frankenphp.ServeHTTP(w, req) + err = frankenphp.ServeHTTP(w, req, frankenphp.WithRequestDocumentRoot("./testdata", false)) assert.NoError(t, err) resp := w.Result() diff --git a/internal/testserver/main.go b/internal/testserver/main.go index 3e4af37b15..eadf927ded 100644 --- a/internal/testserver/main.go +++ b/internal/testserver/main.go @@ -17,12 +17,7 @@ func main() { defer frankenphp.Shutdown() http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - req, err := frankenphp.NewRequestWithContext(r) - if err != nil { - panic(err) - } - - if err := frankenphp.ServeHTTP(w, req); err != nil { + if err := frankenphp.ServeHTTP(w, r); err != nil { panic(err) } }) diff --git a/phpmainthread_test.go b/phpmainthread_test.go index db348b54d0..f4f4ae8f44 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -225,9 +225,7 @@ func assertRequestBody(t *testing.T, url string, expected string) { r := httptest.NewRequest("GET", url, nil) w := httptest.NewRecorder() - req, err := NewRequestWithContext(r, WithRequestDocumentRoot(testDataPath, false)) - assert.NoError(t, err) - err = ServeHTTP(w, req) + err := ServeHTTP(w, r, WithRequestDocumentRoot(testDataPath, false)) assert.NoError(t, err) resp := w.Result() body, _ := io.ReadAll(resp.Body) diff --git a/worker_test.go b/worker_test.go index c6b4245d47..83c43ab254 100644 --- a/worker_test.go +++ b/worker_test.go @@ -137,12 +137,7 @@ func ExampleServeHTTP_workers() { defer frankenphp.Shutdown() http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot("/path/to/document/root", false)) - if err != nil { - panic(err) - } - - if err := frankenphp.ServeHTTP(w, req); err != nil { + if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot("/path/to/document/root", false)); err != nil { panic(err) } }) From 91290241273d8bd6e93ab09721d44bd9dde6704d Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Fri, 1 Aug 2025 23:56:01 +0200 Subject: [PATCH 02/16] Moves cgi logic to cgi.go --- cgi.go | 40 +++++++++++++++++++++++++++++++++++++--- context.go | 34 +++------------------------------- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/cgi.go b/cgi.go index 238562fb17..4b388762cd 100644 --- a/cgi.go +++ b/cgi.go @@ -211,18 +211,52 @@ func go_register_variables(threadIndex C.uintptr_t, trackVarsArray *C.zval) { addPreparedEnvToServer(fc, trackVarsArray) } +// splitCgiPath splits the request path into SCRIPT_NAME, SCRIPT_FILENAME, PATH_INFO, DOCUMENT_URI +func splitCgiPath(fc *frankenPHPContext) { + path := fc.request.URL.Path + splitPath := fc.splitPath + + if splitPath == nil { + splitPath = []string{".php"} + } + + if splitPos := splitPos(path, splitPath); splitPos > -1 { + fc.docURI = path[:splitPos] + 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, use its filename + if fc.worker != nil { + fc.scriptFilename = fc.worker.fileName + return + } + + // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME + fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) + fc.worker = getWorkerByPath(fc.scriptFilename) +} + // splitPos returns the index where path should // be split based on SplitPath. // // Adapted from https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go // Copyright 2015 Matthew Holt and The Caddy Authors -func splitPos(fc *frankenPHPContext, path string) int { - if len(fc.splitPath) == 0 { +func splitPos(path string, splitPath []string) int { + if len(splitPath) == 0 { return 0 } lowerPath := strings.ToLower(path) - for _, split := range fc.splitPath { + for _, split := range splitPath { if idx := strings.Index(lowerPath, strings.ToLower(split)); idx > -1 { return idx + len(split) } diff --git a/context.go b/context.go index ede1babd5f..8205450e60 100644 --- a/context.go +++ b/context.go @@ -61,37 +61,9 @@ func newFrankenPHPContext(rw http.ResponseWriter, r *http.Request, opts ...Reque } } - if fc.splitPath == nil { - fc.splitPath = []string{".php"} - } - - if fc.env == nil { - fc.env = make(map[string]string) - } - - if splitPos := splitPos(fc, r.URL.Path); splitPos > -1 { - fc.docURI = r.URL.Path[:splitPos] - fc.pathInfo = r.URL.Path[splitPos:] - - // Strip PATH_INFO from SCRIPT_NAME - fc.scriptName = strings.TrimSuffix(r.URL.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 assigned explicitly, use its filename - // determine if the filename belongs to a worker otherwise - if fc.worker != nil { - fc.scriptFilename = fc.worker.fileName - } else { - // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME - fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) - fc.worker = getWorkerByPath(fc.scriptFilename) - } + // some cgi variables need to already be determined here + // this ensures the correct worker is assigned by path if necessary + splitCgiPath(fc) return fc, nil } From aea5064f87d00a673482ed20e4a67d7ae61d189f Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 3 Aug 2025 14:47:57 +0200 Subject: [PATCH 03/16] Calls 'update_request_info' from the C side. --- cgi.go | 58 +++++++++++++++++++++++++++++++++++++++++------ frankenphp.c | 46 ++++++++++++------------------------- frankenphp.go | 63 --------------------------------------------------- 3 files changed, 66 insertions(+), 101 deletions(-) diff --git a/cgi.go b/cgi.go index 4b388762cd..727b2a3895 100644 --- a/cgi.go +++ b/cgi.go @@ -21,6 +21,16 @@ import ( "github.com/dunglas/frankenphp/internal/phpheaders" ) +// Map of supported protocols to Apache ssl_mod format +// Note that these are slightly different from SupportedProtocols in caddytls/config.go +var tlsProtocolStrings = map[uint16]string{ + tls.VersionTLS10: "TLSv1", + tls.VersionTLS11: "TLSv1.1", + tls.VersionTLS12: "TLSv1.2", + tls.VersionTLS13: "TLSv1.3", +} + +// List of all keys in the $_SERVER array excluding headers and environment variables. var knownServerKeys = []string{ "CONTENT_LENGTH", "DOCUMENT_ROOT", @@ -264,13 +274,47 @@ func splitPos(path string, splitPath []string) int { return -1 } -// Map of supported protocols to Apache ssl_mod format -// Note that these are slightly different from SupportedProtocols in caddytls/config.go -var tlsProtocolStrings = map[uint16]string{ - tls.VersionTLS10: "TLSv1", - tls.VersionTLS11: "TLSv1.1", - tls.VersionTLS12: "TLSv1.2", - tls.VersionTLS13: "TLSv1.3", +// update the sapi_request_info struct +// see: https://github.com/php/php-src/blob/345e04b619c3bc11ea17ee02cdecad6ae8ce5891/main/SAPI.h#L72 +// +//export go_update_request_info +func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) { + thread := phpThreads[threadIndex] + fc := thread.getRequestContext() + request := fc.request + + authUser, authPassword, ok := request.BasicAuth() + if ok && authPassword != "" { + info.auth_password = thread.pinCString(authPassword) + } + if ok && authUser != "" { + info.auth_user = thread.pinCString(authUser) + } + + info.request_method = thread.pinCString(request.Method) + info.query_string = thread.pinCString(request.URL.RawQuery) + info.content_length = C.zend_long(request.ContentLength) + + contentType := request.Header.Get("Content-Type") + if contentType != "" { + info.content_type = thread.pinCString(contentType) + } + + if fc.pathInfo != "" { + info.path_translated = thread.pinCString(sanitizedPathJoin(fc.documentRoot, fc.pathInfo)) // Info: http://www.oreilly.com/openbook/cgi/ch02_04.html + } + + info.request_uri = thread.pinCString(request.URL.RequestURI()) + + info.proto_num = C.int(request.ProtoMajor*1000 + request.ProtoMinor) +} + +//export go_is_worker_request +func go_is_worker_request(threadIndex C.uintptr_t) C.bool { + thread := phpThreads[threadIndex] + fc := thread.getRequestContext() + + return C.bool(fc.worker != nil) } // SanitizedPathJoin performs filepath.Join(root, reqPath) that diff --git a/frankenphp.c b/frankenphp.c index 102af2db7f..4435671ed6 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -75,6 +75,16 @@ __thread uintptr_t thread_index; __thread bool is_worker_thread = false; __thread zval *os_environment = NULL; +static void frankenphp_update_request_context(){ + // update server context here ? + SG(server_context) = (void *)1; + // It is not reset by zend engine, set it to 200. + SG(sapi_headers).http_response_code = 200; + + go_update_request_info(thread_index, &SG(request_info)); + is_worker_thread = go_is_worker_request(thread_index); +} + static void frankenphp_free_request_context() { if (SG(request_info).cookie_data != NULL) { free(SG(request_info).cookie_data); @@ -174,6 +184,8 @@ void frankenphp_add_assoc_str_ex(zval *track_vars_array, char *key, static int frankenphp_worker_request_startup() { int retval = SUCCESS; + frankenphp_update_request_context(); + zend_try { frankenphp_destroy_super_globals(); frankenphp_release_temporary_streams(); @@ -507,36 +519,8 @@ static void frankenphp_request_shutdown() { zval_ptr_dtor_nogc(&PG(http_globals)[TRACK_VARS_ENV]); array_init(&PG(http_globals)[TRACK_VARS_ENV]); } - php_request_shutdown((void *)0); frankenphp_free_request_context(); -} - -int frankenphp_update_server_context(bool is_worker_request, - - const char *request_method, - char *query_string, - zend_long content_length, - char *path_translated, char *request_uri, - const char *content_type, char *auth_user, - char *auth_password, int proto_num) { - - SG(server_context) = (void *)1; - is_worker_thread = is_worker_request; - - // It is not reset by zend engine, set it to 200. - SG(sapi_headers).http_response_code = 200; - - SG(request_info).auth_password = auth_password; - SG(request_info).auth_user = auth_user; - SG(request_info).request_method = request_method; - SG(request_info).query_string = query_string; - SG(request_info).content_type = content_type; - SG(request_info).content_length = content_length; - SG(request_info).path_translated = path_translated; - SG(request_info).request_uri = request_uri; - SG(request_info).proto_num = proto_num; - - return SUCCESS; + php_request_shutdown((void *)0); } static int frankenphp_startup(sapi_module_struct *sapi_module) { @@ -974,7 +958,8 @@ bool frankenphp_new_php_thread(uintptr_t thread_index) { return true; } -int frankenphp_request_startup() { +static int frankenphp_request_startup() { + frankenphp_update_request_context(); if (php_request_startup() == SUCCESS) { return SUCCESS; } @@ -1014,7 +999,6 @@ int frankenphp_execute_script(char *file_name) { zend_destroy_file_handle(&file_handle); - frankenphp_free_request_context(); frankenphp_request_shutdown(); return status; diff --git a/frankenphp.go b/frankenphp.go index 961521c1b9..8f0b0c529c 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -12,8 +12,6 @@ package frankenphp // // We also set these flags for hardening: https://github.com/docker-library/php/blob/master/8.2/bookworm/zts/Dockerfile#L57-L59 -// #cgo nocallback frankenphp_update_server_context -// #cgo noescape frankenphp_update_server_context // #include // #include // #include @@ -32,7 +30,6 @@ import ( "os" "os/signal" "runtime" - "strconv" "strings" "sync" "syscall" @@ -317,66 +314,6 @@ func Shutdown() { logger.Debug("FrankenPHP shut down") } -func updateServerContext(thread *phpThread, fc *frankenPHPContext, isWorkerRequest bool) error { - request := fc.request - authUser, authPassword, ok := request.BasicAuth() - var cAuthUser, cAuthPassword *C.char - if ok && authPassword != "" { - cAuthPassword = thread.pinCString(authPassword) - } - if ok && authUser != "" { - cAuthUser = thread.pinCString(authUser) - } - - cMethod := thread.pinCString(request.Method) - cQueryString := thread.pinCString(request.URL.RawQuery) - contentLengthStr := request.Header.Get("Content-Length") - contentLength := 0 - if contentLengthStr != "" { - var err error - contentLength, err = strconv.Atoi(contentLengthStr) - if err != nil || contentLength < 0 { - return fmt.Errorf("invalid Content-Length header: %w", err) - } - } - - contentType := request.Header.Get("Content-Type") - var cContentType *C.char - if contentType != "" { - cContentType = thread.pinCString(contentType) - } - - // compliance with the CGI specification requires that - // PATH_TRANSLATED should only exist if PATH_INFO is defined. - // Info: https://www.ietf.org/rfc/rfc3875 Page 14 - var cPathTranslated *C.char - if fc.pathInfo != "" { - cPathTranslated = thread.pinCString(sanitizedPathJoin(fc.documentRoot, fc.pathInfo)) // Info: http://www.oreilly.com/openbook/cgi/ch02_04.html - } - - cRequestUri := thread.pinCString(request.URL.RequestURI()) - - ret := C.frankenphp_update_server_context( - C.bool(isWorkerRequest || fc.responseWriter == nil), - - cMethod, - cQueryString, - C.zend_long(contentLength), - cPathTranslated, - cRequestUri, - cContentType, - cAuthUser, - cAuthPassword, - C.int(request.ProtoMajor*1000+request.ProtoMinor), - ) - - if ret > 0 { - return ErrRequestContextCreation - } - - return nil -} - // ServeHTTP executes a PHP script according to the given context and options. func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { if !isRunning { From 1abb5b0d58c08e2663239ea1f2b30ff0b853273a Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 3 Aug 2025 14:51:22 +0200 Subject: [PATCH 04/16] Calls 'update_request_info' from the C side. --- context.go | 18 ++++++++++++++---- frankenphp.h | 8 -------- threadregular.go | 8 -------- threadworker.go | 14 +------------- 4 files changed, 15 insertions(+), 33 deletions(-) diff --git a/context.go b/context.go index 8205450e60..8fe7bb97d5 100644 --- a/context.go +++ b/context.go @@ -4,6 +4,7 @@ import ( "log/slog" "net/http" "os" + "strconv" "strings" "time" ) @@ -95,13 +96,22 @@ func (fc *frankenPHPContext) closeContext() { // validate checks if the request should be outright rejected func (fc *frankenPHPContext) validate() bool { - if !strings.Contains(fc.request.URL.Path, "\x00") { - return true + if strings.Contains(fc.request.URL.Path, "\x00") { + fc.rejectBadRequest("Invalid request path") + return false } - fc.rejectBadRequest("Invalid request path") + contentLengthStr := fc.request.Header.Get("Content-Length") + if contentLengthStr != "" { + contentLength, err := strconv.Atoi(contentLengthStr) + if err != nil || contentLength < 0 { + fc.rejectBadRequest("invalid Content-Length header: " + contentLengthStr) + + return false + } + } - return false + return true } func (fc *frankenPHPContext) clientHasClosed() bool { diff --git a/frankenphp.h b/frankenphp.h index 246a221d65..bd5022db97 100644 --- a/frankenphp.h +++ b/frankenphp.h @@ -51,14 +51,6 @@ int frankenphp_new_main_thread(int num_threads); bool frankenphp_new_php_thread(uintptr_t thread_index); bool frankenphp_shutdown_dummy_request(void); -int frankenphp_update_server_context(bool is_worker_request, - - const char *request_method, - char *query_string, - zend_long content_length, - char *path_translated, char *request_uri, - const char *content_type, char *auth_user, - char *auth_password, int proto_num); int frankenphp_request_startup(); int frankenphp_execute_script(char *file_name); diff --git a/threadregular.go b/threadregular.go index 4ee7da95f7..88cef7e79d 100644 --- a/threadregular.go +++ b/threadregular.go @@ -75,14 +75,6 @@ func (handler *regularThread) waitForRequest() string { handler.requestContext = fc handler.state.markAsWaiting(false) - if err := updateServerContext(handler.thread, fc, false); err != nil { - fc.rejectBadRequest(err.Error()) - handler.afterRequest() - handler.thread.Unpin() - // go back to beforeScriptExecution - return handler.beforeScriptExecution() - } - // set the scriptFilename that should be executed return fc.scriptFilename } diff --git a/threadworker.go b/threadworker.go index 212863d536..b7dc820367 100644 --- a/threadworker.go +++ b/threadworker.go @@ -91,10 +91,7 @@ func setupWorkerScript(handler *workerThread, worker *worker) { panic(err) } - if err := updateServerContext(handler.thread, fc, false); err != nil { - panic(err) - } - + fc.worker = worker handler.dummyContext = fc handler.isBootingScript = true clearSandboxedEnv(handler.thread) @@ -192,15 +189,6 @@ func (handler *workerThread) waitForWorkerRequest() bool { logger.LogAttrs(ctx, slog.LevelDebug, "request handling started", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex), slog.String("url", fc.request.RequestURI)) - if err := updateServerContext(handler.thread, fc, true); err != nil { - // Unexpected error or invalid request - logger.LogAttrs(ctx, slog.LevelDebug, "unexpected error", slog.String("worker", handler.worker.name), slog.Int("thread", handler.thread.threadIndex), slog.String("url", fc.request.RequestURI), slog.Any("error", err)) - fc.rejectBadRequest(err.Error()) - handler.workerContext = nil - - return handler.waitForWorkerRequest() - } - return true } From 6cdaa3e2986eaf2dac45bae528adc870084a70c4 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 3 Aug 2025 14:54:40 +0200 Subject: [PATCH 05/16] clang-format --- frankenphp.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index 4435671ed6..bb1dd2bec7 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -75,10 +75,11 @@ __thread uintptr_t thread_index; __thread bool is_worker_thread = false; __thread zval *os_environment = NULL; -static void frankenphp_update_request_context(){ - // update server context here ? +static void frankenphp_update_request_context() { + /* the server context is stored on the go side, still SG(server_context) needs + * to not be NULL */ SG(server_context) = (void *)1; - // It is not reset by zend engine, set it to 200. + /* status It is not reset by zend engine, set it to 200. */ SG(sapi_headers).http_response_code = 200; go_update_request_info(thread_index, &SG(request_info)); From 50fd3e1cf392eaa8d378df30d5401e316a51d0ca Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 3 Aug 2025 15:04:00 +0200 Subject: [PATCH 06/16] Removes unnecessary export. --- frankenphp.h | 1 - 1 file changed, 1 deletion(-) diff --git a/frankenphp.h b/frankenphp.h index bd5022db97..c17df6061a 100644 --- a/frankenphp.h +++ b/frankenphp.h @@ -51,7 +51,6 @@ int frankenphp_new_main_thread(int num_threads); bool frankenphp_new_php_thread(uintptr_t thread_index); bool frankenphp_shutdown_dummy_request(void); -int frankenphp_request_startup(); int frankenphp_execute_script(char *file_name); int frankenphp_execute_script_cli(char *script, int argc, char **argv, From 4014b0baf2597978657860e9c79452ebd3289853 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 3 Aug 2025 15:08:20 +0200 Subject: [PATCH 07/16] Adds TODO. --- context.go | 1 + 1 file changed, 1 insertion(+) diff --git a/context.go b/context.go index 8fe7bb97d5..08fbd61cfa 100644 --- a/context.go +++ b/context.go @@ -101,6 +101,7 @@ func (fc *frankenPHPContext) validate() bool { return false } + // TODO: Caddy already handles this, is it necessary? contentLengthStr := fc.request.Header.Get("Content-Length") if contentLengthStr != "" { contentLength, err := strconv.Atoi(contentLengthStr) From 54282da3235aabb649476f4178a0ef997450d2cc Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 3 Aug 2025 15:12:49 +0200 Subject: [PATCH 08/16] Adds TODO. --- cgi.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cgi.go b/cgi.go index 727b2a3895..429c6c1306 100644 --- a/cgi.go +++ b/cgi.go @@ -250,6 +250,7 @@ func splitCgiPath(fc *frankenPHPContext) { return } + // 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) fc.worker = getWorkerByPath(fc.scriptFilename) @@ -257,6 +258,8 @@ func splitCgiPath(fc *frankenPHPContext) { // splitPos returns the index where path should // be split based on SplitPath. +// example: if splitPath is [".php"] +// "/path/to/script.php/some/path" => "/path/to/script.php" + "/some/path" // // Adapted from https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go // Copyright 2015 Matthew Holt and The Caddy Authors From 1bcf3d89d9d4733810fd8ad67e134461edc0b6b5 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sun, 10 Aug 2025 13:39:33 +0200 Subject: [PATCH 09/16] Removes 'is_worker_thread' --- frankenphp.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/frankenphp.c b/frankenphp.c index bb1dd2bec7..d00c4bc02e 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -72,7 +72,6 @@ frankenphp_config frankenphp_get_config() { bool should_filter_var = 0; __thread uintptr_t thread_index; -__thread bool is_worker_thread = false; __thread zval *os_environment = NULL; static void frankenphp_update_request_context() { @@ -83,7 +82,6 @@ static void frankenphp_update_request_context() { SG(sapi_headers).http_response_code = 200; go_update_request_info(thread_index, &SG(request_info)); - is_worker_thread = go_is_worker_request(thread_index); } static void frankenphp_free_request_context() { @@ -404,7 +402,7 @@ PHP_FUNCTION(frankenphp_handle_request) { Z_PARAM_FUNC(fci, fcc) ZEND_PARSE_PARAMETERS_END(); - if (!is_worker_thread) { + if (!go_is_worker_request(thread_index)) { /* not a worker, throw an error */ zend_throw_exception( spl_ce_RuntimeException, @@ -515,7 +513,7 @@ static zend_module_entry frankenphp_module = { STANDARD_MODULE_PROPERTIES}; static void frankenphp_request_shutdown() { - if (is_worker_thread) { + if (go_is_worker_request(thread_index)) { /* ensure $_ENV is not in an invalid state before shutdown */ zval_ptr_dtor_nogc(&PG(http_globals)[TRACK_VARS_ENV]); array_init(&PG(http_globals)[TRACK_VARS_ENV]); @@ -750,7 +748,7 @@ static void frankenphp_register_variables(zval *track_vars_array) { */ /* in non-worker mode we import the os environment regularly */ - if (!is_worker_thread) { + if (!go_is_worker_request(thread_index)) { get_full_env(track_vars_array); // php_import_environment_variables(track_vars_array); go_register_variables(thread_index, track_vars_array); From 72cb3a39fe06394cfcfcea20f10aa3f643675f4c Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 16 Aug 2025 17:03:53 +0200 Subject: [PATCH 10/16] Shortens return statement. --- cgi.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/cgi.go b/cgi.go index 429c6c1306..da657e8d3c 100644 --- a/cgi.go +++ b/cgi.go @@ -314,10 +314,7 @@ func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) //export go_is_worker_request func go_is_worker_request(threadIndex C.uintptr_t) C.bool { - thread := phpThreads[threadIndex] - fc := thread.getRequestContext() - - return C.bool(fc.worker != nil) + return C.bool(phpThreads[threadIndex].getRequestContext().worker != nil) } // SanitizedPathJoin performs filepath.Join(root, reqPath) that From 470d2ce0a97a55142e3dae72d0735baafce61e8c Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 16 Aug 2025 21:57:30 +0200 Subject: [PATCH 11/16] Removes the context refactor. --- caddy/module.go | 7 ++++--- context.go | 28 ++++++++++++++++++--------- frankenphp.go | 12 +++++++----- frankenphp_test.go | 38 +++++++++++++++++++++++++++++-------- internal/testext/exttest.go | 5 ++++- internal/testserver/main.go | 7 ++++++- phpmainthread_test.go | 4 +++- worker_test.go | 7 ++++++- 8 files changed, 79 insertions(+), 29 deletions(-) diff --git a/caddy/module.go b/caddy/module.go index 2579e4ab32..dfd78a0a5a 100644 --- a/caddy/module.go +++ b/caddy/module.go @@ -185,15 +185,16 @@ func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ c } } - if err := frankenphp.ServeHTTP( - w, + fr, err := frankenphp.NewRequestWithContext( r, documentRootOption, frankenphp.WithRequestSplitPath(f.SplitPath), frankenphp.WithRequestPreparedEnv(env), frankenphp.WithOriginalRequest(&origReq), frankenphp.WithWorkerName(workerName), - ); err != nil { + ) + + if err = frankenphp.ServeHTTP(w, fr); err != nil { return caddyhttp.Error(http.StatusInternalServerError, err) } diff --git a/context.go b/context.go index c63161af2d..8b41a1d8b0 100644 --- a/context.go +++ b/context.go @@ -1,6 +1,7 @@ package frankenphp import ( + "context" "log/slog" "net/http" "os" @@ -33,13 +34,18 @@ type frankenPHPContext struct { startedAt time.Time } -// newFrankenPHPContext creates a new FrankenPHP request context. -func newFrankenPHPContext(rw http.ResponseWriter, r *http.Request, opts ...RequestOption) (*frankenPHPContext, error) { +// fromContext extracts the frankenPHPContext from a context. +func fromContext(ctx context.Context) (fctx *frankenPHPContext, ok bool) { + fctx, ok = ctx.Value(contextKey).(*frankenPHPContext) + return +} + +// NewRequestWithContext creates a new FrankenPHP request context. +func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Request, error) { fc := &frankenPHPContext{ - done: make(chan any), - startedAt: time.Now(), - request: r, - responseWriter: rw, + done: make(chan any), + startedAt: time.Now(), + request: r, } for _, o := range opts { if err := o(fc); err != nil { @@ -66,21 +72,25 @@ func newFrankenPHPContext(rw http.ResponseWriter, r *http.Request, opts ...Reque // this ensures the correct worker is assigned by path if necessary splitCgiPath(fc) - return fc, nil + c := context.WithValue(r.Context(), contextKey, fc) + + return r.WithContext(c), nil } -// newDummyContext creates a fake context from just a request path. +// newDummyContext creates a fake context from a request path. func newDummyContext(requestPath string, opts ...RequestOption) (*frankenPHPContext, error) { r, err := http.NewRequest(http.MethodGet, requestPath, nil) if err != nil { return nil, err } - fc, err := newFrankenPHPContext(nil, r, opts...) + fr, err := NewRequestWithContext(r, opts...) if err != nil { return nil, err } + fc, _ := fromContext(fr.Context()) + return fc, nil } diff --git a/frankenphp.go b/frankenphp.go index 79e978eec3..34ce443fb5 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -314,17 +314,19 @@ func Shutdown() { logger.Debug("FrankenPHP shut down") } -// ServeHTTP executes a PHP script according to the given context and options. -func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request, opts ...RequestOption) error { +// ServeHTTP executes a PHP script according to the given context. +func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error { if !isRunning { return ErrNotRunning } - fc, err := newFrankenPHPContext(responseWriter, request, opts...) - if err != nil { - return fmt.Errorf("%w: %s", ErrRequestContextCreation, err.Error()) + fc, ok := fromContext(request.Context()) + if !ok { + return ErrInvalidRequest } + fc.responseWriter = responseWriter + if !fc.validate() { return nil } diff --git a/frankenphp_test.go b/frankenphp_test.go index 9229a40a7a..7b4b44dbce 100644 --- a/frankenphp_test.go +++ b/frankenphp_test.go @@ -82,7 +82,10 @@ func runTest(t *testing.T, test func(func(http.ResponseWriter, *http.Request), * defer frankenphp.Shutdown() handler := func(w http.ResponseWriter, r *http.Request) { - err = frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot(testDataDir, false)) + req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(testDataDir, false)) + assert.NoError(t, err) + + err = frankenphp.ServeHTTP(w, req) assert.NoError(t, err) } @@ -207,13 +210,14 @@ func testPathInfo(t *testing.T, opts *testOptions) { requestURI := r.URL.RequestURI() r.URL.Path = path - err := frankenphp.ServeHTTP( - w, - r, + rewriteRequest, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(testDataDir, false), frankenphp.WithRequestEnv(map[string]string{"REQUEST_URI": requestURI}), ) assert.NoError(t, err) + + err = frankenphp.ServeHTTP(w, rewriteRequest) + assert.NoError(t, err) } body, _ := testGet(fmt.Sprintf("http://example.com/pathinfo/%d", i), handler, t) @@ -733,7 +737,12 @@ func ExampleServeHTTP() { defer frankenphp.Shutdown() http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot("/path/to/document/root", false)); err != nil { + req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot("/path/to/document/root", false)) + if err != nil { + panic(err) + } + + if err := frankenphp.ServeHTTP(w, req); err != nil { panic(err) } }) @@ -758,7 +767,12 @@ func BenchmarkHelloWorld(b *testing.B) { testDataDir := cwd + "/testdata/" handler := func(w http.ResponseWriter, r *http.Request) { - if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot(testDataDir, false)); err != nil { + req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(testDataDir, false)) + if err != nil { + panic(err) + } + + if err := frankenphp.ServeHTTP(w, req); err != nil { panic(err) } } @@ -780,7 +794,11 @@ func BenchmarkEcho(b *testing.B) { testDataDir := cwd + "/testdata/" handler := func(w http.ResponseWriter, r *http.Request) { - if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot(testDataDir, false)); err != nil { + req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(testDataDir, false)) + if err != nil { + panic(err) + } + if err := frankenphp.ServeHTTP(w, req); err != nil { panic(err) } } @@ -883,9 +901,13 @@ func BenchmarkServerSuperGlobal(b *testing.B) { preparedEnv := frankenphp.PrepareEnv(env) handler := func(w http.ResponseWriter, r *http.Request) { + req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot(testDataDir, false), frankenphp.WithRequestPreparedEnv(preparedEnv)) + if err != nil { + panic(err) + } r.Header = headers - if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot(testDataDir, false), frankenphp.WithRequestPreparedEnv(preparedEnv)); err != nil { + if err := frankenphp.ServeHTTP(w, req); err != nil { panic(err) } } diff --git a/internal/testext/exttest.go b/internal/testext/exttest.go index 862b6a09b1..abebee4c1d 100644 --- a/internal/testext/exttest.go +++ b/internal/testext/exttest.go @@ -31,7 +31,10 @@ func testRegisterExtension(t *testing.T) { req := httptest.NewRequest("GET", "http://example.com/index.php", nil) w := httptest.NewRecorder() - err = frankenphp.ServeHTTP(w, req, frankenphp.WithRequestDocumentRoot("./testdata", false)) + req, err = frankenphp.NewRequestWithContext(req, frankenphp.WithRequestDocumentRoot("./testdata", false)) + assert.NoError(t, err) + + err = frankenphp.ServeHTTP(w, req) assert.NoError(t, err) resp := w.Result() diff --git a/internal/testserver/main.go b/internal/testserver/main.go index eadf927ded..3e4af37b15 100644 --- a/internal/testserver/main.go +++ b/internal/testserver/main.go @@ -17,7 +17,12 @@ func main() { defer frankenphp.Shutdown() http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - if err := frankenphp.ServeHTTP(w, r); err != nil { + req, err := frankenphp.NewRequestWithContext(r) + if err != nil { + panic(err) + } + + if err := frankenphp.ServeHTTP(w, req); err != nil { panic(err) } }) diff --git a/phpmainthread_test.go b/phpmainthread_test.go index ab149d8c3e..18012a41a7 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -225,7 +225,9 @@ func assertRequestBody(t *testing.T, url string, expected string) { r := httptest.NewRequest("GET", url, nil) w := httptest.NewRecorder() - err := ServeHTTP(w, r, WithRequestDocumentRoot(testDataPath, false)) + req, err := NewRequestWithContext(r, WithRequestDocumentRoot(testDataPath, false)) + assert.NoError(t, err) + err = ServeHTTP(w, req) assert.NoError(t, err) resp := w.Result() body, _ := io.ReadAll(resp.Body) diff --git a/worker_test.go b/worker_test.go index 83c43ab254..c6b4245d47 100644 --- a/worker_test.go +++ b/worker_test.go @@ -137,7 +137,12 @@ func ExampleServeHTTP_workers() { defer frankenphp.Shutdown() http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) { - if err := frankenphp.ServeHTTP(w, r, frankenphp.WithRequestDocumentRoot("/path/to/document/root", false)); err != nil { + req, err := frankenphp.NewRequestWithContext(r, frankenphp.WithRequestDocumentRoot("/path/to/document/root", false)) + if err != nil { + panic(err) + } + + if err := frankenphp.ServeHTTP(w, req); err != nil { panic(err) } }) From 4d4ed3898b76e83d8fd6748d8900b9dede548d43 Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Sat, 16 Aug 2025 22:25:47 +0200 Subject: [PATCH 12/16] adjusts comment. --- context.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/context.go b/context.go index 8b41a1d8b0..f720cb74b2 100644 --- a/context.go +++ b/context.go @@ -111,7 +111,7 @@ func (fc *frankenPHPContext) validate() bool { return false } - // TODO: Caddy already handles this, is it necessary? + // TODO: Caddy already does this, probably unnecessary contentLengthStr := fc.request.Header.Get("Content-Length") if contentLengthStr != "" { contentLength, err := strconv.Atoi(contentLengthStr) From a11bf0b5ba0a758ba25d8fa4180a8b68f5d336ed Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Mon, 18 Aug 2025 21:38:05 +0200 Subject: [PATCH 13/16] Skips parsing cgi path variables on explicitly assigned worker. --- cgi.go | 6 ------ context.go | 11 ++++++++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/cgi.go b/cgi.go index da657e8d3c..644f3df831 100644 --- a/cgi.go +++ b/cgi.go @@ -244,12 +244,6 @@ func splitCgiPath(fc *frankenPHPContext) { } } - // if a worker is already assigned explicitly, use its filename - if fc.worker != nil { - fc.scriptFilename = fc.worker.fileName - return - } - // 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) diff --git a/context.go b/context.go index f720cb74b2..1a69f978fc 100644 --- a/context.go +++ b/context.go @@ -68,9 +68,14 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques } } - // some cgi variables need to already be determined here - // this ensures the correct worker is assigned by path if necessary - splitCgiPath(fc) + // 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) + } c := context.WithValue(r.Context(), contextKey, fc) From 23c57c61c25e4ede85eb7da6df673fa4814f535a Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 21 Aug 2025 17:51:27 +0200 Subject: [PATCH 14/16] suggesions by @dunglas. --- cgi.go | 22 ++++++++++++---------- context.go | 7 +++---- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/cgi.go b/cgi.go index 644f3df831..bc33f39b29 100644 --- a/cgi.go +++ b/cgi.go @@ -21,7 +21,7 @@ import ( "github.com/dunglas/frankenphp/internal/phpheaders" ) -// Map of supported protocols to Apache ssl_mod format +// Protocol versions, in Apache mod_ssl format: https://httpd.apache.org/docs/current/mod/mod_ssl.html // Note that these are slightly different from SupportedProtocols in caddytls/config.go var tlsProtocolStrings = map[uint16]string{ tls.VersionTLS10: "TLSv1", @@ -30,7 +30,7 @@ var tlsProtocolStrings = map[uint16]string{ tls.VersionTLS13: "TLSv1.3", } -// List of all keys in the $_SERVER array excluding headers and environment variables. +// Known $_SERVER keys var knownServerKeys = []string{ "CONTENT_LENGTH", "DOCUMENT_ROOT", @@ -253,7 +253,7 @@ func splitCgiPath(fc *frankenPHPContext) { // splitPos returns the index where path should // be split based on SplitPath. // example: if splitPath is [".php"] -// "/path/to/script.php/some/path" => "/path/to/script.php" + "/some/path" +// "/path/to/script.php/some/path": ("/path/to/script.php", "/some/path") // // Adapted from https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go // Copyright 2015 Matthew Holt and The Caddy Authors @@ -271,8 +271,8 @@ func splitPos(path string, splitPath []string) int { return -1 } -// update the sapi_request_info struct -// see: https://github.com/php/php-src/blob/345e04b619c3bc11ea17ee02cdecad6ae8ce5891/main/SAPI.h#L72 +// go_update_request_info updates the sapi_request_info struct +// See: https://github.com/php/php-src/blob/345e04b619c3bc11ea17ee02cdecad6ae8ce5891/main/SAPI.h#L72 // //export go_update_request_info func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) { @@ -281,11 +281,13 @@ func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) request := fc.request authUser, authPassword, ok := request.BasicAuth() - if ok && authPassword != "" { - info.auth_password = thread.pinCString(authPassword) - } - if ok && authUser != "" { - info.auth_user = thread.pinCString(authUser) + if ok { + if authPassword != "" { + info.auth_password = thread.pinCString(authPassword) + } + if authUser != "" { + info.auth_user = thread.pinCString(authUser) + } } info.request_method = thread.pinCString(request.Method) diff --git a/context.go b/context.go index 1a69f978fc..4eb7a9b0a6 100644 --- a/context.go +++ b/context.go @@ -82,7 +82,7 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques return r.WithContext(c), nil } -// newDummyContext creates a fake context from a request path. +// newDummyContext creates a fake context from a request path func newDummyContext(requestPath string, opts ...RequestOption) (*frankenPHPContext, error) { r, err := http.NewRequest(http.MethodGet, requestPath, nil) if err != nil { @@ -113,14 +113,13 @@ func (fc *frankenPHPContext) closeContext() { func (fc *frankenPHPContext) validate() bool { if strings.Contains(fc.request.URL.Path, "\x00") { fc.rejectBadRequest("Invalid request path") + return false } - // TODO: Caddy already does this, probably unnecessary contentLengthStr := fc.request.Header.Get("Content-Length") if contentLengthStr != "" { - contentLength, err := strconv.Atoi(contentLengthStr) - if err != nil || contentLength < 0 { + if contentLength, err := strconv.Atoi(contentLengthStr); err != nil || contentLength < 0 { fc.rejectBadRequest("invalid Content-Length header: " + contentLengthStr) return false From d85857ae3912625ea075be603cce2eb4c71f097f Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 21 Aug 2025 18:16:42 +0200 Subject: [PATCH 15/16] Re-introduces 'is_worker_thread'. --- cgi.go | 7 ++----- frankenphp.c | 9 +++++---- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/cgi.go b/cgi.go index bc33f39b29..6a1482c576 100644 --- a/cgi.go +++ b/cgi.go @@ -275,7 +275,7 @@ func splitPos(path string, splitPath []string) int { // See: https://github.com/php/php-src/blob/345e04b619c3bc11ea17ee02cdecad6ae8ce5891/main/SAPI.h#L72 // //export go_update_request_info -func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) { +func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) C.bool { thread := phpThreads[threadIndex] fc := thread.getRequestContext() request := fc.request @@ -306,11 +306,8 @@ func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) info.request_uri = thread.pinCString(request.URL.RequestURI()) info.proto_num = C.int(request.ProtoMajor*1000 + request.ProtoMinor) -} -//export go_is_worker_request -func go_is_worker_request(threadIndex C.uintptr_t) C.bool { - return C.bool(phpThreads[threadIndex].getRequestContext().worker != nil) + return C.bool(fc.worker != nil) } // SanitizedPathJoin performs filepath.Join(root, reqPath) that diff --git a/frankenphp.c b/frankenphp.c index d00c4bc02e..b6300dbca1 100644 --- a/frankenphp.c +++ b/frankenphp.c @@ -72,6 +72,7 @@ frankenphp_config frankenphp_get_config() { bool should_filter_var = 0; __thread uintptr_t thread_index; +__thread bool is_worker_thread = false; __thread zval *os_environment = NULL; static void frankenphp_update_request_context() { @@ -81,7 +82,7 @@ static void frankenphp_update_request_context() { /* status It is not reset by zend engine, set it to 200. */ SG(sapi_headers).http_response_code = 200; - go_update_request_info(thread_index, &SG(request_info)); + is_worker_thread = go_update_request_info(thread_index, &SG(request_info)); } static void frankenphp_free_request_context() { @@ -402,7 +403,7 @@ PHP_FUNCTION(frankenphp_handle_request) { Z_PARAM_FUNC(fci, fcc) ZEND_PARSE_PARAMETERS_END(); - if (!go_is_worker_request(thread_index)) { + if (!is_worker_thread) { /* not a worker, throw an error */ zend_throw_exception( spl_ce_RuntimeException, @@ -513,7 +514,7 @@ static zend_module_entry frankenphp_module = { STANDARD_MODULE_PROPERTIES}; static void frankenphp_request_shutdown() { - if (go_is_worker_request(thread_index)) { + if (is_worker_thread) { /* ensure $_ENV is not in an invalid state before shutdown */ zval_ptr_dtor_nogc(&PG(http_globals)[TRACK_VARS_ENV]); array_init(&PG(http_globals)[TRACK_VARS_ENV]); @@ -748,7 +749,7 @@ static void frankenphp_register_variables(zval *track_vars_array) { */ /* in non-worker mode we import the os environment regularly */ - if (!go_is_worker_request(thread_index)) { + if (!is_worker_thread) { get_full_env(track_vars_array); // php_import_environment_variables(track_vars_array); go_register_variables(thread_index, track_vars_array); From ec85146b79ceeba5450d4e94f850dc4b1f8c4c5f Mon Sep 17 00:00:00 2001 From: Alliballibaba Date: Thu, 21 Aug 2025 18:21:19 +0200 Subject: [PATCH 16/16] More formatting. --- cgi.go | 5 ++--- context.go | 6 +++--- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/cgi.go b/cgi.go index 6a1482c576..f9aae8690c 100644 --- a/cgi.go +++ b/cgi.go @@ -294,13 +294,12 @@ func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) info.query_string = thread.pinCString(request.URL.RawQuery) info.content_length = C.zend_long(request.ContentLength) - contentType := request.Header.Get("Content-Type") - if contentType != "" { + if contentType := request.Header.Get("Content-Type"); contentType != "" { info.content_type = thread.pinCString(contentType) } if fc.pathInfo != "" { - info.path_translated = thread.pinCString(sanitizedPathJoin(fc.documentRoot, fc.pathInfo)) // Info: http://www.oreilly.com/openbook/cgi/ch02_04.html + info.path_translated = thread.pinCString(sanitizedPathJoin(fc.documentRoot, fc.pathInfo)) // See: http://www.oreilly.com/openbook/cgi/ch02_04.html } info.request_uri = thread.pinCString(request.URL.RequestURI()) diff --git a/context.go b/context.go index 4eb7a9b0a6..65aee5b757 100644 --- a/context.go +++ b/context.go @@ -68,12 +68,12 @@ 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 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 + // 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) }