Add Actor ID validation (#539)

* Add Actor ID validation Signed-off-by: heunghingwan <[email protected]> * Validatie Actor ID cannot be empty Signed-off-by: heunghingwan <[email protected]> * Encode URI instead of checking Signed-off-by: heunghingwan <[email protected]> * Make empty check more general Signed-off-by: heunghingwan <[email protected]> * Update ActorId.ts Signed-off-by: heunghingwan <[email protected]> * only use encoded actor id in http protocol Signed-off-by: heunghingwan <[email protected]> * fix prettier error Signed-off-by: heunghingwan <[email protected]> --------- Signed-off-by: heunghingwan <[email protected]>
dapr · Nov 28, 2023 · 46c83d9 · 46c83d9
1 parent f661574
commit 46c83d9
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 10 deletions.
diff --git a/src/actors/ActorId.ts b/src/actors/ActorId.ts
@@ -17,6 +17,9 @@ export default class ActorId {
   private readonly id: string;
 
   constructor(id: string) {
+    if (!id) {
+      throw new Error("ActorId cannot be empty");
+    }
     this.id = id;
   }
 
@@ -28,6 +31,10 @@ export default class ActorId {
     return this.id;
   }
 
+  getURLSafeId() {
+    return encodeURIComponent(this.id);
+  }
+
   toString() {
     return this.id;
   }

diff --git 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<object> {
-    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<void> {
-    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<KeyValueType | string> {
-    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<void> {
-    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<object> {
-    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<void> {
-    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<void> {
-    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<void> {
-    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<void> {
-    await this.client.execute(`/actors/${actorType}/${actorId}`, {
+  async deactivate(actorType: string, actorId: ActorId): Promise<void> {
+    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
@@ -123,6 +123,26 @@ describe("http/actors", () => {
     });
   });
 
+  describe("actorId", () => {
+    it("should be able to create an actorId", () => {
+      const actorId = ActorId.createRandomId();
+      expect(actorId.getId()).toBeDefined();
+      expect(actorId.getURLSafeId()).toBeDefined();
+      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 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");
+    });
+  });
+
   describe("actorProxy", () => {
     it("should be able to create an actor object through the proxy", async () => {
       const builder = new ActorProxyBuilder<DemoActorCounterInterface>(DemoActorCounterImpl, client);