Skip to content

fix: session handlers #2192

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
AlliBalliBaba wants to merge 2 commits into
base: main
Choose a base branch
from
+41 −204
Open

fix: session handlers #2192

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73f7004
Fixes session handlers.
AlliBalliBaba Feb 15, 2026
c5d4285
Removes sessions from symbol table.
AlliBalliBaba Feb 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 1 addition & 119 deletions frankenphp.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
#include <Zend/zend_interfaces.h>
#include <Zend/zend_types.h>
#include <errno.h>
#include <ext/session/php_session.h>
#include <ext/spl/spl_exceptions.h>
#include <ext/standard/head.h>
#include <inttypes.h>
Expand Down Expand Up @@ -76,41 +75,6 @@ __thread bool is_worker_thread = false;
__thread zval *os_environment = NULL;
__thread HashTable *worker_ini_snapshot = NULL;

/* Session user handler names (same structure as PS(mod_user_names)).
* In PHP 8.2, mod_user_names is a union with .name.ps_* access.
* In PHP 8.3+, mod_user_names is a direct struct with .ps_* access. */
typedef struct {
zval ps_open;
zval ps_close;
zval ps_read;
zval ps_write;
zval ps_destroy;
zval ps_gc;
zval ps_create_sid;
zval ps_validate_sid;
zval ps_update_timestamp;
} session_user_handlers;

/* Macro to access PS(mod_user_names) handlers across PHP versions */
#if PHP_VERSION_ID >= 80300
#define PS_MOD_USER_NAMES(handler) PS(mod_user_names).handler
#else
#define PS_MOD_USER_NAMES(handler) PS(mod_user_names).name.handler
#endif

#define FOR_EACH_SESSION_HANDLER(op) \
op(ps_open); \
op(ps_close); \
op(ps_read); \
op(ps_write); \
op(ps_destroy); \
op(ps_gc); \
op(ps_create_sid); \
op(ps_validate_sid); \
op(ps_update_timestamp)

__thread session_user_handlers *worker_session_handlers_snapshot = NULL;

void frankenphp_update_local_thread_context(bool is_worker) {
is_worker_thread = is_worker;
}
Expand Down Expand Up @@ -156,11 +120,7 @@ static void frankenphp_reset_super_globals() {
zval_ptr_dtor_nogc(files);
memset(files, 0, sizeof(*files));

/* $_SESSION must be explicitly deleted from the symbol table.
* Unlike other superglobals, $_SESSION is stored in EG(symbol_table)
* with a reference to PS(http_session_vars). The session RSHUTDOWN
* only decrements the refcount but doesn't remove it from the symbol
* table, causing data to leak between requests. */
/* $_SESSION must be explicitly deleted from the symbol table. */
zend_hash_str_del(&EG(symbol_table), "_SESSION", sizeof("_SESSION") - 1);
}
zend_end_try();
Expand Down Expand Up @@ -300,59 +260,6 @@ static void frankenphp_restore_ini(void) {
}
}

/* Save session user handlers set before the worker loop.
* This allows frameworks to define custom session handlers that persist. */
static void frankenphp_snapshot_session_handlers(void) {
if (worker_session_handlers_snapshot != NULL) {
return; /* Already snapshotted */
}

/* Check if session module is loaded */
if (zend_hash_str_find_ptr(&module_registry, "session",
sizeof("session") - 1) == NULL) {
return; /* Session module not available */
}

/* Check if user session handlers are defined */
if (Z_ISUNDEF(PS_MOD_USER_NAMES(ps_open))) {
return; /* No user handlers to snapshot */
}

worker_session_handlers_snapshot = emalloc(sizeof(session_user_handlers));

/* Copy each handler zval with incremented reference count */
#define SNAPSHOT_HANDLER(h) \
if (!Z_ISUNDEF(PS_MOD_USER_NAMES(h))) { \
ZVAL_COPY(&worker_session_handlers_snapshot->h, &PS_MOD_USER_NAMES(h)); \
} else { \
ZVAL_UNDEF(&worker_session_handlers_snapshot->h); \
}

FOR_EACH_SESSION_HANDLER(SNAPSHOT_HANDLER);

#undef SNAPSHOT_HANDLER
}

