add php_server's env vars to sandboxed environment

keep track of separate prepared_env set for a php_server
correctly exposes env vars into $_ENV when E is in `variables_order`
also fixes the problem of lazy server eval I didn't think about last commit

Author: henderkes

cgi.go

@@ -4,10 +4,12 @@ package frankenphp
 // #cgo nocallback frankenphp_register_variable_safe
 // #cgo nocallback frankenphp_register_known_variable
 // #cgo nocallback frankenphp_init_persistent_string
+// #cgo nocallback frankenphp_add_to_sandboxed_env
 // #cgo noescape frankenphp_register_server_vars
 // #cgo noescape frankenphp_register_variable_safe
 // #cgo noescape frankenphp_register_known_variable
 // #cgo noescape frankenphp_init_persistent_string
+// #cgo noescape frankenphp_add_to_sandboxed_env
 // #include "frankenphp.h"
 // #include <php_variables.h>
 import "C"
@@ -180,6 +182,13 @@ func addPreparedEnvToServer(fc *frankenPHPContext, trackVarsArray *C.zval) {
 	fc.env = nil
 }
 
+// addPreparedEnvToSandbox exposes fc.env to getenv() before any PHP code runs.
+func addPreparedEnvToSandbox(fc *frankenPHPContext) {
+	for k, v := range fc.env {
+		C.frankenphp_add_to_sandboxed_env(toUnsafeChar(k), C.size_t(len(k)-1), toUnsafeChar(v), C.size_t(len(v)))
+	}
+}
+
 //export go_register_server_variables
 func go_register_server_variables(threadIndex C.uintptr_t, trackVarsArray *C.zval) {
 	thread := phpThreads[threadIndex]
@@ -298,6 +307,8 @@ func go_update_request_info(threadIndex C.uintptr_t, info *C.sapi_request_info)
 		return nil
 	}
 
+	addPreparedEnvToSandbox(fc)
+
 	if m, ok := cStringHTTPMethods[request.Method]; ok {
 		info.request_method = m
 	} else {

frankenphp.c

@@ -92,6 +92,10 @@ HashTable *main_thread_env = NULL;
 __thread uintptr_t thread_index;
 __thread bool is_worker_thread = false;
 __thread HashTable *sandboxed_env = NULL;
+/* prepared_env holds entries from php(_server)'s `env KEY VAL`,
+ * so they can be merged into $_ENV when 'E' is in
+ * variables_order. Separate from putenv() so we don't leak into $_ENV */
+__thread HashTable *prepared_env = NULL;
 
 /* Published via SG(server_context) so ext-parallel children, which inherit
  * the parent's SG(server_context), can route SAPI callbacks back to the
@@ -429,9 +433,16 @@ bool frankenphp_shutdown_dummy_request(void) {
 }
 
 void get_full_env(zval *track_vars_array) {
-  zend_hash_extend(Z_ARR_P(track_vars_array),
-                   zend_hash_num_elements(main_thread_env), 0);
+  size_t total = zend_hash_num_elements(main_thread_env);
+  if (prepared_env != NULL) {
+    // perf: doesn't matter if we get the exact count, just >= needed
+    total += zend_hash_num_elements(prepared_env);
+  }
+  zend_hash_extend(Z_ARR_P(track_vars_array), total, 0);
   zend_hash_copy(Z_ARR_P(track_vars_array), main_thread_env, NULL);
+  if (prepared_env != NULL) {
+    zend_hash_copy(Z_ARR_P(track_vars_array), prepared_env, NULL);
+  }
 }
 
 /* Adapted from php_request_startup() */
@@ -1239,6 +1250,31 @@ static inline void reset_sandboxed_environment() {
     zend_hash_release(sandboxed_env);
     sandboxed_env = NULL;
   }
+  if (prepared_env != NULL) {
+    zend_hash_release(prepared_env);
+    prepared_env = NULL;
+  }
+}
+
+/* Adds a key/value pair to the per-thread sandboxed environment so it becomes
+ * visible to getenv()/$_ENV. Used to expose env vars declared in the php(_server)
+ * directive, which would otherwise only appear in $_SERVER. */
+void frankenphp_add_to_sandboxed_env(char *name, size_t name_len, char *val,
+                                     size_t val_len) {
+  if (sandboxed_env == NULL) {
+    sandboxed_env = zend_array_dup(main_thread_env);
+  }
+  zval zv = {0};
+  ZVAL_STRINGL(&zv, val, val_len);
+  zend_hash_str_update(sandboxed_env, name, name_len, &zv);
+
+  if (prepared_env == NULL) {
+    ALLOC_HASHTABLE(prepared_env);
+    zend_hash_init(prepared_env, 8, NULL, ZVAL_PTR_DTOR, 0);
+  }
+  zval zv2 = {0};
+  ZVAL_STRINGL(&zv2, val, val_len);
+  zend_hash_str_update(prepared_env, name, name_len, &zv2);
 }
 
 static void *php_thread(void *arg) {

frankenphp.h

@@ -202,6 +202,8 @@ void frankenphp_register_variable_safe(char *key, char *var, size_t val_len,
                                        zval *track_vars_array);
 void frankenphp_register_server_vars(zval *track_vars_array,
                                      frankenphp_server_vars vars);
+void frankenphp_add_to_sandboxed_env(char *name, size_t name_len, char *val,
+                                     size_t val_len);
 
 zend_string *frankenphp_init_persistent_string(const char *string, size_t len);
 int frankenphp_reset_opcache(void);

frankenphp_test.go

@@ -767,6 +767,25 @@ func TestEnvIsNotResetInWorkerMode(t *testing.T) {
 	}, &testOptions{workerScript: "env/remember-env.php"})
 }
 
