mirror of
https://github.com/toeverything/AFFiNE.git
synced 2026-02-04 00:28:33 +00:00
fix: safe cookie parsing (#14292)
# Summary
This PR fixes a server-side cookie parsing edge case where malformed
cookie values throw `URI malformed`, causing socket.io auth to fail and
clients to get stuck in infinite workspace loading/syncing.
# Observed Behavior
- User creates a cloud-backed workspace and invites another user to it.
- Second user accepts the invite, awaits approval, and attempts to load
the workspace, getting stuck in infinite loading state.
- `api/workspaces/<id>/docs/<id>` return 404 for those users, as the
workspace they are trying to access was not synced to the server.
- Server logs show socket.io `CONNECT_ERROR` with `URI malformed`, then
connection closed.
# Confirmed Trigger
An externally-managed `auth_session` cookie containing a raw `%` symbol
causes `decodeURIComponent` to throw. This matches the observed
socket.io `CONNECT_ERROR`, explaining why some users were affected while
the rest were not.
# Root Cause
The `parseCookies` function calls `decodeURIComponent` on every cookie
key/value without guard, so when a malformed percent-encoded value is
encountered, `decodeURIComponent` throws, which bubbles into the
socket.io auth middleware, aborting the connection.
# Fix
Wrap `decodeURIComponent` calls in `try/catch`, on failure falling back
to the raw key/value.
# Testing
- Manually regenerating the bad cookie until no malformed parts are
present resolves the issue.
- With the guard in place, affected users can open shared workspaces
with sync successfully completing.
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved cookie parsing robustness so malformed cookie values no
longer cause errors; the system now preserves raw cookie values when
decoding fails.
* **Tests**
* Added test coverage to ensure cookie parsing handles invalid/malformed
cookie values without throwing.
<sub>✏️ Tip: You can customize this high-level summary in your review
settings.</sub>
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
@@ -1,10 +1,12 @@
|
||||
import { randomUUID } from 'node:crypto';
|
||||
import { IncomingMessage } from 'node:http';
|
||||
|
||||
import { HttpStatus } from '@nestjs/common';
|
||||
import { PrismaClient } from '@prisma/client';
|
||||
import ava, { TestFn } from 'ava';
|
||||
import Sinon from 'sinon';
|
||||
|
||||
import { parseCookies as safeParseCookies } from '../../base/utils/request';
|
||||
import { AuthService } from '../../core/auth/service';
|
||||
import {
|
||||
createTestingApp,
|
||||
@@ -157,6 +159,19 @@ test('should be able to correct user id cookie', async t => {
|
||||
t.is(userIdCookie, u1.id);
|
||||
});
|
||||
|
||||
test('should not throw on parse of a bad cookie', async t => {
|
||||
const badCookieKey = 'auth_session';
|
||||
const badCookieVal = '^13l3PK9qJs*J%X$MOOOIguhkqWvVh7*';
|
||||
|
||||
const req = {
|
||||
headers: { cookie: `${badCookieKey}=${badCookieVal}` },
|
||||
} as IncomingMessage & { cookies?: Record<string, string> };
|
||||
|
||||
t.notThrows(() => safeParseCookies(req));
|
||||
|
||||
t.is(req.cookies?.[badCookieKey], badCookieVal);
|
||||
});
|
||||
|
||||
// multiple accounts session tests
|
||||
test('should be able to sign in another account in one session', async t => {
|
||||
const { app } = t.context;
|
||||
|
||||
@@ -69,9 +69,23 @@ export function parseCookies(
|
||||
const [key, val] = cookie.split('=');
|
||||
|
||||
if (key) {
|
||||
cookies[decodeURIComponent(key.trim())] = val
|
||||
? decodeURIComponent(val.trim())
|
||||
: val;
|
||||
const rawKey = key.trim();
|
||||
const rawVal = val ? val.trim() : val;
|
||||
|
||||
let safeKey = rawKey;
|
||||
let safeVal = rawVal;
|
||||
|
||||
try {
|
||||
safeKey = decodeURIComponent(rawKey);
|
||||
} catch {}
|
||||
|
||||
if (rawVal) {
|
||||
try {
|
||||
safeVal = decodeURIComponent(rawVal);
|
||||
} catch {}
|
||||
}
|
||||
|
||||
cookies[safeKey] = safeVal;
|
||||
}
|
||||
|
||||
return cookies;
|
||||
|
||||
Reference in New Issue
Block a user