Fix URL parsing w/ trailing slash & querystring

lib/core/httpRouter/index.js

@@ -28,9 +28,7 @@ const
   RoutePart = require('./routePart'),
   { Request, errors: { KuzzleError } } = require('kuzzle-common-objects');
 
-const
-  LeadingSlashRegex = /\/+$/,
-  CharsetRegex = /charset=([\w-]+)/i;
+const CharsetRegex = /charset=([\w-]+)/i;
 
 /**
  * Attach handler to routes and dispatch a HTTP
@@ -148,8 +146,6 @@ class Router {
       return this.routeUnhandledHttpMethod(message, cb);
     }
 
-    message.url = message.url.replace(LeadingSlashRegex, '');
-
     let routeHandler;
 
     try {
@@ -245,10 +241,10 @@ class Router {
  * @param {RoutePart} target
  */
 function attach(url, handler, target) {
-  const cleanedUrl = url.replace(LeadingSlashRegex, '');
+  const sanitized = url[url.length - 1] === '/' ? url.slice(0, -1) : url;
 
-  if (!attachParts(cleanedUrl.split('/'), handler, target)) {
-    errorsManager.throw('duplicate_url', cleanedUrl);
+  if (!attachParts(sanitized.split('/'), handler, target)) {
+    errorsManager.throw('duplicate_url', sanitized);
   }
 }
 

lib/core/httpRouter/routePart.js

@@ -22,9 +22,9 @@
 'use strict';
 
 const
+  URL = require('url'),
   querystring = require('querystring'),
   RouteHandler = require('./routeHandler'),
-  URL = require('url'),
   { has } = require('../../util/safeObject');
 
 /**
@@ -73,10 +73,18 @@ class RoutePart {
    * @return {RouteHandler} registered function handler
    */
   getHandler(message) {
-    const
-      parsed = URL.parse(message.url, true),
-      pathname = parsed.pathname || '', // pathname is set to null if empty
-      routeHandler = new RouteHandler(pathname, parsed.query, message);
+    // Do not use WHATWG API yet, stick with the legacy (and deprecated) URL
+    // There are two issues:
+    //   - Heavy performance impact: https://github.com/nodejs/node/issues/30334
+    //   - Double slash bug: https://github.com/nodejs/node/issues/30776
+    const parsed = URL.parse(message.url, true);
+    let pathname = parsed.pathname || ''; // pathname is set to null if empty
+
+    if (pathname[pathname.length - 1] === '/') {
+      pathname = pathname.slice(0, -1);
+    }
+
+    const routeHandler = new RouteHandler(pathname, parsed.query, message);
 
     return getHandlerPart(this, pathname.split('/'), routeHandler);
   }

test/core/httpRouter/httpRouter.test.js

@@ -65,8 +65,11 @@ describe('core/httpRouter', () => {
 
     it('should raise an internal error when trying to add a duplicate', () => {
       router.post('/foo/bar', handler);
+
       should(function () { router.post('/foo/bar', handler); })
         .throw(InternalError, { id: 'network.http.duplicate_url' });
+      should(function () { router.post('/foo/bar/', handler); })
+        .throw(InternalError, { id: 'network.http.duplicate_url' });
     });
   });
 
@@ -166,6 +169,50 @@ describe('core/httpRouter', () => {
       });
     });
 
+    it('should properly handle querystrings (w/o url trailing slash)', done => {
+      router.post('/foo/bar', handler);
+
+      rq.url = '/foo/bar?foo=bar';
+      rq.method = 'POST';
+
+      router.route(rq, () => {
+        try {
+          should(handler).be.calledOnce();
+
+          const payload = handler.firstCall.args[0];
+          should(payload).be.instanceOf(Request);
+          should(payload.input.args.foo).eql('bar');
+
+          done();
+        }
+        catch (e) {
+          done(e);
+        }
+      });
+    });
+
+    it('should properly handle querystrings (w/ url trailing slash)', done => {
+      router.post('/foo/bar', handler);
+
+      rq.url = '/foo/bar/?foo=bar';
+      rq.method = 'POST';
+
+      router.route(rq, () => {
+        try {
+          should(handler).be.calledOnce();
+
+          const payload = handler.firstCall.args[0];
+          should(payload).be.instanceOf(Request);
+          should(payload.input.args.foo).eql('bar');
+
+          done();
+        }
+        catch (e) {
+          done(e);
+        }
+      });
+    });
+
     it('should amend the request object if a body is found in the content', done => {
       router.post('/foo/bar', handler);