Skip to content
Permalink
Browse files

Fix URL parsing w/ trailing slash & querystring

  • Loading branch information
scottinet committed Dec 2, 2019
1 parent b6443aa commit 235b87b02b4be42d0cfc2703b18066bcc24bc316
Showing with 70 additions and 13 deletions.
  1. +4 −8 lib/core/httpRouter/index.js
  2. +19 −5 lib/core/httpRouter/routePart.js
  3. +47 −0 test/core/httpRouter/httpRouter.test.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);
}
}

@@ -24,7 +24,6 @@
const
querystring = require('querystring'),
RouteHandler = require('./routeHandler'),
URL = require('url'),
{ has } = require('../../util/safeObject');

/**
@@ -73,10 +72,25 @@ 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);
// we need a fake URL base because the WHATWG API currently doesn't handle
// relative URLs, and the http.IncomingMessage class doesn't provide an
// easy way (and an inexpensive one) of getting an absolute URL
// See: https://github.com/nodejs/node/issues/12682
const parsed = new URL(
message.url,
message.url[0] === '/' ? 'http://fake' : null);

// limit calls to "parsed.pathname", it's a getter joining an array
let pathname = parsed.pathname;

if (pathname[pathname.length - 1] === '/') {
pathname = pathname.slice(0, -1);
}

const qs = {};
parsed.searchParams.forEach((v, k) => (qs[k] = v));

const routeHandler = new RouteHandler(pathname, qs, message);

return getHandlerPart(this, pathname.split('/'), routeHandler);
}
@@ -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);

0 comments on commit 235b87b

Please sign in to comment.
You can’t perform that action at this time.