fix(transport-commons): Handle invalid service paths on socket lookups (#3242)

Author: daffl

package-lock.json

@@ -15,7 +15,13 @@ export interface SocketOptions {
   getParams: (socket: any) => RealTimeConnection;
 }
 
-export function socket ({ done, emit, socketMap, socketKey, getParams }: SocketOptions) {
+export function socket ({
+  done,
+  emit,
+  socketMap,
+  socketKey,
+  getParams
+}: SocketOptions) {
   return (app: Application) => {
     const leaveChannels = (connection: RealTimeConnection) => {
       const { channels } = app;
@@ -39,53 +45,71 @@ export function socket ({ done, emit, socketMap, socketKey, getParams }: SocketO
     });
 
     // `connection` event
-    done.then(provider => provider.on('connection', (connection: any) =>
-      app.emit('connection', getParams(connection)))
+    done.then((provider) =>
+      provider.on('connection', (connection: any) =>
+        app.emit('connection', getParams(connection))
+      )
     );
 
     // `socket.emit('methodName', 'serviceName', ...args)` handlers
-    done.then(provider => provider.on('connection', (connection: any) => {
-      for (const method of app.methods) {
-        connection.on(method, (...args: any[]) => {
-          const path = args.shift();
+    done.then((provider) =>
+      provider.on('connection', (connection: any) => {
+        for (const method of app.methods) {
+          connection.on(method, (...args: any[]) => {
+            const [path, ...rest] = args;
 
-          debug(`Got '${method}' call for service '${path}'`);
-          runMethod(app, getParams(connection), path, method, args);
-        });
-      }
-
-      connection.on('authenticate', (...args: any[]) => {
-        if (app.get('defaultAuthentication')) {
-          debug('Got legacy authenticate event');
-          runMethod(app, getParams(connection), app.get('defaultAuthentication'), 'create', args);
+            runMethod(app, getParams(connection), path, method, rest);
+          });
         }
-      });
 
-      connection.on('logout', (callback: any) => {
-        if (app.get('defaultAuthentication')) {
-          debug('Got legacy authenticate event');
-          runMethod(app, getParams(connection), app.get('defaultAuthentication'), 'remove', [ null, {}, callback ]);
-        }
-      });
-    }));
+        connection.on('authenticate', (...args: any[]) => {
+          if (app.get('defaultAuthentication')) {
+            debug('Got legacy authenticate event');
+            runMethod(
+              app,
+              getParams(connection),
+              app.get('defaultAuthentication'),
+              'create',
+              args
+            );
+          }
+        });
+
+        connection.on('logout', (callback: any) => {
+          if (app.get('defaultAuthentication')) {
+            debug('Got legacy authenticate event');
+            runMethod(
+              app,
+              getParams(connection),
+              app.get('defaultAuthentication'),
+              'remove',
+              [null, {}, callback]
+            );
+          }
+        });
+      })
+    );
 
     // Legacy `socket.emit('serviceName::methodName', ...args)` handlers
-    app.mixins.push((service, path) => done.then(provider => {
-      provider.on('connection', (socket: any) => {
-        const methods = app.methods.filter(current =>
-          // @ts-ignore
-          typeof service[current] === 'function'
-        );
+    app.mixins.push((service, path) =>
+      done.then((provider) => {
+        provider.on('connection', (socket: any) => {
+          const methods = app.methods.filter(
+            (current) =>
+              // @ts-ignore
+              typeof service[current] === 'function'
+          );
 
-        for (const method of methods) {
-          const eventName = `${path}::${method}`;
+          for (const method of methods) {
+            const eventName = `${path}::${method}`;
 
-          socket.on(eventName, (...args: any[]) => {
-            debug(`Got legacy method call '${eventName}'`);
-            runMethod(app, getParams(socket), path, method, args);
-          });
-        }
-      });
-    }));
+            socket.on(eventName, (...args: any[]) => {
+              debug(`Got legacy method call '${eventName}'`);
+              runMethod(app, getParams(socket), path, method, args);
+            });
+          }
+        });
+      })
+    );
   };
 }

packages/transport-commons/src/socket/index.ts

@@ -62,7 +62,9 @@ export function getDispatcher (emit: string, socketMap: WeakMap<RealTimeConnecti
   };
 }
 
-export function runMethod (app: Application, connection: RealTimeConnection, path: string, method: string, args: any[]) {
+export function runMethod (app: Application, connection: RealTimeConnection, _path: string, _method: string, args: any[]) {
+  const path = typeof _path === 'string' ? _path : null
+  const method = typeof _method === 'string' ? _method : null
   const trace = `method '${method}' on service '${path}'`;
   const methodArgs = args.slice(0);
   const callback = typeof methodArgs[methodArgs.length - 1] === 'function'
@@ -81,7 +83,7 @@ export function runMethod (app: Application, connection: RealTimeConnection, pat
     // No valid service was found, return a 404
     // just like a REST route would
     if (lookup === null) {
-      return Promise.reject(new errors.NotFound(`Service '${path}' not found`));
+      return Promise.reject(new errors.NotFound(path === null ? 'Invalid service path' : `Service '${path}' not found`));
     }
 
     const { service, params: route = {} } = lookup;

packages/transport-commons/src/socket/utils.ts

@@ -76,14 +76,26 @@ describe('@feathersjs/transport-commons', () => {
       });
     });
 
-    it('.get with invalid service name and arguments', done => {
+    it('method with invalid service name and arguments', (done) => {
       const socket = new EventEmitter();
 
       provider.emit('connection', socket);
 
       socket.emit('get', null, (error: any) => {
         assert.strictEqual(error.name, 'NotFound');
-        assert.strictEqual(error.message, `Service 'null' not found`);
+        assert.strictEqual(error.message, 'Invalid service path');
+        done();
+      });
+    });
+
+    it('method with implicit toString errors properly', (done) => {
+      const socket = new EventEmitter()
+
+      provider.emit('connection', socket)
+
+      socket.emit('get', { toString: '' }, (error: any) => {
+        assert.strictEqual(error.name, 'NotFound');
+        assert.strictEqual(error.message, 'Invalid service path');
         done();
       });
     });