perf: optimized request headers (#1335)

* Optimizes header registration.

* Adds malformed cookie tests.

* Sets key to NULL (releasing them is unnecessary)

* Adjusts test.

* Sanitizes null bytes anyways.

* Sorts headers.

* trigger

* clang-format

* More clang-format.

* Updates headers and tests.

* Adds header test.

* Adds more headers.

* Updates headers again.

* ?Removes comments.

* ?Reformats headers

* ?Reformats headers

* renames header files.

* ?Renames test.

* ?Fixes assertion.

* test

* test

* test

* Moves headers test to main package.

* Properly capitalizes headers.

* Allows and tests multiple cookie headers.

* Fixes comment.

* Adds otter back in.

* Verifies correct capitalization.

* Resets package version.

* Removes debug log.

* Makes persistent strings also interned and saves them once on the main thread.

---------

Co-authored-by: Alliballibaba <alliballibaba@gmail.com>

Author: AlliBalliBaba

cgi.go

@@ -10,6 +10,8 @@ import (
 	"path/filepath"
 	"strings"
 	"unsafe"
+
+	"github.com/dunglas/frankenphp/internal/phpheaders"
 )
 
 var knownServerKeys = []string{
@@ -47,7 +49,7 @@ var knownServerKeys = []string{
 // TODO: handle this case https://github.com/caddyserver/caddy/issues/3718
 // Inspired by https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go
 func addKnownVariablesToServer(thread *phpThread, request *http.Request, fc *FrankenPHPContext, trackVarsArray *C.zval) {
-	keys := getKnownVariableKeys(thread)
+	keys := mainThread.knownServerKeys
 	// Separate remote IP and port; more lenient than net.SplitHostPort
 	var ip, port string
 	if idx := strings.LastIndex(request.RemoteAddr, ":"); idx > -1 {
@@ -158,18 +160,17 @@ func packCgiVariable(key *C.zend_string, value string) C.ht_key_value_pair {
 	return C.ht_key_value_pair{key, toUnsafeChar(value), C.size_t(len(value))}
 }
 
-func addHeadersToServer(request *http.Request, fc *FrankenPHPContext, trackVarsArray *C.zval) {
+func addHeadersToServer(request *http.Request, thread *phpThread, fc *FrankenPHPContext, trackVarsArray *C.zval) {
 	for field, val := range request.Header {
-		k, ok := headerKeyCache.Get(field)
-		if !ok {
-			k = "HTTP_" + headerNameReplacer.Replace(strings.ToUpper(field)) + "\x00"
-			headerKeyCache.SetIfAbsent(field, k)
-		}
-
-		if _, ok := fc.env[k]; ok {
+		if k := mainThread.commonHeaders[field]; k != nil {
+			v := strings.Join(val, ", ")
+			C.frankenphp_register_single(k, toUnsafeChar(v), C.size_t(len(v)), trackVarsArray)
 			continue
 		}
 
+		// if the header name could not be cached, it needs to be registered safely
+		// this is more inefficient but allows additional sanitizing by PHP
+		k := phpheaders.GetUnCommonHeader(field)
 		v := strings.Join(val, ", ")
 		C.frankenphp_register_variable_safe(toUnsafeChar(k), toUnsafeChar(v), C.size_t(len(v)), trackVarsArray)
 	}
@@ -182,39 +183,17 @@ func addPreparedEnvToServer(fc *FrankenPHPContext, trackVarsArray *C.zval) {
 	fc.env = nil
 }
 
-func getKnownVariableKeys(thread *phpThread) map[string]*C.zend_string {
-	if thread.knownVariableKeys != nil {
-		return thread.knownVariableKeys
-	}
-	threadServerKeys := make(map[string]*C.zend_string)
-	for _, k := range knownServerKeys {
-		threadServerKeys[k] = C.frankenphp_init_persistent_string(toUnsafeChar(k), C.size_t(len(k)))
-	}
-	thread.knownVariableKeys = threadServerKeys
-	return threadServerKeys
-}
-
 //export go_register_variables
 func go_register_variables(threadIndex C.uintptr_t, trackVarsArray *C.zval) {
 	thread := phpThreads[threadIndex]
 	r := thread.getActiveRequest()
 	fc := r.Context().Value(contextKey).(*FrankenPHPContext)
 
 	addKnownVariablesToServer(thread, r, fc, trackVarsArray)
-	addHeadersToServer(r, fc, trackVarsArray)
-	addPreparedEnvToServer(fc, trackVarsArray)
-}
+	addHeadersToServer(r, thread, fc, trackVarsArray)
 
-//export go_frankenphp_release_known_variable_keys
-func go_frankenphp_release_known_variable_keys(threadIndex C.uintptr_t) {
-	thread := phpThreads[threadIndex]
-	if thread.knownVariableKeys == nil {
-		return
-	}
-	for _, v := range thread.knownVariableKeys {
-		C.frankenphp_release_zend_string(v)
-	}
-	thread.knownVariableKeys = nil
+	// The Prepared Environment is registered last and can overwrite any previous values
+	addPreparedEnvToServer(fc, trackVarsArray)
 }
 
 // splitPos returns the index where path should
@@ -245,8 +224,6 @@ var tlsProtocolStrings = map[uint16]string{
 	tls.VersionTLS13: "TLSv1.3",
 }
 
-var headerNameReplacer = strings.NewReplacer(" ", "_", "-", "_")
-
 // SanitizedPathJoin performs filepath.Join(root, reqPath) that
 // is safe against directory traversal attacks. It uses logic
 // similar to that in the Go standard library, specifically

frankenphp.c

@@ -683,6 +683,12 @@ void frankenphp_register_trusted_var(zend_string *z_key, char *value,
   }
 }
 
+void frankenphp_register_single(zend_string *z_key, char *value, size_t val_len,
+                                zval *track_vars_array) {
+  HashTable *ht = Z_ARRVAL_P(track_vars_array);
+  frankenphp_register_trusted_var(z_key, value, val_len, ht);
+}
+
 /* Register known $_SERVER variables in bulk to avoid cgo overhead */
 void frankenphp_register_bulk(
     zval *track_vars_array, ht_key_value_pair remote_addr,
@@ -743,10 +749,15 @@ void frankenphp_register_bulk(
                                   request_uri.val_len, ht);
 }
 
-/** Persistent strings are ignored by the PHP GC, we have to release these
- * ourselves **/
+/** Create an immutable zend_string that lasts for the whole process **/
 zend_string *frankenphp_init_persistent_string(const char *string, size_t len) {
-  return zend_string_init(string, len, 1);
+  /* persistent strings will be ignored by the GC at the end of a request */
+  zend_string *z_string = zend_string_init(string, len, 1);
+
+  /* interned strings will not be ref counted by the GC */
+  GC_ADD_FLAGS(z_string, IS_STR_INTERNED);
+
+  return z_string;
 }
 
 void frankenphp_release_zend_string(zend_string *z_string) {
@@ -920,8 +931,6 @@ static void *php_thread(void *arg) {
                                          frankenphp_execute_script(scriptName));
   }
 
-  go_frankenphp_release_known_variable_keys(thread_index);
-
 #ifdef ZTS
   ts_free_thread();
 #endif

frankenphp.go

@@ -43,7 +43,6 @@ import (
 	"time"
 	"unsafe"
 
-	"github.com/maypok86/otter"
 	"go.uber.org/zap"
 	"go.uber.org/zap/zapcore"
 	// debug on Linux
@@ -524,17 +523,6 @@ func go_ub_write(threadIndex C.uintptr_t, cBuf *C.char, length C.int) (C.size_t,
 	return C.size_t(i), C.bool(clientHasClosed(r))
 }
 
-// There are around 60 common request headers according to https://en.wikipedia.org/wiki/List_of_HTTP_header_fields#Request_fields
-// Give some space for custom headers
-var headerKeyCache = func() otter.Cache[string, string] {
-	c, err := otter.MustBuilder[string, string](256).Build()
-	if err != nil {
-		panic(err)
-	}
-
-	return c
-}()
-
 //export go_apache_request_headers
 func go_apache_request_headers(threadIndex C.uintptr_t, hasActiveRequest bool) (*C.go_string, C.size_t) {
 	thread := phpThreads[threadIndex]
@@ -650,19 +638,17 @@ func go_read_post(threadIndex C.uintptr_t, cBuf *C.char, countBytes C.size_t) (r
 
 //export go_read_cookies
 func go_read_cookies(threadIndex C.uintptr_t) *C.char {
-	r := phpThreads[threadIndex].getActiveRequest()
-
-	cookies := r.Cookies()
-	if len(cookies) == 0 {
+	cookies := phpThreads[threadIndex].getActiveRequest().Header.Values("Cookie")
+	cookie := strings.Join(cookies, "; ")
+	if cookie == "" {
 		return nil
 	}
-	cookieStrings := make([]string, len(cookies))
-	for i, cookie := range cookies {
-		cookieStrings[i] = cookie.String()
-	}
+
+	// remove potential null bytes
+	cookie = strings.ReplaceAll(cookie, "\x00", "")
 
 	// freed in frankenphp_free_request_context()
-	return C.CString(strings.Join(cookieStrings, "; "))
+	return C.CString(cookie)
 }
 
 //export go_log

frankenphp.h

@@ -72,6 +72,8 @@ zend_string *frankenphp_init_persistent_string(const char *string, size_t len);
 void frankenphp_release_zend_string(zend_string *z_string);
 int frankenphp_reset_opcache(void);
 
+void frankenphp_register_single(zend_string *z_key, char *value, size_t val_len,
+                                zval *track_vars_array);
 void frankenphp_register_bulk(
     zval *track_vars_array, ht_key_value_pair remote_addr,
     ht_key_value_pair remote_host, ht_key_value_pair remote_port,

frankenphp_test.go

@@ -321,6 +321,31 @@ func testCookies(t *testing.T, opts *testOptions) {
 	}, opts)
 }
 
+func TestMalformedCookie(t *testing.T) {
+	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) {
+		req := httptest.NewRequest("GET", "http://example.com/cookies.php", nil)
+		req.Header.Add("Cookie", "foo =bar; ===;;==;  .dot.=val  ;\x00 ; PHPSESSID=1234")
+		// Muliple Cookie header should be joined https://www.rfc-editor.org/rfc/rfc7540#section-8.1.2.5
+		req.Header.Add("Cookie", "secondCookie=test; secondCookie=overwritten")
+		w := httptest.NewRecorder()
+		handler(w, req)
+
+		resp := w.Result()
+		body, _ := io.ReadAll(resp.Body)
+
+		assert.Contains(t, string(body), "'foo_' => 'bar'")
+		assert.Contains(t, string(body), "'_dot_' => 'val  '")
+
+		// PHPSESSID should still be present since we remove the null byte
+		assert.Contains(t, string(body), "'PHPSESSID' => '1234'")
+
+		// The cookie in the second headers should be present
+		// but it should not be overwritten by following values
+		assert.Contains(t, string(body), "'secondCookie' => 'test'")
+
+	}, &testOptions{nbParallelRequests: 1})
+}
+
 func TestSession_module(t *testing.T) { testSession(t, nil) }
 func TestSession_worker(t *testing.T) {
 	testSession(t, &testOptions{workerScript: "session.php"})

internal/phpheaders/phpheaders.go

@@ -0,0 +1,136 @@
+package phpheaders
+
+import (
+	"strings"
+
+	"github.com/maypok86/otter"
+)
+
+// Translate header names to PHP header names
+// All headers in 'commonHeaders' can be cached and registered safely
+// All other headers must be sanitized
+// Note: net/http will capitalize lowercase headers, so we don't need to worry about case sensitivity
+var CommonRequestHeaders = map[string]string{
+	"Accept":                            "HTTP_ACCEPT",
+	"Accept-Charset":                    "HTTP_ACCEPT_CHARSET",
+	"Accept-Encoding":                   "HTTP_ACCEPT_ENCODING",
+	"Accept-Language":                   "HTTP_ACCEPT_LANGUAGE",
+	"Access-Control-Request-Headers":    "HTTP_ACCESS_CONTROL_REQUEST_HEADERS",
+	"Access-Control-Request-Method":     "HTTP_ACCESS_CONTROL_REQUEST_METHOD",
+	"Authorization":                     "HTTP_AUTHORIZATION",
+	"Cache-Control":                     "HTTP_CACHE_CONTROL",
+	"Connection":                        "HTTP_CONNECTION",
+	"Content-Disposition":               "HTTP_CONTENT_DISPOSITION",
+	"Content-Encoding":                  "HTTP_CONTENT_ENCODING",
+	"Content-Length":                    "HTTP_CONTENT_LENGTH",
+	"Content-Type":                      "HTTP_CONTENT_TYPE",
+	"Cookie":                            "HTTP_COOKIE",
+	"Date":                              "HTTP_DATE",
+	"Device-Memory":                     "HTTP_DEVICE_MEMORY",
+	"Dnt":                               "HTTP_DNT",
+	"Downlink":                          "HTTP_DOWNLINK",
+	"Dpr":                               "HTTP_DPR",
+	"Early-Data":                        "HTTP_EARLY_DATA",
+	"Ect":                               "HTTP_ECT",
+	"Am-I":                              "HTTP_AM_I",
+	"Expect":                            "HTTP_EXPECT",
+	"Forwarded":                         "HTTP_FORWARDED",
+	"From":                              "HTTP_FROM",
+	"Host":                              "HTTP_HOST",
+	"If-Match":                          "HTTP_IF_MATCH",
+	"If-Modified-Since":                 "HTTP_IF_MODIFIED_SINCE",
+	"If-None-Match":                     "HTTP_IF_NONE_MATCH",
+	"If-Range":                          "HTTP_IF_RANGE",
+	"If-Unmodified-Since":               "HTTP_IF_UNMODIFIED_SINCE",
+	"Keep-Alive":                        "HTTP_KEEP_ALIVE",
+	"Max-Forwards":                      "HTTP_MAX_FORWARDS",
+	"Origin":                            "HTTP_ORIGIN",
+	"Pragma":                            "HTTP_PRAGMA",
+	"Proxy-Authorization":               "HTTP_PROXY_AUTHORIZATION",
+	"Range":                             "HTTP_RANGE",
+	"Referer":                           "HTTP_REFERER",
+	"Rtt":                               "HTTP_RTT",
+	"Save-Data":                         "HTTP_SAVE_DATA",
+	"Sec-Ch-Ua":                         "HTTP_SEC_CH_UA",
+	"Sec-Ch-Ua-Arch":                    "HTTP_SEC_CH_UA_ARCH",
+	"Sec-Ch-Ua-Bitness":                 "HTTP_SEC_CH_UA_BITNESS",
+	"Sec-Ch-Ua-Full-Version":            "HTTP_SEC_CH_UA_FULL_VERSION",
+	"Sec-Ch-Ua-Full-Version-List":       "HTTP_SEC_CH_UA_FULL_VERSION_LIST",
+	"Sec-Ch-Ua-Mobile":                  "HTTP_SEC_CH_UA_MOBILE",
+	"Sec-Ch-Ua-Model":                   "HTTP_SEC_CH_UA_MODEL",
+	"Sec-Ch-Ua-Platform":                "HTTP_SEC_CH_UA_PLATFORM",
+	"Sec-Ch-Ua-Platform-Version":        "HTTP_SEC_CH_UA_PLATFORM_VERSION",
+	"Sec-Fetch-Dest":                    "HTTP_SEC_FETCH_DEST",
+	"Sec-Fetch-Mode":                    "HTTP_SEC_FETCH_MODE",
+	"Sec-Fetch-Site":                    "HTTP_SEC_FETCH_SITE",
+	"Sec-Fetch-User":                    "HTTP_SEC_FETCH_USER",
+	"Sec-Gpc":                           "HTTP_SEC_GPC",
+	"Service-Worker-Navigation-Preload": "HTTP_SERVICE_WORKER_NAVIGATION_PRELOAD",
+	"Te":                                "HTTP_TE",
+	"Priority":                          "HTTP_PRIORITY",
+	"Trailer":                           "HTTP_TRAILER",
+	"Transfer-Encoding":                 "HTTP_TRANSFER_ENCODING",
+	"Upgrade":                           "HTTP_UPGRADE",
+	"Upgrade-Insecure-Requests":         "HTTP_UPGRADE_INSECURE_REQUESTS",
+	"User-Agent":                        "HTTP_USER_AGENT",
+	"Via":                               "HTTP_VIA",
+	"Viewport-Width":                    "HTTP_VIEWPORT_WIDTH",
+	"Want-Digest":                       "HTTP_WANT_DIGEST",
+	"Warning":                           "HTTP_WARNING",
+	"Width":                             "HTTP_WIDTH",
+	"X-Forwarded-For":                   "HTTP_X_FORWARDED_FOR",
+	"X-Forwarded-Host":                  "HTTP_X_FORWARDED_HOST",
+	"X-Forwarded-Proto":                 "HTTP_X_FORWARDED_PROTO",
+	"A-Im":                              "HTTP_A_IM",
+	"Accept-Datetime":                   "HTTP_ACCEPT_DATETIME",
+	"Content-Md5":                       "HTTP_CONTENT_MD5",
+	"Http2-Settings":                    "HTTP_HTTP2_SETTINGS",
+	"Prefer":                            "HTTP_PREFER",
+	"X-Requested-With":                  "HTTP_X_REQUESTED_WITH",
+	"Front-End-Https":                   "HTTP_FRONT_END_HTTPS",
+	"X-Http-Method-Override":            "HTTP_X_HTTP_METHOD_OVERRIDE",
+	"X-Att-Deviceid":                    "HTTP_X_ATT_DEVICEID",
+	"X-Wap-Profile":                     "HTTP_X_WAP_PROFILE",
+	"Proxy-Connection":                  "HTTP_PROXY_CONNECTION",
+	"X-Uidh":                            "HTTP_X_UIDH",
+	"X-Csrf-Token":                      "HTTP_X_CSRF_TOKEN",
+	"X-Request-Id":                      "HTTP_X_REQUEST_ID",
+	"X-Correlation-Id":                  "HTTP_X_CORRELATION_ID",
+	// Additional CDN/Framework headers
+	"Cloudflare-Visitor":        "HTTP_CLOUDFLARE_VISITOR",
+	"Cloudfront-Viewer-Address": "HTTP_CLOUDFRONT_VIEWER_ADDRESS",
+	"Cloudfront-Viewer-Country": "HTTP_CLOUDFRONT_VIEWER_COUNTRY",
+	"X-Amzn-Trace-Id":           "HTTP_X_AMZN_TRACE_ID",
+	"X-Cloud-Trace-Context":     "HTTP_X_CLOUD_TRACE_CONTEXT",
+	"Cf-Ray":                    "HTTP_CF_RAY",
+	"Cf-Visitor":                "HTTP_CF_VISITOR",
+	"Cf-Request-Id":             "HTTP_CF_REQUEST_ID",
+	"Cf-Ipcountry":              "HTTP_CF_IPCOUNTRY",
+	"X-Device-Type":             "HTTP_X_DEVICE_TYPE",
+	"X-Network-Info":            "HTTP_X_NETWORK_INFO",
+	"X-Client-Id":               "HTTP_X_CLIENT_ID",
+	"X-Livewire":                "HTTP_X_LIVEWIRE",
+}
+
+// Cache up to 256 uncommon headers
+// This is ~2.5x faster than converting the header each time
+var headerKeyCache = func() otter.Cache[string, string] {
+	c, err := otter.MustBuilder[string, string](256).Build()
+	if err != nil {
+		panic(err)
+	}
+
+	return c
+}()
+
+var headerNameReplacer = strings.NewReplacer(" ", "_", "-", "_")
+
+func GetUnCommonHeader(key string) string {
+	phpHeaderKey, ok := headerKeyCache.Get(key)
+	if !ok {
+		phpHeaderKey = "HTTP_" + headerNameReplacer.Replace(strings.ToUpper(key)) + "\x00"
+		headerKeyCache.SetIfAbsent(key, phpHeaderKey)
+	}
+
+	return phpHeaderKey
+}