Skip to content

Commit

Permalink
Coverage
Browse files Browse the repository at this point in the history
  • Loading branch information
@hueniverse
hueniverse committed Jan 18, 2019
1 parent a3a5ca9 commit 7521468
Show file tree
Hide file tree
Showing 18 changed files with 308 additions and 112 deletions.
12 changes: 5 additions & 7 deletions lib/auth.js
View file
Expand Up @@ -428,11 +428,11 @@ internals.setupScope = function (access) {
scope[type] = scope[type] || [];
scope[type].push(clean);

if ((!scope._parameters || !scope._parameters[type]) &&
if ((!scope._hasParameters || !scope._hasParameters[type]) &&
/{([^}]+)}/.test(clean)) {

scope._parameters = scope._parameters || {};
scope._parameters[type] = true;
scope._hasParameters = scope._hasParameters || {};
scope._hasParameters[type] = true;
}
}

Expand Down Expand Up @@ -487,7 +487,7 @@ internals.validate = function (err, result, name, config, request, errors) {

internals.expandScope = function (request, scope) {

if (!scope._parameters) {
if (!scope._hasParameters) {
return scope;
}

Expand All @@ -503,9 +503,7 @@ internals.expandScope = function (request, scope) {

internals.expandScopeType = function (request, scope, type) {

if (!scope[type] ||
!scope._parameters[type]) {

if (!scope._hasParameters[type]) {
return scope[type];
}

Expand Down
9 changes: 6 additions & 3 deletions lib/config.js
View file
Expand Up @@ -3,7 +3,6 @@
const Os = require('os');

const Joi = require('joi');
const Hoek = require('hoek');


const internals = {};
Expand All @@ -12,10 +11,14 @@ const internals = {};
exports.symbol = Symbol('hapi-response');


exports.apply = function (type, options, message) {
exports.apply = function (type, options, ...message) {

const result = Joi.validate(options, internals[type]);
Hoek.assert(!result.error, 'Invalid', type, 'options', message ? '(' + message + ')' : '', result.error && result.error.annotate());

if (result.error) {
throw new Error(`Invalid ${type} options ${message.length ? '(' + message.join(' ') + ')' : ''} ${result.error.annotate()}`);
}

return result.value;
};

Expand Down
8 changes: 4 additions & 4 deletions lib/methods.js
View file
Expand Up @@ -104,15 +104,15 @@ exports = module.exports = internals.Methods = class {
};


internals.supportedArgs = ['string', 'number', 'boolean'];


internals.generateKey = function (...args) {

let key = '';
for (let i = 0; i < args.length; ++i) {
const arg = args[i];
if (typeof arg !== 'string' &&
typeof arg !== 'number' &&
typeof arg !== 'boolean') {

if (!internals.supportedArgs.includes(typeof arg)) {
return null;
}

Expand Down
5 changes: 1 addition & 4 deletions lib/request.js
View file
Expand Up @@ -309,10 +309,7 @@ exports = module.exports = internals.Request = class {
}

try {
var response = (typeof func === 'function' ? func(this) : this._invoke(func));
if (response && typeof response.then === 'function') { // Skip await if no reason to
response = await response;
}
var response = await (typeof func === 'function' ? func(this) : this._invoke(func));
}
catch (err) {
Bounce.rethrow(err, 'system');
Expand Down
31 changes: 17 additions & 14 deletions lib/response.js
View file
Expand Up @@ -250,23 +250,26 @@ exports = module.exports = internals.Response = class {

// Weak verifier

const ifModifiedSinceHeader = request.headers['if-modified-since'];

if (ifModifiedSinceHeader &&
options.modified) {
if (!options.modified) {
return false;
}

const ifModifiedSince = internals.parseDate(ifModifiedSinceHeader);
const lastModified = internals.parseDate(options.modified);
const ifModifiedSinceHeader = request.headers['if-modified-since'];
if (!ifModifiedSinceHeader) {
return false;
}

if (ifModifiedSince &&
lastModified &&
ifModifiedSince >= lastModified) {
const ifModifiedSince = internals.parseDate(ifModifiedSinceHeader);
if (!ifModifiedSince) {
return false;
}

return true;
}
const lastModified = internals.parseDate(options.modified);
if (!lastModified) {
return false;
}

return false;
return ifModifiedSince >= lastModified;
}

type(type) {
Expand Down Expand Up @@ -527,8 +530,8 @@ exports = module.exports = internals.Response = class {
// Stream source

if (source instanceof Stream) {
if (typeof source._read !== 'function' || typeof source._readableState !== 'object') {
throw Boom.badImplementation('Stream must have a streams2 readable interface');
if (typeof source._read !== 'function') {
throw Boom.badImplementation('Stream must have a readable interface');
}

if (source._readableState.objectMode) {
Expand Down
127 changes: 73 additions & 54 deletions lib/route.js
View file
Expand Up @@ -30,17 +30,21 @@ exports = module.exports = internals.Route = class {

// Routing information

const display = `${route.method} ${route.path}`;
Config.apply('route', route, display);
Config.apply('route', route, route.method, route.path);

const method = route.method.toLowerCase();
Hoek.assert(method !== 'head', 'Method name not allowed:', display);
Hoek.assert(method !== 'head', 'Cannot set HEAD route:', route.path);

const path = (realm.modifiers.route.prefix ? realm.modifiers.route.prefix + (route.path !== '/' ? route.path : '') : route.path);
Hoek.assert(path === '/' || path[path.length - 1] !== '/' || !core.settings.router.stripTrailingSlash, 'Path cannot end with a trailing slash when configured to strip:', display);
Hoek.assert(path === '/' || path[path.length - 1] !== '/' || !core.settings.router.stripTrailingSlash, 'Path cannot end with a trailing slash when configured to strip:', route.method, route.path);

const vhost = (realm.modifiers.route.vhost || route.vhost);

// Set identifying members (assert)

this.method = method;
this.path = path;

// Prepare configuration

let config = route.options || route.config || {};
Expand All @@ -50,17 +54,22 @@ exports = module.exports = internals.Route = class {

config = Config.enable(config); // Shallow clone

// Verify route level config (as opposed to the merged settings)

this._assert(method !== 'get' || !config.payload, 'Cannot set payload settings on HEAD or GET request');
this._assert(method !== 'get' || !config.validate || !config.validate.payload, 'Cannot validate HEAD or GET request payload');

// Rules

Hoek.assert(!route.rules || !config.rules, 'Route rules can only appear once:', display); // XOR
this._assert(!route.rules || !config.rules, 'Route rules can only appear once'); // XOR
const rules = (route.rules || config.rules);
const rulesConfig = internals.rules(rules, { method, path, vhost }, server);
delete config.rules;

// Handler

Hoek.assert(route.handler || config.handler, 'Missing or undefined handler:', display);
Hoek.assert(!!route.handler ^ !!config.handler, 'Handler must only appear once:', display); // XOR
this._assert(route.handler || config.handler, 'Missing or undefined handler');
this._assert(!!route.handler ^ !!config.handler, 'Handler must only appear once'); // XOR

const handler = Config.apply('handler', route.handler || config.handler);
delete config.handler;
Expand All @@ -70,19 +79,17 @@ exports = module.exports = internals.Route = class {
// Apply settings in order: server <- handler <- realm <- route

const settings = internals.config([core.settings.routes, handlerDefaults, realm.settings, rulesConfig, config]);
this.settings = Config.apply('routeConfig', settings, display);
this.settings = Config.apply('routeConfig', settings, method, path);

// Validate timeouts

const socketTimeout = (this.settings.timeout.socket === undefined ? 2 * 60 * 1000 : this.settings.timeout.socket);
Hoek.assert(!this.settings.timeout.server || !socketTimeout || this.settings.timeout.server < socketTimeout, 'Server timeout must be shorter than socket timeout:', display);
Hoek.assert(!this.settings.payload.timeout || !socketTimeout || this.settings.payload.timeout < socketTimeout, 'Payload timeout must be shorter than socket timeout:', display);
this._assert(!this.settings.timeout.server || !socketTimeout || this.settings.timeout.server < socketTimeout, 'Server timeout must be shorter than socket timeout');
this._assert(!this.settings.payload.timeout || !socketTimeout || this.settings.payload.timeout < socketTimeout, 'Payload timeout must be shorter than socket timeout');

// Route members

this._core = core;
this.path = path;
this.method = method;
this.realm = realm;

this.settings.vhost = vhost;
Expand Down Expand Up @@ -110,45 +117,7 @@ exports = module.exports = internals.Route = class {

// Validation

const validation = this.settings.validate;
if (this.method === 'get') {

// Assert on config, not on merged settings

Hoek.assert(!config.payload, 'Cannot set payload settings on HEAD or GET request:', display);
Hoek.assert(!config.validate || !config.validate.payload, 'Cannot validate HEAD or GET requests:', display);

validation.payload = null;
}

Hoek.assert(!validation.params || this.params.length, 'Cannot set path parameters validations without path parameters:', display);

['headers', 'params', 'query', 'payload', 'state'].forEach((type) => {

validation[type] = Validation.compile(validation[type]);
});

if (this.settings.response.schema !== undefined ||
this.settings.response.status) {

this.settings.response._validate = true;

const rule = this.settings.response.schema;
this.settings.response.status = this.settings.response.status || {};
const statuses = Object.keys(this.settings.response.status);

if (rule === true &&
!statuses.length) {

this.settings.response._validate = false;
}
else {
this.settings.response.schema = Validation.compile(rule);
for (const code of statuses) {
this.settings.response.status[code] = Validation.compile(this.settings.response.status[code]);
}
}
}
this._setupValidation();

// Payload parsing

Expand All @@ -159,9 +128,9 @@ exports = module.exports = internals.Route = class {
this.settings.payload.decoders = this._core.compression._decoders; // Reference the shared object to keep up to date
}

Hoek.assert(!this.settings.validate.payload || this.settings.payload.parse, 'Route payload must be set to \'parse\' when payload validation enabled:', display);
Hoek.assert(!this.settings.validate.state || this.settings.state.parse, 'Route state must be set to \'parse\' when state validation enabled:', display);
Hoek.assert(!this.settings.jsonp || typeof this.settings.jsonp === 'string', 'Bad route JSONP parameter name:', display);
this._assert(!this.settings.validate.payload || this.settings.payload.parse, 'Route payload must be set to \'parse\' when payload validation enabled');
this._assert(!this.settings.validate.state || this.settings.state.parse, 'Route state must be set to \'parse\' when state validation enabled');
this._assert(!this.settings.jsonp || typeof this.settings.jsonp === 'string', 'Bad route JSONP parameter name');

// Authentication configuration

Expand Down Expand Up @@ -211,6 +180,43 @@ exports = module.exports = internals.Route = class {
this.rebuild();
}

_setupValidation() {

const validation = this.settings.validate;
if (this.method === 'get') {
validation.payload = null;
}

this._assert(!validation.params || this.params.length, 'Cannot set path parameters validations without path parameters');

['headers', 'params', 'query', 'payload', 'state'].forEach((type) => {

validation[type] = Validation.compile(validation[type]);
});

if (this.settings.response.schema !== undefined ||
this.settings.response.status) {

this.settings.response._validate = true;

const rule = this.settings.response.schema;
this.settings.response.status = this.settings.response.status || {};
const statuses = Object.keys(this.settings.response.status);

if (rule === true &&
!statuses.length) {

this.settings.response._validate = false;
}
else {
this.settings.response.schema = Validation.compile(rule);
for (const code of statuses) {
this.settings.response.status[code] = Validation.compile(this.settings.response.status[code]);
}
}
}
}

rebuild(event) {

if (event) {
Expand Down Expand Up @@ -344,6 +350,19 @@ exports = module.exports = internals.Route = class {
this._marshalCycle.push(Auth.response); // Must be last in case requires access to headers
}
}

_assert(condition, message) {

This comment has been minimized.

Sign in to view

Copy link
@kanongil

kanongil Jan 18, 2019

Contributor

Why throw plain Error instead of Assert.AssertionError? It might mess up bounce.rethrow()-based logic.

This comment has been minimized.

Sign in to view

Copy link
@hueniverse

hueniverse Jan 18, 2019

Author Contributor

Just an oversight.

if (condition) {
return;
}

if (this.method[0] === '_') {
throw new Error(message);
}

throw new Error(`${message}: ${this.method.toUpperCase()} ${this.path}`);
}
};


Expand Down
2 changes: 1 addition & 1 deletion lib/toolkit.js
View file
Expand Up @@ -152,7 +152,7 @@ internals.Toolkit = class {
entity(options) {

Hoek.assert(options, 'Entity method missing required options');
Hoek.assert(options.etag || options.modified, 'Entity methods missing require options key');
Hoek.assert(options.etag || options.modified, 'Entity methods missing required options key');

this.request._entity = options;

Expand Down
5 changes: 1 addition & 4 deletions lib/transmit.js
View file
Expand Up @@ -38,10 +38,7 @@ exports.send = async function (request) {
internals.marshal = async function (request) {

for (const func of request._route._marshalCycle) {
const result = func(request);
if (result && typeof result.then === 'function') { // Skip await if no reason to
await result;
}
await func(request);
}
};

Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Expand Up @@ -39,7 +39,7 @@
"handlebars": "4.x.x",
"hapitoc": "1.x.x",
"inert": "5.x.x",
"lab": "17.x.x",
"lab": "18.x.x",
"vision": "5.x.x",
"wreck": "14.x.x"
},
Expand Down

0 comments on commit 7521468

Please sign in to comment.