Skip to content

Commit

Permalink
Move param value decoding to end
Browse files Browse the repository at this point in the history
  • Loading branch information
Eran Hammer committed Nov 23, 2014
1 parent 21d01cf commit fc7978b
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 68 deletions.
36 changes: 23 additions & 13 deletions lib/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
var Hoek = require('hoek');
var Boom = require('boom');
var Regex = require('./regex');
var Router = require('./router');
var Segment = require('./segment');


// Declare internals
Expand Down Expand Up @@ -45,7 +45,7 @@ internals.Router.prototype.add = function (config, route) {
}

var table = (vhost === '*' ? self.routes : self.vhosts[vhost]);
table[method] = table[method] || { routes: [], router: new Router() };
table[method] = table[method] || { routes: [], router: new Segment() };

var analysis = config.analysis || this.analyze(config.path);
var record = {
Expand Down Expand Up @@ -110,21 +110,22 @@ internals.Router.prototype._lookup = function (path, segments, table, method) {
return null;
}

var match = set.router.lookup(path, segments, this.settings); // Returns Error, null, or result object
var match = set.router.lookup(path, segments, this.settings);
if (!match) {
return null;
}

if (match.isBoom) {
return this.specials.badRequest || match;
}

var assignments = {};
var array = [];
for (var i = 0, il = match.array.length; i < il; ++i) {
var name = match.record.params[i];
var value = match.array[i];
if (value !== undefined) {
if (value) {
value = internals.decode(value);
if (value.isBoom) {
return this.specials.badRequest || value;
}

if (assignments[name] !== undefined) {
assignments[name] += '/' + value;
}
Expand All @@ -144,6 +145,17 @@ internals.Router.prototype._lookup = function (path, segments, table, method) {
};


internals.decode = function (value) {

try {
return decodeURIComponent(value);
}
catch (err) {
return Boom.badRequest('Invalid request path');
}
};


internals.Router.prototype.normalize = function (path) {

if (path &&
Expand Down Expand Up @@ -227,28 +239,27 @@ internals.Router.prototype.analyze = function (path) {
if (item.count) {
for (var m = 0; m < item.count; ++m) {
fingers.push('?');
segments.push({ param: true });
segments.push({});
if (m) {
params.push(item.name);
}
}
}
else {
fingers.push('#');
segments.push({ param: true, wildcard: true });
segments.push({ wildcard: true });
}
}
else {
fingers.push('?');
segments.push({ param: true, empty: item.empty });
segments.push({ empty: item.empty });
}
}
else {

// Mixed parameter

var seg = {
param: true,
length: parts.length,
first: typeof parts[0] !== 'string',
segments: [],
Expand Down Expand Up @@ -279,7 +290,6 @@ internals.Router.prototype.analyze = function (path) {
}

return {
path: path,
segments: segments,
fingerprint: '/' + fingers.join('/'),
params: params
Expand Down
71 changes: 16 additions & 55 deletions lib/router.js → lib/segment.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ var Boom = require('boom');
var internals = {};


exports = module.exports = internals.Router = function () {
exports = module.exports = internals.Segment = function () {

this._edge = null; // { segment, record }
this._literals = null; // { literal: { segment, <node> } }
Expand All @@ -19,7 +19,7 @@ exports = module.exports = internals.Router = function () {
};


internals.Router.prototype.add = function (segments, record) {
internals.Segment.prototype.add = function (segments, record) {

/*
{ literal: 'x' } -> x
Expand Down Expand Up @@ -50,7 +50,7 @@ internals.Router.prototype.add = function (segments, record) {

this._literals = this._literals || {};
var literal = (record.settings.isCaseSensitive ? current.literal : current.literal.toLowerCase());
this._literals[literal] = this._literals[literal] || new internals.Router();
this._literals[literal] = this._literals[literal] || new internals.Segment();
var node = this._literals[literal];
if (isEdge) {
Hoek.assert(!node._edge, 'New route', record.path, 'conflicts with existing', node._edge && node._edge.record.path);
Expand All @@ -77,7 +77,7 @@ internals.Router.prototype.add = function (segments, record) {

var mixed = this._mixedLookup(current);
if (!mixed) {
mixed = { segment: current, node: new internals.Router() };
mixed = { segment: current, node: new internals.Segment() };
this._mixed.push(mixed);
this._mixed.sort(internals.mixed);
}
Expand All @@ -95,7 +95,7 @@ internals.Router.prototype.add = function (segments, record) {

// Parameter

this._param = this._param || new internals.Router();
this._param = this._param || new internals.Segment();

if (isEdge) {
Hoek.assert(!this._param._edge, 'New route', record.path, 'conflicts with existing', this._param._edge && this._param._edge.record.path);
Expand All @@ -110,7 +110,7 @@ internals.Router.prototype.add = function (segments, record) {
};


internals.Router.prototype._mixedLookup = function (segment) {
internals.Segment.prototype._mixedLookup = function (segment) {

for (var i = 0, il = this._mixed.length; i < il; ++i) {
if (internals.mixed({ segment: segment }, this._mixed[i]) === 0) {
Expand Down Expand Up @@ -157,7 +157,7 @@ internals.mixed = function (a, b) {
};


internals.Router.prototype.lookup = function (path, segments, options) {
internals.Segment.prototype.lookup = function (path, segments, options) {

// Full literal

Expand Down Expand Up @@ -193,14 +193,7 @@ internals.Router.prototype.lookup = function (path, segments, options) {
if (params) {
var array = [];
for (var p = 1, pl = params.length; p < pl; ++p) {
var value = internals.param(params[p], true);
if (value &&
value.isBoom) {

return value;
}

array.push(value);
array.push(params[p]);
}

var record = internals.deeper(match.node, current, path, segments, array, options);
Expand All @@ -214,30 +207,21 @@ internals.Router.prototype.lookup = function (path, segments, options) {
// Param

if (this._param) {
var value = internals.param(current, this._param._edge && this._param._edge.segment.empty);
if (value === false ||
(value && value.isBoom)) {

return value;
}
if (current ||
!this._param._edge ||
this._param._edge.segment.empty) {

var record = internals.deeper(this._param, current, path, segments, [value], options);
if (record) {
return record;
var record = internals.deeper(this._param, current, path, segments, [current], options);
if (record) {
return record;
}
}
}

// Wildcard

if (this._wildcard) {
var value = internals.param(path.slice(1), true);
if (value &&
value.isBoom) {

return value;
}

return { record: this._wildcard.record, array: [value] };
return { record: this._wildcard.record, array: [path.slice(1)] };
}

return null;
Expand All @@ -258,32 +242,9 @@ internals.deeper = function (match, current, path, segments, array, options) {
else {
var result = match.lookup(path.slice(current.length + 1), segments.slice(1), options);
if (result) {
if (result.isBoom) {
return result;
}

return { record: result.record, array: array.concat(result.array) };
}
}

return null;
};


internals.param = function (value, empty) {

if (!empty && !value) {
return false;
}

if (empty && !value) {
return undefined;
}

try {
return decodeURIComponent(value);
}
catch (err) {
return Boom.badRequest('Invalid request path');
}
};
3 changes: 3 additions & 0 deletions test/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,9 @@ describe('Router', function () {
},
'/aBcde': false,
'/bcde': false
},
'/a/{p}/b': {
'/a/': false
}
};

Expand Down

0 comments on commit fc7978b

Please sign in to comment.