/* Restore session user handlers from snapshot after RSHUTDOWN freed them. */
static void frankenphp_restore_session_handlers(void) {
if (worker_session_handlers_snapshot == NULL) {
return;
}

/* Restore each handler zval.
* Session RSHUTDOWN already freed the handlers via zval_ptr_dtor and set
* them to UNDEF, so we don't need to destroy them again. We simply copy
* from the snapshot (which holds its own reference). */
#define RESTORE_HANDLER(h) \
if (!Z_ISUNDEF(worker_session_handlers_snapshot->h)) { \
ZVAL_COPY(&PS_MOD_USER_NAMES(h), &worker_session_handlers_snapshot->h); \
}

FOR_EACH_SESSION_HANDLER(RESTORE_HANDLER);

#undef RESTORE_HANDLER
}

/* Free worker state when the worker script terminates. */
static void frankenphp_cleanup_worker_state(void) {
/* Free INI snapshot */
Expand All @@ -361,21 +268,6 @@ static void frankenphp_cleanup_worker_state(void) {
FREE_HASHTABLE(worker_ini_snapshot);
worker_ini_snapshot = NULL;
}

/* Free session handlers snapshot */
if (worker_session_handlers_snapshot != NULL) {
#define FREE_HANDLER(h) \
if (!Z_ISUNDEF(worker_session_handlers_snapshot->h)) { \
zval_ptr_dtor(&worker_session_handlers_snapshot->h); \
}

FOR_EACH_SESSION_HANDLER(FREE_HANDLER);

#undef FREE_HANDLER

efree(worker_session_handlers_snapshot);
worker_session_handlers_snapshot = NULL;
}
}

/* Adapted from php_request_shutdown */
Expand Down Expand Up @@ -412,11 +304,7 @@ bool frankenphp_shutdown_dummy_request(void) {
return false;
}

/* Snapshot INI and session handlers BEFORE shutdown.
* The framework has set these up before the worker loop, and we want
* to preserve them. Session RSHUTDOWN will free the handlers. */
frankenphp_snapshot_ini();
frankenphp_snapshot_session_handlers();

frankenphp_worker_request_shutdown();

Expand Down Expand Up @@ -488,12 +376,6 @@ static int frankenphp_worker_request_startup() {
module->request_startup_func(module->type, module->module_number);
}
}

/* Restore session handlers AFTER session RINIT.
* Session RSHUTDOWN frees mod_user_names callbacks, so we must restore
* them before user code runs. This must happen after RINIT because
* session RINIT may reset some state. */
frankenphp_restore_session_handlers();
}
zend_catch { retval = FAILURE; }
zend_end_try();
Expand Down
66 changes: 25 additions & 41 deletions frankenphp_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1207,47 +1207,31 @@ func TestIniLeakBetweenRequests_worker(t *testing.T) {
})
}

