fix: passing value null in post to type number throws type error (#516)

* fix: passing value null in post to type number throws type error

* test: add regression tests

src/packages/router/route/params/errors/parameter-not-nullable-error.js

@@ -0,0 +1,14 @@
+// @flow
+import { createServerError } from '../../../../server';
+import type { ParameterLike } from '../index';
+
+/**
+ * @private
+ */
+class ParameterNotNullableError extends TypeError {
+  constructor({ path }: ParameterLike) {
+    super(`Parameter '${path}' is not nullable.`);
+  }
+}
+
+export default createServerError(ParameterNotNullableError, 400);

src/packages/router/route/params/parameter-group/index.js

@@ -3,7 +3,6 @@ import { FreezeableMap } from '../../../../freezeable';
 import { InvalidParameterError } from '../errors';
 import isNull from '../../../../../utils/is-null';
 import entries from '../../../../../utils/entries';
-import setType from '../../../../../utils/set-type';
 import validateType from '../utils/validate-type';
 import type { ParameterLike, ParameterLike$opts } from '../index';
 
@@ -39,34 +38,33 @@ class ParameterGroup extends FreezeableMap<string, ParameterLike> {
   }
 
   validate<V: Object>(params: V): V {
-    return setType(() => {
-      const validated = {};
+    const validated = {};
 
-      if (isNull(params)) {
-        return params;
-      }
+    if (isNull(params)) {
+      return params;
+    }
 
-      if (validateType(this, params) && hasRequiredParams(this, params)) {
-        const { sanitize } = this;
-        let { path } = this;
+    if (validateType(this, params) && hasRequiredParams(this, params)) {
+      const { sanitize } = this;
+      let { path } = this;
 
-        if (path.length) {
-          path = `${path}.`;
-        }
+      if (path.length) {
+        path = `${path}.`;
+      }
 
-        for (const [key, value] of entries(params)) {
-          const match = this.get(key);
+      for (const [key, value] of entries(params)) {
+        const match = this.get(key);
 
-          if (match) {
-            validated[key] = match.validate(value);
-          } else if (!match && !sanitize) {
-            throw new InvalidParameterError(`${path}${key}`);
-          }
+        if (match) {
+          Reflect.set(validated, key, match.validate(value));
+        } else if (!match && !sanitize) {
+          throw new InvalidParameterError(`${path}${key}`);
         }
       }
+    }
 
-      return validated;
-    });
+    // $FlowIgnore
+    return validated;
   }
 }
 

src/packages/router/route/params/parameter/utils/validate-value.js

@@ -1,11 +1,16 @@
 // @flow
+import isNull from '../../../../../../utils/is-null';
 import { ParameterValueError, ResourceMismatchError } from '../../errors';
 import type Parameter from '../index';
 
 /**
  * @private
  */
 function validateOne<V>(param: Parameter, value: V): V {
+  if (!param.required && isNull(value)) {
+    return value;
+  }
+
   if (!param.has(value)) {
     let expected;
 

src/packages/router/route/params/test/parameter-group.test.js

@@ -44,6 +44,11 @@ describe('module "router/route/params"', () => {
     });
 
     describe('#validate()', () => {
+      it('returns null when then value is null', () => {
+        // $FlowIgnore
+        expect(subject.validate(null)).to.be.null;
+      });
+
       it('fails when required keys are missing', () => {
         expect(() => subject.validate({})).to.throw(TypeError);
         expect(() => subject.validate({ id: 1, meta: {} })).to.throw(TypeError);

src/packages/router/route/params/test/parameter.test.js

@@ -1,35 +1,177 @@
 // @flow
 import { expect } from 'chai';
-import { it, describe, before } from 'mocha';
+import { it, describe, beforeEach } from 'mocha';
 
 import Parameter from '../parameter';
 
 describe('module "router/route/params"', () => {
   describe('class Parameter', () => {
-    let subject: Parameter;
+    describe('#validate()', () => {
+      let subject: Parameter;
+      const primitives = [
+        {
+          type: 'boolean',
+          valid: true,
+          falsy: false,
+          invalid: 'true'
+        },
+        {
+          type: 'string',
+          valid: 'test',
+          falsy: '',
+          invalid: 1
+        },
+        {
+          type: 'number',
+          valid: 1,
+          falsy: 0,
+          invalid: '1'
+        }
+      ];
+
+      describe('- type "array"', () => {
+        [true, false].forEach(required => {
+          describe(`- ${required ? 'required' : 'optional'}`, () => {
+            beforeEach(() => {
+              subject = new Parameter({
+                required,
+                type: 'array',
+                path: 'meta.test',
+                values: [1, 'test', false]
+              });
+            });
+
+            if (required) {
+              it('fails when the value is null', () => {
+                expect(() => subject.validate(null)).to.throw(TypeError);
+              });
+            } else {
+              it('passes when the value is null', () => {
+                expect(subject.validate(null)).to.be.null;
+              });
+            }
+
+            it('fails when there is a type mismatch', () => {
+              expect(() => subject.validate('test')).to.throw(TypeError);
+            });
+
+            it('fails when there is a value mismatch', () => {
+              expect(() => subject.validate([new Date()])).to.throw(TypeError);
+            });
 
-    before(() => {
-      subject = new Parameter({
-        type: 'array',
-        path: 'meta.test',
-        values: [1, 'test', false]
+            it('returns the value when the type and value match', () => {
+              expect(subject.validate(['test', false])).to.deep.equal([
+                'test',
+                false
+              ]);
+            });
+          });
+        });
       });
-    });
 
-    describe('#validate()', () => {
-      it('fails when there is a type mismatch', () => {
-        expect(() => subject.validate('test')).to.throw(TypeError);
+      describe('- type "buffer"', () => {
+        [true, false].forEach(required => {
+          describe(`- ${required ? 'required' : 'optional'}`, () => {
+            let value;
+
+            beforeEach(() => {
+              value = new Buffer('test', 'utf8');
+              subject = new Parameter({
+                required,
+                type: 'buffer',
+                path: 'meta.test'
+              });
+            });
+
+            if (required) {
+              it('fails when the value is null', () => {
+                expect(() => subject.validate(null)).to.throw(TypeError);
+              });
+            } else {
+              it('passes when the value is null', () => {
+                expect(subject.validate(null)).to.be.null;
+              });
+            }
+
+            it('fails when there is a type mismatch', () => {
+              expect(() => subject.validate('test')).to.throw(TypeError);
+            });
+
+            it('returns the value when the type and value match', () => {
+              expect(subject.validate(value)).to.equal(value);
+            });
+          });
+        });
       });
 
-      it('fails when there is a value mismatch', () => {
-        expect(() => subject.validate([new Date()])).to.throw(TypeError);
+      describe('- type "object"', () => {
+        [true, false].forEach(required => {
+          describe(`- ${required ? 'required' : 'optional'}`, () => {
+            let value;
+
+            beforeEach(() => {
+              value = {};
+              subject = new Parameter({
+                required,
+                type: 'object',
+                path: 'meta.test'
+              });
+            });
+
+            if (required) {
+              it('fails when the value is null', () => {
+                expect(() => subject.validate(null)).to.throw(TypeError);
+              });
+            } else {
+              it('passes when the value is null', () => {
+                expect(subject.validate(null)).to.be.null;
+              });
+            }
+
+            it('fails when there is a type mismatch', () => {
+              expect(() => subject.validate('test')).to.throw(TypeError);
+            });
+
+            it('returns the value when the type and value match', () => {
+              expect(subject.validate(value)).to.equal(value);
+            });
+          });
+        });
       });
 
-      it('returns the value(s) when the type and value(s) match', () => {
-        expect(subject.validate(['test', false])).to.deep.equal([
-          'test',
-          false
-        ]);
+      primitives.forEach(({ type, valid, falsy, invalid }) => {
+        describe(`- type "${type}"`, () => {
+          [true, false].forEach(required => {
+            describe(`- ${required ? 'required' : 'optional'}`, () => {
+              beforeEach(() => {
+                subject = new Parameter({
+                  type,
+                  required,
+                  path: 'meta.test'
+                });
+              });
+
+              if (required) {
+                it('fails when the value is null', () => {
+                  expect(() => subject.validate(null)).to.throw(TypeError);
+                });
+              } else {
+                it('passes when the value is null', () => {
+                  expect(subject.validate(null)).to.be.null;
+                });
+              }
+
+              it('fails when there is a type mismatch', () => {
+                expect(() => subject.validate(invalid)).to.throw(TypeError);
+              });
+
+              it('returns the value when the type and value match', () => {
+                expect(subject.validate(valid)).to.equal(valid);
+                expect(subject.validate(falsy)).to.equal(falsy);
+              });
+            });
+          });
+        });
       });
     });
   });

src/packages/router/route/params/utils/validate-type.js

@@ -1,5 +1,6 @@
 // @flow
-import { ParameterTypeError } from '../errors';
+import ParameterTypeError from '../errors/parameter-type-error';
+import ParameterNotNullableError from '../errors/parameter-not-nullable-error';
 import isNull from '../../../../../utils/is-null';
 import isObject from '../../../../../utils/is-object';
 import isBuffer from '../../../../../utils/is-buffer';
@@ -12,33 +13,38 @@ export default function validateType(
   param: Parameter | ParameterGroup,
   value: mixed
 ): true {
-  const { type } = param;
+  const { type, required } = param;
+  const valueIsNull = isNull(value);
 
-  if (type) {
-    const valueType = typeof value;
-    let isValid;
+  if (required && valueIsNull) {
+    throw new ParameterNotNullableError(param);
+  } else if (valueIsNull || !type) {
+    return true;
+  }
+
+  const valueType = typeof value;
+  let isValid = true;
 
-    switch (type) {
-      case 'array':
-        isValid = Array.isArray(value);
-        break;
+  switch (type) {
+    case 'array':
+      isValid = Array.isArray(value);
+      break;
 
-      case 'buffer':
-        isValid = isBuffer(value);
-        break;
+    case 'buffer':
+      isValid = isBuffer(value);
+      break;
 
-      case 'object':
-        isValid = isObject(value) || isNull(value);
-        break;
+    case 'object':
+      isValid = isObject(value) || isNull(value);
+      break;
 
-      default:
-        isValid = type === valueType;
-    }
+    default:
+      isValid = type === valueType;
+  }
 
-    if (!isValid) {
-      throw new ParameterTypeError(param, valueType);
-    }
+  if (!isValid) {
+    throw new ParameterTypeError(param, valueType);
   }
 
-  return true;
+  return isValid;
 }