refactor: convert Request, Response, CodeResponseType, TokenResponseType to ES6 classes

Merge pull request #225 from menewman/fix-convert-request-response-classes-to-es6 thanks to @menewman

Author: jankapunkt

lib/request.js

@@ -7,67 +7,53 @@
 const InvalidArgumentError = require('./errors/invalid-argument-error');
 const typeis = require('type-is');
 
-/**
- * Constructor.
- */
-
-function Request(options) {
-  options = options || {};
+class Request {
+  constructor({ headers, method, query, body, ...otherOptions } = {}) {
+    if (!headers) {
+      throw new InvalidArgumentError('Missing parameter: `headers`');
+    }
 
-  if (!options.headers) {
-    throw new InvalidArgumentError('Missing parameter: `headers`');
-  }
+    if (!method) {
+      throw new InvalidArgumentError('Missing parameter: `method`');
+    }
 
-  if (!options.method) {
-    throw new InvalidArgumentError('Missing parameter: `method`');
-  }
+    if (!query) {
+      throw new InvalidArgumentError('Missing parameter: `query`');
+    }
 
-  if (!options.query) {
-    throw new InvalidArgumentError('Missing parameter: `query`');
+    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;
+      });
   }
 
-  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];
-    }
+  /**
+   * Get a request header.
+   * @param {String} field
+   */
+  get(field) {
+    return this.headers[field.toLowerCase()];
   }
 
-  // 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];
-    }
+  /**
+   * Check if the content-type matches any of the given mime types.
+   * @param {...String|Array} types
+   */
+  is(...types) {
+    return typeis(this, types.flat()) || 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;

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;

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;

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;

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();
 

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();