Address comments

Author: minggangw

lib/parameter_event_handler.js

@@ -16,6 +16,7 @@
 
 const { TypeValidationError, OperationError } = require('./errors');
 const { normalizeNodeName } = require('./utils');
+const validator = require('./validator');
 const debug = require('debug')('rclnodejs:parameter_event_handler');
 
 const PARAMETER_EVENT_MSG_TYPE = 'rcl_interfaces/msg/ParameterEvent';
@@ -251,7 +252,10 @@ class ParameterEventHandler {
           }
         );
       }
-      return this.#resolvePath(nodeName.trim());
+
+      const resolvedNodeName = this.#resolvePath(nodeName.trim());
+      this.#validateFullyQualifiedNodePath(resolvedNodeName);
+      return resolvedNodeName;
     });
 
     const contentFilter = {
@@ -510,10 +514,6 @@ class ParameterEventHandler {
    * @private
    */
   #resolvePath(nodePath) {
-    if (!nodePath) {
-      return this.#node.getFullyQualifiedName();
-    }
-
     if (nodePath.startsWith('/')) {
       return nodePath;
     }
@@ -523,6 +523,22 @@ class ParameterEventHandler {
     return resolvedPath.startsWith('/') ? resolvedPath : `/${resolvedPath}`;
   }
 
+  /**
+   * Validate a fully qualified node path before using it in a content filter.
+   * @private
+   */
+  #validateFullyQualifiedNodePath(nodePath) {
+    const normalizedPath =
+      nodePath.length > 1 ? nodePath.replace(/\/+$/, '') : nodePath;
+    const separatorIndex = normalizedPath.lastIndexOf('/');
+    const nodeNamespace =
+      separatorIndex === 0 ? '/' : normalizedPath.slice(0, separatorIndex);
+    const nodeName = normalizedPath.slice(separatorIndex + 1);
+
+    validator.validateNamespace(nodeNamespace);
+    validator.validateNodeName(nodeName);
+  }
+
   /**
    * Check if the handler has been destroyed and throw if so.
    * @private

test/test-parameter-event-handler.js

@@ -418,6 +418,12 @@ describe('ParameterEventHandler tests', function () {
       assert.throws(() => {
         handler.configureNodesFilter([1]);
       });
+      assert.throws(() => {
+        handler.configureNodesFilter(["bad'node"]);
+      });
+      assert.throws(() => {
+        handler.configureNodesFilter(['/invalid_node?']);
+      });
     });
   });
 

test/types/index.test-d.ts

@@ -111,6 +111,28 @@ expectType<rclnodejs.Options<string | rclnodejs.QoS>>(
 );
 expectType<string>(node.getFullyQualifiedName());
 expectType<string>(node.getRMWImplementationIdentifier());
+const parameterEventHandler = node.createParameterEventHandler();
+expectType<rclnodejs.ParameterEventHandler>(parameterEventHandler);
+expectType<boolean>(parameterEventHandler.configureNodesFilter());
+expectType<boolean>(parameterEventHandler.configureNodesFilter(['/test_node']));
+
+const parameterCallbackHandle = parameterEventHandler.addParameterCallback(
+  'test_param',
+  '/test_node',
+  (parameter: any) => {
+    const receivedParameter = parameter;
+  }
+);
+expectType<rclnodejs.ParameterCallbackHandle>(parameterCallbackHandle);
+
+const parameterEventCallbackHandle =
+  parameterEventHandler.addParameterEventCallback((event: any) => {
+    const receivedEvent = event;
+  });
+expectType<rclnodejs.ParameterEventCallbackHandle>(
+  parameterEventCallbackHandle
+);
+
 const nodeWithArgs = rclnodejs.createNode(
   NODE_NAME,
   'topic',