Skip to content

Commit 092e7ea

Browse files
Merge pull request #201 from ambikeesshh/fix/sort-object-keys-circular-ref
fix(routing): unify sortObjectKeys and guard against circular refs
2 parents 38f7ae8 + 0f83cfd commit 092e7ea

6 files changed

Lines changed: 154 additions & 54 deletions

File tree

src/websocket/routing/message-router.spec.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,28 @@ describe('MessageRouter', () => {
331331
});
332332
});
333333

334+
describe('circular reference protection', () => {
335+
it('should throw when registering a handler with a circular pattern', () => {
336+
const pattern: Record<string, unknown> = { cmd: 'test' };
337+
pattern.self = pattern;
338+
339+
expect(() => {
340+
router.registerHandlers([createHandler(pattern, () => 'result')]);
341+
}).toThrow('Circular reference detected');
342+
});
343+
344+
it('should throw when routing a circular event object', () => {
345+
router.registerHandlers([createHandler('safe', () => 'ok')]);
346+
347+
const circular: Record<string, unknown> = { cmd: 'test' };
348+
circular.self = circular;
349+
350+
expect(() => {
351+
router.hasHandler(circular);
352+
}).toThrow('Circular reference detected');
353+
});
354+
});
355+
334356
describe('edge cases with JSON serialization', () => {
335357
it('should treat undefined values as omitted (JSON behavior)', async () => {
336358
const pattern = { cmd: 'test', value: 1 };

src/websocket/routing/message-router.ts

Lines changed: 2 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Logger } from '@nestjs/common';
22
import { MessageHandler } from './metadata-scanner';
3+
import { sortObjectKeys } from './pattern-key';
34

45
/**
56
* Represents an incoming WebSocket message
@@ -157,33 +158,7 @@ export class MessageRouter {
157158
return pattern;
158159
}
159160
// For object patterns, create a stable JSON string key with sorted keys (recursively)
160-
return JSON.stringify(this.sortObjectKeys(pattern));
161-
}
162-
163-
/**
164-
* Recursively sorts object keys for stable serialization
165-
* @private
166-
*/
167-
private sortObjectKeys(obj: Record<string, unknown>): Record<string, unknown> {
168-
const sorted: Record<string, unknown> = {};
169-
for (const key of Object.keys(obj).sort()) {
170-
sorted[key] = this.sortValue(obj[key]);
171-
}
172-
return sorted;
173-
}
174-
175-
/**
176-
* Recursively sorts values (handles objects, arrays, and primitives)
177-
* @private
178-
*/
179-
private sortValue(value: unknown): unknown {
180-
if (value === null || typeof value !== 'object') {
181-
return value;
182-
}
183-
if (Array.isArray(value)) {
184-
return value.map((item) => this.sortValue(item));
185-
}
186-
return this.sortObjectKeys(value as Record<string, unknown>);
161+
return JSON.stringify(sortObjectKeys(pattern));
187162
}
188163

189164
/**

src/websocket/routing/metadata-scanner.spec.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,30 @@ describe('MetadataScanner', () => {
186186
).toBe('handleMessage');
187187
});
188188

189+
it('should match object patterns with arrays containing out-of-order keys', () => {
190+
const pattern = {
191+
cmd: 'batch',
192+
items: [
193+
{ b: 1, a: 2 },
194+
{ d: 4, c: 3 },
195+
],
196+
};
197+
addMetadata('handleMessage', pattern);
198+
199+
scanner.scanForMessageHandlers(gateway);
200+
201+
// Same pattern but object keys in different order inside arrays
202+
expect(
203+
scanner.getMethodNameForEvent(gateway, {
204+
cmd: 'batch',
205+
items: [
206+
{ a: 2, b: 1 },
207+
{ c: 3, d: 4 },
208+
],
209+
})
210+
).toBe('handleMessage');
211+
});
212+
189213
it('should return null for non-existent event', () => {
190214
addMetadata('handleMessage', 'message');
191215

src/websocket/routing/metadata-scanner.ts

Lines changed: 3 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { Logger } from '@nestjs/common';
22
import 'reflect-metadata';
3+
import { sortObjectKeys } from './pattern-key';
34

45
/**
56
* Metadata keys used by @SubscribeMessage decorator
@@ -103,7 +104,7 @@ export class MetadataScanner {
103104
message: messagePattern,
104105
messageKey:
105106
typeof messagePattern === 'object'
106-
? JSON.stringify(this.sortObjectKeys(messagePattern))
107+
? JSON.stringify(sortObjectKeys(messagePattern))
107108
: undefined,
108109
methodName,
109110
callback: method.bind(instance),
@@ -156,7 +157,7 @@ export class MetadataScanner {
156157
// Pre-compute eventKey for object patterns to avoid repeated serialization
157158
const eventKey =
158159
typeof event === 'object' && event !== null
159-
? JSON.stringify(this.sortObjectKeys(event))
160+
? JSON.stringify(sortObjectKeys(event))
160161
: undefined;
161162

162163
const handler = handlers.find((h) => {
@@ -178,29 +179,4 @@ export class MetadataScanner {
178179

179180
return handler ? handler.methodName : null;
180181
}
181-
182-
/**
183-
* Recursively sorts object keys for stable serialization
184-
* @private
185-
*/
186-
private sortObjectKeys(
187-
obj: Record<string, unknown>,
188-
seen = new WeakSet<object>()
189-
): Record<string, unknown> {
190-
if (seen.has(obj)) {
191-
throw new Error('Circular reference detected in message pattern');
192-
}
193-
seen.add(obj);
194-
195-
const sorted: Record<string, unknown> = {};
196-
for (const key of Object.keys(obj).sort()) {
197-
const value = obj[key];
198-
sorted[key] =
199-
value !== null && typeof value === 'object' && !Array.isArray(value)
200-
? this.sortObjectKeys(value as Record<string, unknown>, seen)
201-
: value;
202-
}
203-
seen.delete(obj);
204-
return sorted;
205-
}
206182
}
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { sortObjectKeys } from './pattern-key';
2+
3+
describe('sortObjectKeys', () => {
4+
it('should sort top-level keys alphabetically', () => {
5+
const result = sortObjectKeys({ b: 2, a: 1, c: 3 });
6+
expect(Object.keys(result)).toEqual(['a', 'b', 'c']);
7+
expect(result).toEqual({ a: 1, b: 2, c: 3 });
8+
});
9+
10+
it('should return empty object for empty input', () => {
11+
expect(sortObjectKeys({})).toEqual({});
12+
});
13+
14+
it('should sort nested object keys recursively', () => {
15+
const result = sortObjectKeys({ z: { y: 2, x: 1 }, a: 0 });
16+
expect(Object.keys(result)).toEqual(['a', 'z']);
17+
expect(Object.keys(result.z as object)).toEqual(['x', 'y']);
18+
});
19+
20+
it('should handle arrays without modifying order', () => {
21+
const result = sortObjectKeys({ items: [3, 1, 2] });
22+
expect(result.items).toEqual([3, 1, 2]);
23+
});
24+
25+
it('should sort keys of objects nested inside arrays', () => {
26+
const result = sortObjectKeys({
27+
items: [
28+
{ b: 1, a: 2 },
29+
{ d: 3, c: 4 },
30+
],
31+
});
32+
const items = result.items as Record<string, unknown>[];
33+
expect(Object.keys(items[0])).toEqual(['a', 'b']);
34+
expect(Object.keys(items[1])).toEqual(['c', 'd']);
35+
});
36+
37+
it('should handle nested arrays', () => {
38+
const result = sortObjectKeys({ matrix: [[{ z: 1, a: 2 }]] });
39+
const inner = (result.matrix as unknown[][])[0][0] as Record<string, unknown>;
40+
expect(Object.keys(inner)).toEqual(['a', 'z']);
41+
});
42+
43+
it('should pass through primitives as-is', () => {
44+
const result = sortObjectKeys({ str: 'hello', num: 42, bool: true, nil: null });
45+
expect(result).toEqual({ bool: true, nil: null, num: 42, str: 'hello' });
46+
});
47+
48+
it('should throw on circular references', () => {
49+
const obj: Record<string, unknown> = { a: 1 };
50+
obj.self = obj;
51+
expect(() => sortObjectKeys(obj)).toThrow('Circular reference detected');
52+
});
53+
54+
it('should throw on deeply nested circular references', () => {
55+
const inner: Record<string, unknown> = { value: 1 };
56+
const outer: Record<string, unknown> = { nested: inner };
57+
inner.parent = outer;
58+
expect(() => sortObjectKeys(outer)).toThrow('Circular reference detected');
59+
});
60+
61+
it('should allow the same object in multiple non-circular branches', () => {
62+
const shared = { x: 1, y: 2 };
63+
const result = sortObjectKeys({ a: shared, b: shared });
64+
expect(Object.keys(result.a as object)).toEqual(['x', 'y']);
65+
expect(Object.keys(result.b as object)).toEqual(['x', 'y']);
66+
});
67+
68+
it('should handle undefined and function values like JSON.stringify', () => {
69+
const result = sortObjectKeys({ a: 1, b: undefined, c: () => {} });
70+
expect(result).toEqual({ a: 1, b: undefined, c: expect.any(Function) });
71+
});
72+
});
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* Recursively sorts object keys for stable JSON serialization.
3+
* Handles nested objects, arrays, and primitives.
4+
* Throws on circular references.
5+
*/
6+
export function sortObjectKeys(
7+
obj: Record<string, unknown>,
8+
seen = new WeakSet<object>()
9+
): Record<string, unknown> {
10+
if (seen.has(obj)) {
11+
throw new Error('Circular reference detected in message pattern');
12+
}
13+
seen.add(obj);
14+
15+
const sorted: Record<string, unknown> = {};
16+
for (const key of Object.keys(obj).sort()) {
17+
sorted[key] = sortValue(obj[key], seen);
18+
}
19+
seen.delete(obj);
20+
return sorted;
21+
}
22+
23+
function sortValue(value: unknown, seen: WeakSet<object>): unknown {
24+
if (value === null || typeof value !== 'object') {
25+
return value;
26+
}
27+
if (Array.isArray(value)) {
28+
return value.map((item) => sortValue(item, seen));
29+
}
30+
return sortObjectKeys(value as Record<string, unknown>, seen);
31+
}

0 commit comments

Comments
 (0)