From b4e757784e2b9ffe2c22b65b21e8bf665abd9f56 Mon Sep 17 00:00:00 2001
From: =?utf8?q?J=C3=A9r=C3=B4me=20Benoit?= <jerome.benoit@sap.com>
Date: Sun, 2 Apr 2023 15:27:54 +0200
Subject: [PATCH] fix: use UUIDv4 for message id to avoid integer overflow
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

Signed-off-by: Jérôme Benoit <jerome.benoit@sap.com>
---
 CHANGELOG.md               |  1 +
 src/pools/abstract-pool.ts | 22 ++++++++--------------
 src/utility-types.ts       |  2 +-
 3 files changed, 10 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 36db7f82..b21ed16f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ### Fixed
 
 - Ensure trimmable characters are checked at pool initialization.
+- Fix message id integer overflow.
 
 ## [2.3.10] - 2023-03-18
 
diff --git a/src/pools/abstract-pool.ts b/src/pools/abstract-pool.ts
index 1d4e7ae0..d63c19ac 100644
--- a/src/pools/abstract-pool.ts
+++ b/src/pools/abstract-pool.ts
@@ -50,14 +50,9 @@ export abstract class AbstractPool<
    * When we receive a message from the worker we get a map entry and resolve/reject the promise based on the message.
    */
   protected promiseMap: Map<
-  number,
+  string,
   PromiseWorkerResponseWrapper<Worker, Response>
-  > = new Map<number, PromiseWorkerResponseWrapper<Worker, Response>>()
-
-  /**
-   * Id of the next message.
-   */
-  protected nextMessageId: number = 0
+  > = new Map<string, PromiseWorkerResponseWrapper<Worker, Response>>()
 
   /**
    * Worker choice strategy instance implementing the worker choice algorithm.
@@ -158,7 +153,7 @@ export abstract class AbstractPool<
   }
 
   /**
-   * Gets worker key.
+   * Gets the given worker key.
    *
    * @param worker - The worker.
    * @returns The worker key.
@@ -220,16 +215,15 @@ export abstract class AbstractPool<
 
   /** {@inheritDoc} */
   public async execute (data: Data): Promise<Response> {
-    // Configure worker to handle message with the specified task
     const worker = this.chooseWorker()
-    const res = this.internalExecute(worker, this.nextMessageId)
+    const messageId = crypto.randomUUID()
+    const res = this.internalExecute(worker, messageId)
     this.checkAndEmitBusy()
     this.sendToWorker(worker, {
       // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
       data: data ?? ({} as Data),
-      id: this.nextMessageId
+      id: messageId
     })
-    ++this.nextMessageId
     // eslint-disable-next-line @typescript-eslint/return-await
     return res
   }
@@ -398,7 +392,7 @@ export abstract class AbstractPool<
 
   private async internalExecute (
     worker: Worker,
-    messageId: number
+    messageId: string
   ): Promise<Response> {
     this.beforePromiseWorkerResponseHook(worker)
     return await new Promise<Response>((resolve, reject) => {
@@ -431,7 +425,7 @@ export abstract class AbstractPool<
   }
 
   /**
-   * Get tasks usage of the given worker.
+   * Gets tasks usage of the given worker.
    *
    * @param worker - Worker which tasks usage is returned.
    */
diff --git a/src/utility-types.ts b/src/utility-types.ts
index 2d142f86..fba39012 100644
--- a/src/utility-types.ts
+++ b/src/utility-types.ts
@@ -22,7 +22,7 @@ export interface MessageValue<
   /**
    * Id of the message.
    */
-  readonly id?: number
+  readonly id?: string
   /**
    * Kill code.
    */
-- 
2.34.1