Skip to content

Commit 1cd2d98

Browse files
committed
Reject annotation publish/delete without a type (RSAN1a3)
The spec requires a client-side type check so callers get 40003 instead of a request that is missing the only required annotation field.
1 parent 42a6093 commit 1cd2d98

4 files changed

Lines changed: 15 additions & 39 deletions

File tree

src/common/lib/client/restannotations.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,15 @@ export function constructValidateAnnotation(
5757
}
5858

5959
const annotation = Annotation.fromValues(annotationValues);
60+
if (!annotation.type || typeof annotation.type !== 'string') {
61+
throw new ErrorInfo({
62+
message: 'The annotation argument of annotations.' + methodName + '() must include a non-empty string `type`',
63+
code: 40003,
64+
statusCode: 400,
65+
remediation:
66+
'Set type on the annotation object, e.g. { type: "reaction:unique.v1", name: "👍" }. Other fields are optional.',
67+
});
68+
}
6069
annotation.messageSerial = messageSerial;
6170
if (!annotation.action) {
6271
annotation.action = 'annotation.create';

test/uts/deviations.md

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -86,18 +86,6 @@ These tests assert spec behavior but are skipped by default because they are kno
8686

8787
---
8888

89-
### annotations: RSAN1a3 - type validation missing
90-
91-
**Spec (RSAN1a3)**: The SDK must validate that the user supplied a `type`.
92-
93-
**ably-js behavior**: `constructValidateAnnotation()` does not validate that `type` is present.
94-
95-
**Tests**: `RSAN1a3 - type required` (realtime), `RTAN1a - publish validates type is required` (REST).
96-
97-
**Issue**: [#2194](https://github.com/ably/ably-js/issues/2194)
98-
99-
---
100-
10189
### annotations: RSAN1c4 / RSC22d - idempotent IDs not generated
10290

10391
**Spec (RSAN1c4)**: Annotations with empty `id` should get a generated idempotent ID. **Spec (RSC22d)**: Same for batch publish.

test/uts/realtime/unit/channels/channel_annotations.test.ts

Lines changed: 5 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -610,10 +610,11 @@ describe('uts/realtime/unit/channels/channel_annotations', function () {
610610
/**
611611
* RTAN1a - publish validates type is required
612612
*
613-
* Publishing an annotation without a type field should throw an error.
613+
* Publishing an annotation without a type field should throw an error
614+
* with code 40003.
614615
*/
615616
// UTS: realtime/unit/RTAN1a/validates-type-required-1
616-
it('RTAN1a - publish validates type is required (deviation: ably-js does not validate type client-side)', async function () {
617+
it('RTAN1a - publish validates type is required', async function () {
617618
const { mock } = setupMock({
618619
onMessage: (msg, conn) => {
619620
if (msg.action === 21) {
@@ -642,22 +643,13 @@ describe('uts/realtime/unit/channels/channel_annotations', function () {
642643
const channel = client.channels.get('test-RTAN1a-validate', { attachOnSubscribe: false });
643644
await channel.attach();
644645

645-
// Deviation: ably-js does not validate that type is required client-side.
646-
// The annotation is sent to the server without type validation.
647-
if (!process.env.RUN_DEVIATIONS) {
648-
this.skip();
649-
return;
650-
}
651-
652646
try {
653647
await channel.annotations.publish('msg-serial-1', {
654648
name: 'like',
655-
// type is missing
656649
} as any);
657-
expect.fail('Should have thrown');
650+
expect.fail('Expected publish without type to throw with code 40003');
658651
} catch (err: any) {
659-
expect(err).to.exist;
660-
expect(err.code).to.be.a('number');
652+
expect(err.code).to.equal(40003);
661653
}
662654
client.close();
663655
});

test/uts/rest/unit/channel/annotations.test.ts

Lines changed: 1 addition & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -77,31 +77,18 @@ describe('uts/rest/unit/channel/annotations', function () {
7777
*
7878
* Publishing an annotation without a type field should throw an error
7979
* with code 40003.
80-
*
81-
* NOTE: ably-js does not currently validate the type field in
82-
* constructValidateAnnotation(). This test documents the spec
83-
* requirement (RSAN1a3) as a known deviation — the publish succeeds
84-
* without a type instead of throwing.
8580
*/
8681
// UTS: rest/unit/RSAN1a3/publish-type-required-0
8782
it('RSAN1a3 - type required', async function () {
88-
// DEVIATION: see deviations.md
89-
if (!process.env.RUN_DEVIATIONS) this.skip();
90-
const captured: any[] = [];
9183
const mock = new MockHttpClient({
9284
onConnectionAttempt: (conn) => conn.respond_with_success(),
93-
onRequest: (req) => {
94-
captured.push(req);
95-
req.respond_with(201, {});
96-
},
85+
onRequest: (req) => req.respond_with(201, {}),
9786
});
9887
installMockHttp(mock);
9988

10089
const client = new Ably.Rest({ key: 'appId.keyId:keySecret', useBinaryProtocol: false });
10190
const ch = client.channels.get('test');
10291

103-
// Spec (RSAN1a3): publishing without a type MUST throw with code 40003.
104-
// DEVIATION: ably-js does not validate type. See deviations.md.
10592
try {
10693
await ch.annotations.publish('msg-serial-1', { name: 'like' });
10794
expect.fail('Expected publish without type to throw with code 40003');

0 commit comments

Comments
 (0)