refactor: extract the state module and make the backoff error instead of panic

This PR:
- moves state.go to its own module
- moves the phpheaders test the phpheaders module
- simplifies backoff.go
- makes the backoff error instead of panic (so it can be tested)
- removes some unused C structs

Author: AlliBalliBaba

backoff.go

@@ -277,13 +277,13 @@ 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) C.bool {
+func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info) {
 	thread := phpThreads[threadIndex]
 	fc := thread.frankenPHPContext()
 	request := fc.request
 
 	if request == nil {
-		return C.bool(fc.worker != nil)
+		return
 	}
 
 	authUser, authPassword, ok := request.BasicAuth()
@@ -311,8 +311,6 @@ 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)
-
-	return C.bool(fc.worker != nil)
 }
 
 // SanitizedPathJoin performs filepath.Join(root, reqPath) that

backoff_test.go

@@ -1,5 +1,9 @@
 package frankenphp
 
+import (
+	"github.com/dunglas/frankenphp/internal/state"
+)
+
 // EXPERIMENTAL: ThreadDebugState prints the state of a single PHP thread - debugging purposes only
 type ThreadDebugState struct {
 	Index                    int
@@ -23,7 +27,7 @@ func DebugState() FrankenPHPDebugState {
 		ReservedThreadCount: 0,
 	}
 	for _, thread := range phpThreads {
-		if thread.state.is(stateReserved) {
+		if thread.state.Is(state.Reserved) {
 			fullState.ReservedThreadCount++
 			continue
 		}
@@ -38,9 +42,9 @@ func threadDebugState(thread *phpThread) ThreadDebugState {
 	return ThreadDebugState{
 		Index:                    thread.threadIndex,
 		Name:                     thread.name(),
-		State:                    thread.state.name(),
-		IsWaiting:                thread.state.isInWaitingState(),
-		IsBusy:                   !thread.state.isInWaitingState(),
-		WaitingSinceMilliseconds: thread.state.waitTime(),
+		State:                    thread.state.Name(),
+		IsWaiting:                thread.state.IsInWaitingState(),
+		IsBusy:                   !thread.state.IsInWaitingState(),
+		WaitingSinceMilliseconds: thread.state.WaitTime(),
 	}
 }

cgi.go

@@ -1,10 +1,9 @@
 package frankenphp
 
 // #cgo nocallback frankenphp_init_persistent_string
-// #cgo nocallback frankenphp_add_assoc_str_ex
 // #cgo noescape frankenphp_init_persistent_string
-// #cgo noescape frankenphp_add_assoc_str_ex
 // #include "frankenphp.h"
+// #include <Zend/zend_API.h>
 import "C"
 import (
 	"os"
@@ -98,7 +97,7 @@ func go_getfullenv(threadIndex C.uintptr_t, trackVarsArray *C.zval) {
 	env := getSandboxedEnv(thread)
 
 	for key, val := range env {
-		C.frankenphp_add_assoc_str_ex(trackVarsArray, toUnsafeChar(key), C.size_t(len(key)), val)
+		C.add_assoc_str_ex(trackVarsArray, toUnsafeChar(key), C.size_t(len(key)), val)
 	}
 }
 

debugstate.go

@@ -51,7 +51,6 @@ frankenphp_version frankenphp_get_version() {
 
 frankenphp_config frankenphp_get_config() {
   return (frankenphp_config){
-      frankenphp_get_version(),
 #ifdef ZTS
       true,
 #else
@@ -75,14 +74,18 @@ __thread uintptr_t thread_index;
 __thread bool is_worker_thread = false;
 __thread zval *os_environment = NULL;
 
+void frankenphp_update_local_thread_context(bool is_worker) {
+  is_worker_thread = is_worker;
+}
+
 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;
   /* status It is not reset by zend engine, set it to 200. */
   SG(sapi_headers).http_response_code = 200;
 
-  is_worker_thread = go_update_request_info(thread_index, &SG(request_info));
+  go_update_request_info(thread_index, &SG(request_info));
 }
 
 static void frankenphp_free_request_context() {
@@ -206,11 +209,6 @@ PHPAPI void get_full_env(zval *track_vars_array) {
   go_getfullenv(thread_index, track_vars_array);
 }
 
-void frankenphp_add_assoc_str_ex(zval *track_vars_array, char *key,
-                                 size_t keylen, zend_string *val) {
-  add_assoc_str_ex(track_vars_array, key, keylen, val);
-}
-
 /* Adapted from php_request_startup() */
 static int frankenphp_worker_request_startup() {
   int retval = SUCCESS;
@@ -652,8 +650,9 @@ static char *frankenphp_read_cookies(void) {
 }
 
 /* all variables with well defined keys can safely be registered like this */
-void frankenphp_register_trusted_var(zend_string *z_key, char *value,
-                                     size_t val_len, HashTable *ht) {
+static inline void frankenphp_register_trusted_var(zend_string *z_key,
+                                                   char *value, size_t val_len,
+                                                   HashTable *ht) {
   if (value == NULL) {
     zval empty;
     ZVAL_EMPTY_STRING(&empty);

env.go

@@ -23,12 +23,6 @@ typedef struct ht_key_value_pair {
   size_t val_len;
 } ht_key_value_pair;
 
-typedef struct php_variable {
-  const char *var;
-  size_t data_len;
-  char *data;
-} php_variable;
-
 typedef struct frankenphp_version {
   unsigned char major_version;
   unsigned char minor_version;
@@ -40,7 +34,6 @@ typedef struct frankenphp_version {
 frankenphp_version frankenphp_get_version();
 
 typedef struct frankenphp_config {
-  frankenphp_version version;
   bool zts;
   bool zend_signals;
   bool zend_max_execution_timers;
@@ -52,6 +45,7 @@ bool frankenphp_new_php_thread(uintptr_t thread_index);
 
 bool frankenphp_shutdown_dummy_request(void);
 int frankenphp_execute_script(char *file_name);
+void frankenphp_update_local_thread_context(bool is_worker);
 
 int frankenphp_execute_script_cli(char *script, int argc, char **argv,
                                   bool eval);
@@ -65,8 +59,6 @@ void frankenphp_register_variable_safe(char *key, char *var, size_t val_len,
 zend_string *frankenphp_init_persistent_string(const char *string, size_t len);
 int frankenphp_reset_opcache(void);
 int frankenphp_get_current_memory_limit();
-void frankenphp_add_assoc_str_ex(zval *track_vars_array, char *key,
-                                 size_t keylen, zend_string *val);
 
 void frankenphp_register_single(zend_string *z_key, char *value, size_t val_len,
                                 zval *track_vars_array);

frankenphp.c

@@ -618,10 +618,12 @@ func testRequestHeaders(t *testing.T, opts *testOptions) {
 }
 
 func TestFailingWorker(t *testing.T) {
-	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) {
-		body, _ := testGet("http://example.com/failing-worker.php", handler, t)
-		assert.Contains(t, body, "ok")
-	}, &testOptions{workerScript: "failing-worker.php"})
+	err := frankenphp.Init(
+		frankenphp.WithLogger(slog.New(slog.NewTextHandler(io.Discard, nil))),
+		frankenphp.WithWorkers("failing worker", "testdata/failing-worker.php", 4, frankenphp.WithWorkerMaxFailures(1)),
+		frankenphp.WithNumThreads(5),
+	)
+	assert.Error(t, err, "should return an immediate error if workers fail on startup")
 }
 
 func TestEnv(t *testing.T) {

frankenphp.h

@@ -1,5 +1,6 @@
 package phpheaders
 
+import "C"
 import (
 	"context"
 	"strings"

frankenphp_test.go

@@ -0,0 +1,22 @@
+package phpheaders
+
+import (
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestAllCommonHeadersAreCorrect(t *testing.T) {
+	fakeRequest := httptest.NewRequest("GET", "http://localhost", nil)
+
+	for header, phpHeader := range CommonRequestHeaders {
+		// verify that common and uncommon headers return the same result
+		expectedPHPHeader := GetUnCommonHeader(t.Context(), header)
+		assert.Equal(t, phpHeader+"\x00", expectedPHPHeader, "header is not well formed: "+phpHeader)
+
+		// net/http will capitalize lowercase headers, verify that headers are capitalized
+		fakeRequest.Header.Add(header, "foo")
+		assert.Contains(t, fakeRequest.Header, header, "header is not correctly capitalized: "+header)
+	}
+}