Skip to content

fix(liveobjects): reject live object references as map values #2246

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
sacOO7 wants to merge 1 commit into
base: main
Choose a base branch
from
+22 −1
Open

fix(liveobjects): reject live object references as map values #2246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 15 additions & 0 deletions src/plugins/liveobjects/livemap.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,12 @@ import {
Primitive,
Value,
} from '../../../liveobjects';
import { DefaultInstance } from './instance';
import { LiveCounter } from './livecounter';
import { LiveCounterValueType } from './livecountervaluetype';
import { LiveMapValueType } from './livemapvaluetype';
import { LiveObject, LiveObjectData, LiveObjectUpdate, LiveObjectUpdateNoop } from './liveobject';
import { DefaultPathObject } from './pathobject';
import {
getObjectDataPrimitive,
MapRemove,
Expand Down Expand Up @@ -186,6 +188,19 @@ export class LiveMap<T extends Record<string, Value> = Record<string, Value>>
) {
throw new client.ErrorInfo('Map value data type is unsupported', 40013, 400); // OD4a
}

// RTLMV4c1 - live objects obtained from the channel, and the public objects that wrap them,
// are not valid map values. Without this check they would fall through to the JSON encoding
// branch and fail with a confusing serialization error (or leak internal state to the wire).
// To assign an object to a key, a new object must be created via the LiveMap.create() or
// LiveCounter.create() value types instead.
if (value instanceof LiveObject || value instanceof DefaultPathObject || value instanceof DefaultInstance) {
throw new client.ErrorInfo(
'Map value data type is unsupported: a reference to an existing object cannot be assigned as a map value; use LiveMap.create() or LiveCounter.create() to create a new object',
40013,
400,
);
}
}

/**
Expand Down
8 changes: 7 additions & 1 deletion test/realtime/liveobjects.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4060,7 +4060,7 @@ define(['ably', 'shared_helper', 'chai', 'liveobjects', 'liveobjects_helper'], f
{
description: 'LiveMap.set throws on invalid input',
action: async (ctx) => {
const { objectsHelper, channelName, entryInstance } = ctx;
const { objectsHelper, channelName, entryInstance, entryPathObject } = ctx;

const mapCreatedPromise = waitForMapKeyUpdate(entryInstance, 'map');
await objectsHelper.createAndSetOnMap(channelName, {
Expand All @@ -4086,6 +4086,12 @@ define(['ably', 'shared_helper', 'chai', 'liveobjects', 'liveobjects_helper'], f
await expectToThrowAsync(async () => map.set('key', null), 'Map value data type is unsupported');
await expectToThrowAsync(async () => map.set('key', BigInt(1)), 'Map value data type is unsupported');
await expectToThrowAsync(async () => map.set('key', Symbol()), 'Map value data type is unsupported');

// RTLMV4c1 - references to existing objects (Instance or PathObject) are not valid
// map values; only LiveMap.create()/LiveCounter.create() value types can assign objects
await expectToThrowAsync(async () => map.set('key', map), 'Map value data type is unsupported');
await expectToThrowAsync(async () => map.set('key', entryInstance), 'Map value data type is unsupported');
await expectToThrowAsync(async () => map.set('key', entryPathObject), 'Map value data type is unsupported');
Comment on lines +4090 to +4094

@coderabbitai coderabbitai Bot Jun 12, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert RTLMV4c1 error code/status in the new rejection checks.

These new cases currently verify only the message. Since this PR contract requires code: 40013 and statusCode: 400, add those assertions to prevent silent regressions in error mapping.

Suggested test tightening 
-            await expectToThrowAsync(async () => map.set('key', map), 'Map value data type is unsupported');
-            await expectToThrowAsync(async () => map.set('key', entryInstance), 'Map value data type is unsupported');
-            await expectToThrowAsync(async () => map.set('key', entryPathObject), 'Map value data type is unsupported');
+            await expectToThrowAsync(async () => map.set('key', map), 'Map value data type is unsupported', {
+              withCode: 40013,
+              withStatusCode: 400,
+            });
+            await expectToThrowAsync(async () => map.set('key', entryInstance), 'Map value data type is unsupported', {
+              withCode: 40013,
+              withStatusCode: 400,
+            });
+            await expectToThrowAsync(async () => map.set('key', entryPathObject), 'Map value data type is unsupported', {
+              withCode: 40013,
+              withStatusCode: 400,
+            });
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/realtime/liveobjects.test.js` around lines 4090 - 4094, The new
rejection tests for RTLMV4c1 only assert the error message; update the calls to
expectToThrowAsync used around map.set (the three cases referencing map,
entryInstance, entryPathObject) to also assert the error object includes code:
40013 and statusCode: 400; specifically modify the expectToThrowAsync
invocations for these three assertions to verify both the message ('Map value
data type is unsupported') and that the thrown error has code 40013 and
statusCode 400 so the test enforces the PR's error-mapping contract.

},
},

Expand Down
Loading