diff --git a/lib/core/httpRouter/index.js b/lib/core/httpRouter/index.js index c382b26380..ff0629d7d7 100644 --- a/lib/core/httpRouter/index.js +++ b/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); } } diff --git a/lib/core/httpRouter/routePart.js b/lib/core/httpRouter/routePart.js index 4a55a24540..0717fbd33c 100644 --- a/lib/core/httpRouter/routePart.js +++ b/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); } diff --git a/test/core/httpRouter/httpRouter.test.js b/test/core/httpRouter/httpRouter.test.js index 181936d155..85e473771f 100644 --- a/test/core/httpRouter/httpRouter.test.js +++ b/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);