Skip to content

fix: make session clearing resilient #1010

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

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

fix: make session clearing resilient #1010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
81d8fbe
fix: make session clear resilient
nerdoza Apr 2, 2025
8dcb0f3
refactor: consolidate TTL logic
nerdoza Apr 2, 2025
1abf637
docs: add notes about session rotation
nerdoza Apr 2, 2025
382ffe4
style: resolve lint issues
nerdoza Apr 2, 2025
9b53343
fix: use initialize for session sealing
nerdoza Apr 2, 2025
7361537
revert: restore getSession calls in utility functions
nerdoza Apr 2, 2025
16bb190
docs: correct typo in session docs
nerdoza Apr 2, 2025
370ce91
refactor: change to clobber session object
nerdoza Apr 2, 2025
f5644e7
docs: remove notes about session rotation
nerdoza Apr 15, 2025
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
4 changes: 3 additions & 1 deletion docs/4.examples/handle-session.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ This will initialize a session and return an header `Set-Cookie` with a cookie n
If the request contains a cookie named `h3` or a header named `x-h3-session`, the session will be initialized with the content of the cookie or the header.

> [!NOTE]
> The header take precedence over the cookie.
> The header takes precedence over the cookie.

## Get Data from a Session

Expand Down Expand Up @@ -86,6 +86,8 @@ We try to get a session from the request. If there is no session, a new one will

Try to visit the page multiple times and you will see the number of times you visited the page.

If a `maxAge` is configured, the expiration date of the session will not be updated with a call to `update` on the session object, nor with a call using the `updateSession` utility.

> [!NOTE]
> If you use a CLI tool like `curl` to test this example, you will not see the number of times you visited the page because the CLI tool does not save cookies. You must get the cookie from the response and send it back to the server.

Expand Down
59 changes: 44 additions & 15 deletions src/utils/session.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,27 @@ import {
} from "./internal/session";
import { EmptyObject } from "./internal/obj";

/**
* Get the session name from the config.
*/
function getSessionName(config: SessionConfig) {
return config.name || DEFAULT_SESSION_NAME;
}

/**
* Generate the session id from the config.
*/
function generateId(config: SessionConfig) {
return config.generateId?.() ?? (config.crypto || crypto).randomUUID();
}

/**
* Get the max age TTL in ms from the config.
*/
function getMaxAgeTTL(config: SessionConfig) {
return config.maxAge ? config.maxAge * 1000 : 0;
}

/**
* Create a session manager for the current request.
*
Expand All @@ -17,7 +38,7 @@ export async function useSession<T extends SessionData = SessionData>(
config: SessionConfig,
) {
// Create a synced wrapper around the session
const sessionName = config.name || DEFAULT_SESSION_NAME;
const sessionName = getSessionName(config);
await getSession(event, config); // Force init
const sessionManager = {
get id() {
Expand Down Expand Up @@ -45,7 +66,7 @@ export async function getSession<T extends SessionData = SessionData>(
event: H3Event,
config: SessionConfig,
): Promise<Session<T>> {
const sessionName = config.name || DEFAULT_SESSION_NAME;
const sessionName = getSessionName(config);

// Return existing session if available
if (!event.context.sessions) {
Expand Down Expand Up @@ -97,8 +118,7 @@ export async function getSession<T extends SessionData = SessionData>(

// New session store in response cookies
if (!session.id) {
session.id =
config.generateId?.() ?? (config.crypto || crypto).randomUUID();
session.id = generateId(config);
session.createdAt = Date.now();
await updateSession(event, config);
}
Expand All @@ -118,12 +138,12 @@ export async function updateSession<T extends SessionData = SessionData>(
config: SessionConfig,
update?: SessionUpdate<T>,
): Promise<Session<T>> {
const sessionName = config.name || DEFAULT_SESSION_NAME;
const sessionName = getSessionName(config);

// Access current session
const session: Session<T> =
(event.context.sessions?.[sessionName] as Session<T>) ||
(await getSession<T>(event, config));
(await getSession(event, config));

// Update session data if provided
if (typeof update === "function") {
Expand All @@ -139,7 +159,7 @@ export async function updateSession<T extends SessionData = SessionData>(
setCookie(event, sessionName, sealed, {
...DEFAULT_SESSION_COOKIE,
expires: config.maxAge
? new Date(session.createdAt + config.maxAge * 1000)
? new Date(session.createdAt + getMaxAgeTTL(config))
: undefined,
...config.cookie,
});
Expand All @@ -155,7 +175,7 @@ export async function sealSession<T extends SessionData = SessionData>(
event: H3Event,
config: SessionConfig,
) {
const sessionName = config.name || DEFAULT_SESSION_NAME;
const sessionName = getSessionName(config);

// Access current session
const session: Session<T> =
Expand All @@ -164,7 +184,7 @@ export async function sealSession<T extends SessionData = SessionData>(

const sealed = await seal(session, config.password, {
...sealDefaults,
ttl: config.maxAge ? config.maxAge * 1000 : 0,
ttl: getMaxAgeTTL(config),
...config.seal,
});

Expand All @@ -181,12 +201,12 @@ export async function unsealSession(
) {
const unsealed = (await unseal(sealed, config.password, {
...sealDefaults,
ttl: config.maxAge ? config.maxAge * 1000 : 0,
ttl: getMaxAgeTTL(config),
...config.seal,
})) as Partial<Session>;
if (config.maxAge) {
const age = Date.now() - (unsealed.createdAt || Number.NEGATIVE_INFINITY);
if (age > config.maxAge * 1000) {
if (age > getMaxAgeTTL(config)) {
throw new Error("Session expired!");
}
}
Expand All @@ -198,12 +218,21 @@ export async function unsealSession(
*/
export function clearSession(
event: H3Event,
config: Partial<SessionConfig>,
config: SessionConfig,
): Promise<void> {
const sessionName = config.name || DEFAULT_SESSION_NAME;
if (event.context.sessions?.[sessionName]) {
delete event.context.sessions![sessionName];
const sessionName = getSessionName(config);
if (!event.context.sessions) {
event.context.sessions = new EmptyObject();
}

// Clobber the original session with a new session
event.context.sessions![sessionName] = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be reverted to allow this PR move forward as a refactor step.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused @pi0. This line is the main point of this PR. If you want to isolate changes I'd be more apt to revert the refactor changes on this PR and apply them on a separate PR intended for refactor changes. Thoughts?

Copy link
Member

@pi0 pi0 Apr 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do this as part of #1022 for session rotate. clear purpose is to destroy session fully not to create a new one.

Alternatively we can set a _destroyed flag to previous object to indicate it is invalid.

Copy link
Author

@nerdoza nerdoza Apr 23, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pi0, I'm in complete agreement on that point, but the implementation at the code level is not 1:1 with the user experience. When the user calls clear() they want the session cleared, and then if they call update() they would expect that this creates a new session (or at least that's what I would assume). Because the update() call is eagerly restoring the session from the request, calling update() returns a mutation of the same session that was previously cleared.

TLDR; If a new session is not set on the context.sessions[sessionName] when clear() is called, then the session isn't actually cleared, it's just temporarily reset (and will be restored on next access call).

id: generateId(config),
createdAt: Date.now(),
data: new EmptyObject(),
} satisfies Session;

// Set a cleared session cookie
setCookie(event, sessionName, "", {
...DEFAULT_SESSION_COOKIE,
...config.cookie,
Expand Down