+// reproduction of https://github.com/php/frankenphp/issues/1674
+func TestPreparedEnvIsVisibleToGetenv_module(t *testing.T) {
+	testPreparedEnvIsVisibleToGetenv(t, &testOptions{nbParallelRequests: 1})
+}
+func TestPreparedEnvIsVisibleToGetenv_worker(t *testing.T) {
+	testPreparedEnvIsVisibleToGetenv(t, &testOptions{
+		workerScript: "env/prepared-env-getenv.php",
+	})
+}
+func testPreparedEnvIsVisibleToGetenv(t *testing.T, opts *testOptions) {
+	opts.requestOpts = append(opts.requestOpts,
+		frankenphp.WithRequestEnv(map[string]string{"FRANKENPHP_TEST_PHP_SERVER_ENV_IN_GETENV": "hello"}),
+	)
+	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, _ int) {
+		body, _ := testGet("http://example.com/env/prepared-env-getenv.php", handler, t)
+		assert.Equal(t, "getenv='hello'\nserver='hello'\n", body)
+	}, opts)
+}
+
 // reproduction of https://github.com/php/frankenphp/issues/1061
 func TestModificationsToEnvPersistAcrossRequests(t *testing.T) {
 	runTest(t, func(handler func(http.ResponseWriter, *http.Request), _ *httptest.Server, i int) {

testdata/env/prepared-env-getenv.php

@@ -0,0 +1,11 @@
+<?php
+
+require_once __DIR__ . '/../_executor.php';
+
+return function () {
+    // Variables declared via the env subdirective in the php(_server) directive
+    // (or via WithRequestEnv) must be exposed to both $_SERVER and getenv().
+    // See https://github.com/php/frankenphp/issues/1674
+    echo "getenv=" . var_export(getenv('FRANKENPHP_TEST_PHP_SERVER_ENV_IN_GETENV'), true) . "\n";
+    echo "server=" . var_export($_SERVER['FRANKENPHP_TEST_PHP_SERVER_ENV_IN_GETENV'] ?? null, true) . "\n";
+};