From 3a01b7ccb99dd58c6a3ef3a6a42f5f0b21b5c5c1 Mon Sep 17 00:00:00 2001 From: heunghingwan Date: Thu, 19 Oct 2023 03:37:41 +0800 Subject: [PATCH 1/7] Add Actor ID validation Signed-off-by: heunghingwan --- src/actors/ActorId.ts | 1 + test/e2e/http/actors.test.ts | 12 ++++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/actors/ActorId.ts b/src/actors/ActorId.ts index 4fadc8c0..f644f394 100644 --- a/src/actors/ActorId.ts +++ b/src/actors/ActorId.ts @@ -17,6 +17,7 @@ export default class ActorId { private readonly id: string; constructor(id: string) { + if (id.match(/\//)) throw new Error("ActorId cannot contain '/'"); this.id = id; } diff --git a/test/e2e/http/actors.test.ts b/test/e2e/http/actors.test.ts index be36d930..bc277bce 100644 --- a/test/e2e/http/actors.test.ts +++ b/test/e2e/http/actors.test.ts @@ -119,6 +119,18 @@ describe("http/actors", () => { }); }); + describe("actorId", () => { + it("should be able to create an actorId", () => { + const actorId = ActorId.createRandomId(); + expect(actorId.getId()).toBeDefined(); + expect(actorId.toString()).toBeDefined(); + }); + + it("should not be able to create an actorId with a slash", () => { + expect(() => new ActorId("test/test")).toThrowError("ActorId cannot contain '/'"); + }); + }) + describe("actorProxy", () => { it("should be able to create an actor object through the proxy", async () => { const builder = new ActorProxyBuilder(DemoActorCounterImpl, client); From 5416556a3162be240398e623e745d47beb4a6ed2 Mon Sep 17 00:00:00 2001 From: heunghingwan Date: Thu, 19 Oct 2023 15:04:06 +0800 Subject: [PATCH 2/7] Validatie Actor ID cannot be empty Signed-off-by: heunghingwan --- src/actors/ActorId.ts | 1 + test/e2e/http/actors.test.ts | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/actors/ActorId.ts b/src/actors/ActorId.ts index f644f394..4dbca345 100644 --- a/src/actors/ActorId.ts +++ b/src/actors/ActorId.ts @@ -17,6 +17,7 @@ export default class ActorId { private readonly id: string; constructor(id: string) { + if(id==='') throw new Error("ActorId cannot be empty") if (id.match(/\//)) throw new Error("ActorId cannot contain '/'"); this.id = id; } diff --git a/test/e2e/http/actors.test.ts b/test/e2e/http/actors.test.ts index bc277bce..18c9e55f 100644 --- a/test/e2e/http/actors.test.ts +++ b/test/e2e/http/actors.test.ts @@ -126,6 +126,10 @@ describe("http/actors", () => { expect(actorId.toString()).toBeDefined(); }); + it("should not be able to create an actorId with an empty string", () => { + expect(() => new ActorId("")).toThrowError("ActorId cannot be empty"); + }); + it("should not be able to create an actorId with a slash", () => { expect(() => new ActorId("test/test")).toThrowError("ActorId cannot contain '/'"); }); From ceb54c51a73cf8098339f4e24663814d37c3dcb8 Mon Sep 17 00:00:00 2001 From: heunghingwan Date: Thu, 19 Oct 2023 15:23:34 +0800 Subject: [PATCH 3/7] Encode URI instead of checking Signed-off-by: heunghingwan --- src/actors/ActorId.ts | 9 ++++----- test/e2e/http/actors.test.ts | 8 +++++--- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/actors/ActorId.ts b/src/actors/ActorId.ts index 4dbca345..c9b8746d 100644 --- a/src/actors/ActorId.ts +++ b/src/actors/ActorId.ts @@ -17,9 +17,8 @@ export default class ActorId { private readonly id: string; constructor(id: string) { - if(id==='') throw new Error("ActorId cannot be empty") - if (id.match(/\//)) throw new Error("ActorId cannot contain '/'"); - this.id = id; + if (id === "") throw new Error("ActorId cannot be empty"); + this.id = encodeURIComponent(id); } static createRandomId(): ActorId { @@ -27,10 +26,10 @@ export default class ActorId { } getId() { - return this.id; + return decodeURIComponent(this.id); } toString() { - return this.id; + return decodeURIComponent(this.id); } } diff --git a/test/e2e/http/actors.test.ts b/test/e2e/http/actors.test.ts index 18c9e55f..04d61775 100644 --- a/test/e2e/http/actors.test.ts +++ b/test/e2e/http/actors.test.ts @@ -130,10 +130,12 @@ describe("http/actors", () => { expect(() => new ActorId("")).toThrowError("ActorId cannot be empty"); }); - it("should not be able to create an actorId with a slash", () => { - expect(() => new ActorId("test/test")).toThrowError("ActorId cannot contain '/'"); + it("should be able to create an actorId with url unsafe characters like '/'", () => { + const actorId = new ActorId("test/actor"); + expect(actorId.getId()).toEqual("test/actor"); + expect(actorId.toString()).toEqual("test/actor"); }); - }) + }); describe("actorProxy", () => { it("should be able to create an actor object through the proxy", async () => { From b1b55c37f116bd825564640e0d73bb19e96bb6b6 Mon Sep 17 00:00:00 2001 From: heunghingwan Date: Thu, 19 Oct 2023 16:20:06 +0800 Subject: [PATCH 4/7] Make empty check more general Signed-off-by: heunghingwan --- src/actors/ActorId.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actors/ActorId.ts b/src/actors/ActorId.ts index c9b8746d..0418f331 100644 --- a/src/actors/ActorId.ts +++ b/src/actors/ActorId.ts @@ -17,7 +17,7 @@ export default class ActorId { private readonly id: string; constructor(id: string) { - if (id === "") throw new Error("ActorId cannot be empty"); + if (!id) throw new Error("ActorId cannot be empty"); this.id = encodeURIComponent(id); } From 0f59f23cb9cc5b1ac876c68ee569e7ae7983b599 Mon Sep 17 00:00:00 2001 From: heunghingwan Date: Thu, 19 Oct 2023 20:35:17 +0800 Subject: [PATCH 5/7] Update ActorId.ts Signed-off-by: heunghingwan --- src/actors/ActorId.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/actors/ActorId.ts b/src/actors/ActorId.ts index 0418f331..2213f196 100644 --- a/src/actors/ActorId.ts +++ b/src/actors/ActorId.ts @@ -17,7 +17,9 @@ export default class ActorId { private readonly id: string; constructor(id: string) { - if (!id) throw new Error("ActorId cannot be empty"); + if (!id) { + throw new Error("ActorId cannot be empty"); + } this.id = encodeURIComponent(id); } From 242ac1aa28a0ba5040fed2c148fe5f249f659f37 Mon Sep 17 00:00:00 2001 From: heunghingwan Date: Sat, 28 Oct 2023 05:38:01 +0800 Subject: [PATCH 6/7] only use encoded actor id in http protocol Signed-off-by: heunghingwan --- src/actors/ActorId.ts | 10 +++++++--- .../client/ActorClient/ActorClientHTTP.ts | 20 +++++++++---------- test/e2e/http/actors.test.ts | 2 ++ 3 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/actors/ActorId.ts b/src/actors/ActorId.ts index 2213f196..2deae7c3 100644 --- a/src/actors/ActorId.ts +++ b/src/actors/ActorId.ts @@ -20,7 +20,7 @@ export default class ActorId { if (!id) { throw new Error("ActorId cannot be empty"); } - this.id = encodeURIComponent(id); + this.id = id; } static createRandomId(): ActorId { @@ -28,10 +28,14 @@ export default class ActorId { } getId() { - return decodeURIComponent(this.id); + return this.id; + } + + getURLSafeId() { + return encodeURIComponent(this.id); } toString() { - return decodeURIComponent(this.id); + return this.id; } } diff --git a/src/actors/client/ActorClient/ActorClientHTTP.ts b/src/actors/client/ActorClient/ActorClientHTTP.ts index c8176afb..515b503c 100644 --- a/src/actors/client/ActorClient/ActorClientHTTP.ts +++ b/src/actors/client/ActorClient/ActorClientHTTP.ts @@ -28,7 +28,7 @@ export default class ActorClientHTTP implements IClientActor { } async invoke(actorType: string, actorId: ActorId, methodName: string, body?: any): Promise { - const result = await this.client.execute(`/actors/${actorType}/${actorId.getId()}/method/${methodName}`, { + const result = await this.client.execute(`/actors/${actorType}/${actorId.getURLSafeId()}/method/${methodName}`, { method: "POST", // we always use POST calls for Invoking (ref: https://github.com/dapr/js-sdk/pull/137#discussion_r772636068) body, }); @@ -37,7 +37,7 @@ export default class ActorClientHTTP implements IClientActor { } async stateTransaction(actorType: string, actorId: ActorId, operations: OperationType[]): Promise { - await this.client.execute(`/actors/${actorType}/${actorId.getId()}/state`, { + await this.client.execute(`/actors/${actorType}/${actorId.getURLSafeId()}/state`, { method: "POST", headers: { "Content-Type": "application/json", @@ -47,7 +47,7 @@ export default class ActorClientHTTP implements IClientActor { } async stateGet(actorType: string, actorId: ActorId, key: string): Promise { - const result = await this.client.execute(`/actors/${actorType}/${actorId.getId()}/state/${key}`); + const result = await this.client.execute(`/actors/${actorType}/${actorId.getURLSafeId()}/state/${key}`); return result as any; } @@ -57,7 +57,7 @@ export default class ActorClientHTTP implements IClientActor { name: string, reminder: ActorReminderType, ): Promise { - await this.client.execute(`/actors/${actorType}/${actorId.getId()}/reminders/${name}`, { + await this.client.execute(`/actors/${actorType}/${actorId.getURLSafeId()}/reminders/${name}`, { method: "POST", headers: { "Content-Type": "application/json", @@ -72,18 +72,18 @@ export default class ActorClientHTTP implements IClientActor { } async reminderGet(actorType: string, actorId: ActorId, name: string): Promise { - const result = await this.client.execute(`/actors/${actorType}/${actorId.getId()}/reminders/${name}`); + const result = await this.client.execute(`/actors/${actorType}/${actorId.getURLSafeId()}/reminders/${name}`); return result as object; } async unregisterActorReminder(actorType: string, actorId: ActorId, name: string): Promise { - await this.client.execute(`/actors/${actorType}/${actorId.getId()}/reminders/${name}`, { + await this.client.execute(`/actors/${actorType}/${actorId.getURLSafeId()}/reminders/${name}`, { method: "DELETE", }); } async registerActorTimer(actorType: string, actorId: ActorId, name: string, timer: ActorTimerType): Promise { - await this.client.execute(`/actors/${actorType}/${actorId.getId()}/timers/${name}`, { + await this.client.execute(`/actors/${actorType}/${actorId.getURLSafeId()}/timers/${name}`, { method: "POST", headers: { "Content-Type": "application/json", @@ -99,13 +99,13 @@ export default class ActorClientHTTP implements IClientActor { } async unregisterActorTimer(actorType: string, actorId: ActorId, name: string): Promise { - await this.client.execute(`/actors/${actorType}/${actorId.getId()}/timers/${name}`, { + await this.client.execute(`/actors/${actorType}/${actorId.getURLSafeId()}/timers/${name}`, { method: "DELETE", }); } - async deactivate(actorType: string, actorId: string): Promise { - await this.client.execute(`/actors/${actorType}/${actorId}`, { + async deactivate(actorType: string, actorId: ActorId): Promise { + await this.client.execute(`/actors/${actorType}/${actorId.getURLSafeId()}`, { method: "DELETE", }); } diff --git a/test/e2e/http/actors.test.ts b/test/e2e/http/actors.test.ts index 04d61775..5c52c36b 100644 --- a/test/e2e/http/actors.test.ts +++ b/test/e2e/http/actors.test.ts @@ -123,6 +123,7 @@ describe("http/actors", () => { it("should be able to create an actorId", () => { const actorId = ActorId.createRandomId(); expect(actorId.getId()).toBeDefined(); + expect(actorId.getURLSafeId()).toBeDefined(); expect(actorId.toString()).toBeDefined(); }); @@ -132,6 +133,7 @@ describe("http/actors", () => { it("should be able to create an actorId with url unsafe characters like '/'", () => { const actorId = new ActorId("test/actor"); + expect(actorId.getURLSafeId()).toEqual("test%2Factor"); expect(actorId.getId()).toEqual("test/actor"); expect(actorId.toString()).toEqual("test/actor"); }); From 59379c8297560e4d13b4e7b6dbdee28f443581b8 Mon Sep 17 00:00:00 2001 From: heunghingwan Date: Sat, 28 Oct 2023 05:44:43 +0800 Subject: [PATCH 7/7] fix prettier error Signed-off-by: heunghingwan --- src/actors/ActorId.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/actors/ActorId.ts b/src/actors/ActorId.ts index 2deae7c3..0e62c02e 100644 --- a/src/actors/ActorId.ts +++ b/src/actors/ActorId.ts @@ -18,7 +18,7 @@ export default class ActorId { constructor(id: string) { if (!id) { - throw new Error("ActorId cannot be empty"); + throw new Error("ActorId cannot be empty"); } this.id = id; }