func TestSessionHandlerPreLoopPreserved_worker(t *testing.T) {
runTest(t, func(_ func(http.ResponseWriter, *http.Request), ts *httptest.Server, i int) {
// Request 1: Check that the pre-loop session handler is preserved
resp1, err := http.Get(ts.URL + "/worker-with-session-handler.php?action=check")
assert.NoError(t, err)
body1, _ := io.ReadAll(resp1.Body)
_ = resp1.Body.Close()

body1Str := string(body1)
t.Logf("Request 1 response: %s", body1Str)
assert.Contains(t, body1Str, "HANDLER_PRESERVED",
"Session handler set before loop should be preserved")
assert.Contains(t, body1Str, "save_handler=user",
"session.save_handler should remain 'user'")

// Request 2: Use the session - should work with pre-loop handler
resp2, err := http.Get(ts.URL + "/worker-with-session-handler.php?action=use_session")
assert.NoError(t, err)
body2, _ := io.ReadAll(resp2.Body)
_ = resp2.Body.Close()

body2Str := string(body2)
t.Logf("Request 2 response: %s", body2Str)
assert.Contains(t, body2Str, "SESSION_OK",
"Session should work with pre-loop handler.\nResponse: %s", body2Str)
assert.NotContains(t, body2Str, "ERROR:",
"No errors expected.\nResponse: %s", body2Str)
assert.NotContains(t, body2Str, "EXCEPTION:",
"No exceptions expected.\nResponse: %s", body2Str)

// Request 3: Check handler is still preserved after using session
resp3, err := http.Get(ts.URL + "/worker-with-session-handler.php?action=check")
assert.NoError(t, err)
body3, _ := io.ReadAll(resp3.Body)
_ = resp3.Body.Close()

body3Str := string(body3)
t.Logf("Request 3 response: %s", body3Str)
assert.Contains(t, body3Str, "HANDLER_PRESERVED",
"Session handler should still be preserved after use")

func TestSessionHandler_worker(t *testing.T) {
runTest(t, func(handler func(http.ResponseWriter, *http.Request), ts *httptest.Server, i int) {
url := ts.URL + "/worker-with-session-handler.php"
body, _ := testGet(url+"?action=check", handler, t)
assert.Contains(t, body, "no session value", "request without session cookie, should not see session value")

body, responseWithSessionCookie := testGet(url+"?action=read_session", handler, t)
assert.Contains(t, body, "no session value")
assert.Len(t, responseWithSessionCookie.Header["Set-Cookie"], 1, "Expected exactly one Set-Cookie header")

requestWithSessionCookie := httptest.NewRequest("GET", url+"?action=put_session", nil)
requestWithSessionCookie.Header["Cookie"] = []string{responseWithSessionCookie.Header["Set-Cookie"][0]}
body, _ = testRequest(requestWithSessionCookie, handler, t)
assert.Contains(t, body, "session value set", "make request with session cookie, should see session value set in previous request")

body, _ = testGet( url+"?action=read_session", handler, t)
assert.Contains(t, body, "no session value", "make request without session cookie, should not see previous session value")

body, _ = testGet( url+"?action=check", handler, t)
assert.Contains(t, body, "no session value", "make request without starting a session")

requestWithSessionCookie = httptest.NewRequest("GET", url+"?action=read_session", nil)
requestWithSessionCookie.Header["Cookie"] = []string{responseWithSessionCookie.Header["Set-Cookie"][0]}
body, _ = testRequest( requestWithSessionCookie, handler, t)
assert.Contains(t, body, "session value exists", "make final request with session cookie, should see previous value")
}, &testOptions{
workerScript: "worker-with-session-handler.php",
nbWorkers: 1,
Expand Down
59 changes: 15 additions & 44 deletions testdata/worker-with-session-handler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ public function read(string $id): string|false

public function write(string $id, string $data): bool
{
echo "WRITING SESSION: id=$id, data=$data\n";
self::$data[$id] = $data;
return true;
}
Expand All @@ -40,59 +41,29 @@ public function gc(int $max_lifetime): int|false

// Set the session handler BEFORE the worker loop
$handler = new PreLoopSessionHandler();
session_set_save_handler($handler, true);

$requestCount = 0;

do {
$ok = frankenphp_handle_request(function () use (&$requestCount): void {
$requestCount++;
$output = [];
$output[] = "request=$requestCount";

$ok = frankenphp_handle_request(function () use ($handler): void {
$action = $_GET['action'] ?? 'check';

switch ($action) {
case 'use_session':
// Try to use the session - should work with pre-loop handler
$error = null;
set_error_handler(function ($errno, $errstr) use (&$error) {
$error = $errstr;
return true;
});

try {
session_id('test-preloop-' . $requestCount);
$result = session_start();
if ($result) {
$_SESSION['test'] = 'value-' . $requestCount;
session_write_close();
$output[] = "SESSION_OK";
} else {
$output[] = "SESSION_START_FAILED";
}
} catch (Throwable $e) {
$output[] = "EXCEPTION:" . $e->getMessage();
}

restore_error_handler();

if ($error) {
$output[] = "ERROR:" . $error;
}
case 'put_session':
session_set_save_handler($handler, true);
session_start();
$_SESSION['test'] = 'session value exists';
echo 'session value set';
break;
case 'read_session':
session_set_save_handler($handler, true);
session_start();
echo 'session id:' . session_id();
echo $_SESSION['test'] ?? 'no session value';
break;

case 'check':
default:
$saveHandler = ini_get('session.save_handler');
$output[] = "save_handler=$saveHandler";
if ($saveHandler === 'user') {
$output[] = "HANDLER_PRESERVED";
} else {
$output[] = "HANDLER_LOST";
}
echo $_SESSION['test'] ?? 'no session value';
break;
}

echo implode("\n", $output);
});
} while ($ok);
Loading