From 0d142f0d0666c75b3a69b67c5a8e59ade7da580b Mon Sep 17 00:00:00 2001 From: Michael Newman Date: Thu, 17 Aug 2023 09:44:01 -0700 Subject: [PATCH 1/3] Convert Request, Response, CodeResponseType, TokenResponseType to ES6 classes --- lib/request.js | 92 ++++++++++------------- lib/response-types/code-response-type.js | 42 ++++------- lib/response-types/token-response-type.js | 14 +--- lib/response.js | 83 +++++++++----------- 4 files changed, 96 insertions(+), 135 deletions(-) diff --git a/lib/request.js b/lib/request.js index 560b29bc..f20130bc 100644 --- a/lib/request.js +++ b/lib/request.js @@ -7,67 +7,57 @@ const InvalidArgumentError = require('./errors/invalid-argument-error'); const typeis = require('type-is'); -/** - * Constructor. - */ +class Request { + constructor({ headers, method, query, body, ...otherOptions } = {}) { + if (!headers) { + throw new InvalidArgumentError('Missing parameter: `headers`'); + } -function Request(options) { - options = options || {}; + if (!method) { + throw new InvalidArgumentError('Missing parameter: `method`'); + } - if (!options.headers) { - throw new InvalidArgumentError('Missing parameter: `headers`'); - } + if (!query) { + throw new InvalidArgumentError('Missing parameter: `query`'); + } - if (!options.method) { - throw new InvalidArgumentError('Missing parameter: `method`'); + this.body = body || {}; + this.headers = {}; + this.method = method; + this.query = query; + + // Store the headers in lower case. + Object.entries(headers).forEach(([header, value]) => { + this.headers[header.toLowerCase()] = value; + }); + + // Store additional properties of the request object passed in + Object.entries(otherOptions) + .filter(([property]) => !this[property]) + .forEach(([property, value]) => { + this[property] = value; + }); } - if (!options.query) { - throw new InvalidArgumentError('Missing parameter: `query`'); + /** + * Get a request header. + * @param {String} field + */ + get(field) { + return this.headers[field.toLowerCase()]; } - this.body = options.body || {}; - this.headers = {}; - this.method = options.method; - this.query = options.query; - - // Store the headers in lower case. - for (const field in options.headers) { - if (Object.prototype.hasOwnProperty.call(options.headers, field)) { - this.headers[field.toLowerCase()] = options.headers[field]; + /** + * Check if the content-type matches any of the given mime types. + * @param {...String|Array} types + */ + is(...types) { + if (types.length === 1 && Array.isArray(types[0])) { + types = types[0]; } - } - // Store additional properties of the request object passed in - for (const property in options) { - if (Object.prototype.hasOwnProperty.call(options, property) && !this[property]) { - this[property] = options[property]; - } + return typeis(this, types) || false; } } -/** - * Get a request header. - */ - -Request.prototype.get = function(field) { - return this.headers[field.toLowerCase()]; -}; - -/** - * Check if the content-type matches any of the given mime type. - */ - -Request.prototype.is = function(types) { - if (!Array.isArray(types)) { - types = [].slice.call(arguments); - } - - return typeis(this, types) || false; -}; - -/** - * Export constructor. - */ - module.exports = Request; diff --git a/lib/response-types/code-response-type.js b/lib/response-types/code-response-type.js index 8252248f..6311d22a 100644 --- a/lib/response-types/code-response-type.js +++ b/lib/response-types/code-response-type.js @@ -7,37 +7,27 @@ const InvalidArgumentError = require('../errors/invalid-argument-error'); const url = require('url'); -/** - * Constructor. - */ +class CodeResponseType { + constructor(code) { + if (!code) { + throw new InvalidArgumentError('Missing parameter: `code`'); + } -function CodeResponseType(code) { - if (!code) { - throw new InvalidArgumentError('Missing parameter: `code`'); + this.code = code; } - this.code = code; -} + buildRedirectUri(redirectUri) { + if (!redirectUri) { + throw new InvalidArgumentError('Missing parameter: `redirectUri`'); + } -/** - * Build redirect uri. - */ - -CodeResponseType.prototype.buildRedirectUri = function(redirectUri) { - if (!redirectUri) { - throw new InvalidArgumentError('Missing parameter: `redirectUri`'); - } + const uri = url.parse(redirectUri, true); - const uri = url.parse(redirectUri, true); + uri.query.code = this.code; + uri.search = null; - uri.query.code = this.code; - uri.search = null; - - return uri; -}; - -/** - * Export constructor. - */ + return uri; + } +} module.exports = CodeResponseType; diff --git a/lib/response-types/token-response-type.js b/lib/response-types/token-response-type.js index 29c32e70..cd6891b4 100644 --- a/lib/response-types/token-response-type.js +++ b/lib/response-types/token-response-type.js @@ -6,16 +6,10 @@ const ServerError = require('../errors/server-error'); -/** - * Constructor. - */ - -function TokenResponseType() { - throw new ServerError('Not implemented.'); +class TokenResponseType { + constructor() { + throw new ServerError('Not implemented.'); + } } -/** - * Export constructor. - */ - module.exports = TokenResponseType; diff --git a/lib/response.js b/lib/response.js index 29a2c517..23725963 100644 --- a/lib/response.js +++ b/lib/response.js @@ -1,58 +1,45 @@ 'use strict'; -/** - * Constructor. - */ - -function Response(options) { - options = options || {}; +class Response { + constructor({ headers = {}, body = {}, ...otherOptions } = {}) { + this.status = 200; + this.body = body; + this.headers = {}; + + // Store the headers in lower case. + Object.entries(headers).forEach(([header, value]) => { + this.headers[header.toLowerCase()] = value; + }); + + // Store additional properties of the response object passed in + Object.entries(otherOptions) + .filter(([property]) => !this[property]) + .forEach(([property, value]) => { + this[property] = value; + }); + } - this.body = options.body || {}; - this.headers = {}; - this.status = 200; + /** + * Get a response header. + */ + get(field) { + return this.headers[field.toLowerCase()]; + } - // Store the headers in lower case. - for (const field in options.headers) { - if (Object.prototype.hasOwnProperty.call(options.headers, field)) { - this.headers[field.toLowerCase()] = options.headers[field]; - } + /** + * Redirect response. + */ + redirect(url) { + this.set('Location', url); + this.status = 302; } - // Store additional properties of the response object passed in - for (const property in options) { - if (Object.prototype.hasOwnProperty.call(options, property) && !this[property]) { - this[property] = options[property]; - } + /** + * Set a response header. + */ + set(field, value) { + this.headers[field.toLowerCase()] = value; } } -/** - * Get a response header. - */ - -Response.prototype.get = function(field) { - return this.headers[field.toLowerCase()]; -}; - -/** - * Redirect response. - */ - -Response.prototype.redirect = function(url) { - this.set('Location', url); - this.status = 302; -}; - -/** - * Set a response header. - */ - -Response.prototype.set = function(field, value) { - this.headers[field.toLowerCase()] = value; -}; - -/** - * Export constructor. - */ - module.exports = Response; From a23d68249937f9bf83534ebdcdf7f89bce142f92 Mon Sep 17 00:00:00 2001 From: Michael Newman Date: Sat, 26 Aug 2023 01:51:45 -0700 Subject: [PATCH 2/3] Use types.flat() to handle 'is' arguments --- lib/request.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/request.js b/lib/request.js index f20130bc..7ac01a44 100644 --- a/lib/request.js +++ b/lib/request.js @@ -52,11 +52,7 @@ class Request { * @param {...String|Array} types */ is(...types) { - if (types.length === 1 && Array.isArray(types[0])) { - types = types[0]; - } - - return typeis(this, types) || false; + return typeis(this, types.flat()) || false; } } From 8ea66994831892dc28ec0215246ecd2a3752f08c Mon Sep 17 00:00:00 2001 From: Michael Newman Date: Sat, 26 Aug 2023 02:22:51 -0700 Subject: [PATCH 3/3] Push unit tests that verify that prototype methods can't be overwritten --- test/unit/request_test.js | 16 ++++++++++++++++ test/unit/response_test.js | 14 ++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/test/unit/request_test.js b/test/unit/request_test.js index f292e2b4..fe3c136e 100644 --- a/test/unit/request_test.js +++ b/test/unit/request_test.js @@ -127,6 +127,22 @@ describe('Request', function() { request.custom2.should.eql(originalRequest.custom2); }); + it('should not allow overwriting methods on the Request prototype via custom properties', () => { + const request = new Request({ + query: {}, + method: 'GET', + headers: { + 'content-type': 'application/json' + }, + get() { + // malicious attempt to override the 'get' method + return 'text/html'; + } + }); + + request.get('content-type').should.equal('application/json'); + }); + it('should allow getting of headers using `request.get`', function() { const originalRequest = generateBaseRequest(); diff --git a/test/unit/response_test.js b/test/unit/response_test.js index 8d4897c9..af505ba9 100644 --- a/test/unit/response_test.js +++ b/test/unit/response_test.js @@ -83,6 +83,20 @@ describe('Request', function() { response.custom2.should.eql(originalResponse.custom2); }); + it('should not allow overwriting methods on the Response prototype via custom properties', () => { + const response = new Response({ + headers: { + 'content-type': 'application/json' + }, + get() { + // malicious attempt to override the 'get' method + return 'text/html'; + } + }); + + response.get('content-type').should.equal('application/json'); + }); + it('should allow getting of headers using `response.get`', function() { const originalResponse = generateBaseResponse();