Save JSON parse. Closes hapijs#1910

runofthemillgeek · Jun 18, 2019 · ba7ca31 · ba7ca31
1 parent 1abae21
commit ba7ca31
Show file tree

Hide file tree

Showing 11 changed files with 85 additions and 60 deletions.
diff --git a/lib/index.js b/lib/index.js
@@ -326,12 +326,13 @@ internals.root = function () {
 
                     if (ctor.prototype._coerce) {
                         const baseRet = ctor.prototype._coerce.call(this, value, state, prefs);
+                        if (baseRet) {
+                            if (baseRet.errors) {
+                                return baseRet;
+                            }
 
-                        if (baseRet.errors) {
-                            return baseRet;
+                            value = baseRet.value;
                         }
-
-                        value = baseRet.value;
                     }
 
                     const ret = extension.coerce.call(this, value, state, prefs);

diff --git a/lib/types/array.js b/lib/types/array.js
@@ -1,5 +1,6 @@
 'use strict';
 
+const Bourne = require('@hapi/bourne');
 const Hoek = require('@hapi/hoek');
 
 const Any = require('./any');
@@ -29,18 +30,23 @@ internals.Array = class extends Any {
 
         const result = { value };
 
-        if (value &&
-            typeof value === 'string' &&
-            (value[0] === '[' || /^\s*\[/.test(value))) {
+        if (typeof value === 'string') {
+            if (value[0] !== '[' &&
+                !/^\s*\[/.test(value)) {
+
+                return;
+            }
 
             try {
-                result.value = JSON.parse(value);
+                result.value = Bourne.parse(value);
+            }
+            catch (ignoreErr) {
+                return;
             }
-            catch (ignoreErr) { }
         }
 
         if (!Array.isArray(result.value)) {
-            return result;
+            return;
         }
 
         const sort = this._uniqueRules.get('sort');

diff --git a/lib/types/binary.js b/lib/types/binary.js
@@ -19,16 +19,14 @@ internals.Binary = class extends Any {
 
     _coerce(value, state, prefs) {
 
-        const result = { value };
-
-        if (typeof value === 'string') {
-            try {
-                result.value = Buffer.from(value, this._flags.encoding);
-            }
-            catch (ignoreErr) { }
+        if (typeof value !== 'string') {
+            return;
         }
 
-        return result;
+        try {
+            return { value: Buffer.from(value, this._flags.encoding) };
+        }
+        catch (ignoreErr) { }
     }
 
     _base(value, state, prefs) {

diff --git a/lib/types/boolean.js b/lib/types/boolean.js
@@ -24,6 +24,10 @@ internals.Boolean = class extends Any {
 
     _coerce(value, state, prefs) {
 
+        if (typeof value === 'boolean') {
+            return;
+        }
+
         if (typeof value === 'string') {
             const normalized = this._flags.insensitive ? value.toLowerCase() : value;
             value = normalized === 'true' ? true : (normalized === 'false' ? false : value);

diff --git a/lib/types/date.js b/lib/types/date.js
@@ -23,6 +23,10 @@ internals.Date = class extends Any {
 
     _coerce(value, state, prefs) {
 
+        if (value instanceof Date) {
+            return;
+        }
+
         return {
             value: internals.Date.toDate(value, this._flags.format, this._flags.timestamp, this._flags.multiplier) || value
         };

diff --git a/lib/types/number.js b/lib/types/number.js
@@ -27,12 +27,12 @@ internals.Number = class extends Any {
     _coerce(value, state, prefs) {
 
         if (typeof value !== 'string') {
-            return { value };
+            return;
         }
 
         const matches = value.match(internals.numberRx);
         if (!matches) {
-            return { value };
+            return;
         }
 
         value = value.trim();

diff --git a/lib/types/object.js b/lib/types/object.js
@@ -1,5 +1,6 @@
 'use strict';
 
+const Bourne = require('@hapi/bourne');
 const Hoek = require('@hapi/hoek');
 const Topo = require('@hapi/topo');
 
@@ -26,7 +27,7 @@ internals.Object = class extends Any {
         super();
         this._type = 'object';
         this._inner.children = null;
-        this._inner.renames = [];
+        this._inner.renames = null;
         this._inner.dependencies = [];
         this._inner.patterns = [];
     }
@@ -38,22 +39,20 @@ internals.Object = class extends Any {
 
     _coerce(value, state, prefs) {
 
-        if (!value ||
-            typeof value !== 'string') {
-
-            return { value };
+        if (typeof value !== 'string') {
+            return;
         }
 
-        if (value[0] === '{' ||
-            /^\s*\{/.test(value)) {
+        if (value[0] !== '{' &&
+            !/^\s*\{/.test(value)) {
 
-            try {
-                value = JSON.parse(value);
-            }
-            catch (ignoreErr) { }
+            return;
         }
 
-        return { value };
+        try {
+            return { value: Bourne.parse(value) };
+        }
+        catch (ignoreErr) { }
     }
 
     _base(value, state, prefs) {
@@ -68,7 +67,7 @@ internals.Object = class extends Any {
 
         // Skip if there are no other rules to test
 
-        if (!this._inner.renames.length &&
+        if (!this._inner.renames &&
             !this._inner.dependencies.length &&
             !this._inner.children &&                    // null allows any keys
             !this._inner.patterns.length) {
@@ -110,7 +109,9 @@ internals.Object = class extends Any {
 
         // Rename keys
 
-        if (!this._rename(target, state, prefs, errors)) {
+        if (this._inner.renames &&
+            !this._rename(target, state, prefs, errors)) {
+
             return finish();
         }
 
@@ -176,9 +177,7 @@ internals.Object = class extends Any {
                 const localState = this._state(key, [...state.path, key], [target]);
                 const item = target[key];
 
-                for (let i = 0; i < this._inner.patterns.length; ++i) {
-                    const pattern = this._inner.patterns[i];
-
+                for (const pattern of this._inner.patterns) {
                     if (pattern.regex ?
                         pattern.regex.test(key) :
                         pattern.schema._match(key, state, { ...prefs, abortEarly: true })) {
@@ -305,7 +304,7 @@ internals.Object = class extends Any {
             }
         }
 
-        if (this._inner.renames.length) {
+        if (this._inner.renames) {
             description.renames = Hoek.clone(this._inner.renames);
             for (const rename of description.renames) {
                 if (rename.from instanceof RegExp) {
@@ -491,6 +490,7 @@ internals.Object = class extends Any {
 
         Common.assertOptions(options, ['alias', 'ignoreUndefined', 'override', 'multiple']);
 
+        this._inner.renames = this._inner.renames || [];
         for (const rename of this._inner.renames) {
             Hoek.assert(rename.from !== from, 'Cannot rename the same key multiple times');
         }

diff --git a/lib/types/string/index.js b/lib/types/string/index.js
@@ -69,7 +69,7 @@ internals.String = class extends Any {
     _coerce(value, state, prefs) {
 
         if (typeof value !== 'string') {
-            return { value };
+            return;
         }
 
         const normalize = this._uniqueRules.get('normalize');

diff --git a/lib/validator.js b/lib/validator.js
@@ -43,11 +43,13 @@ exports.validate = function (value, schema, state, prefs, reference) {
         prefs.convert) {
 
         const coerced = schema._coerce(value, state, prefs);
-        if (coerced.errors) {
-            return internals.finalize(coerced.value, schema, original, [].concat(coerced.errors), state, prefs);                            // Coerced error always aborts early
-        }
+        if (coerced) {
+            if (coerced.errors) {
+                return internals.finalize(coerced.value, schema, original, [].concat(coerced.errors), state, prefs);                            // Coerced error always aborts early
+            }
 
-        value = coerced.value;
+            value = coerced.value;
+        }
     }
 
     // Empty value
@@ -61,28 +63,26 @@ exports.validate = function (value, schema, state, prefs, reference) {
     // Presence requirements (required, optional, forbidden)
 
     const presence = schema._flags.presence || prefs.presence;
-    if (presence === 'optional') {
-        if (value === undefined) {
-            if (schema._flags.default === Common.symbols.deepDefault &&
-                schema._type === 'object') {
+    if (value === undefined) {
+        if (presence === 'forbidden') {
+            return internals.finalize(value, schema, original, null, state, prefs);
+        }
+
+        if (presence === 'required') {
+            return internals.finalize(value, schema, original, [schema.createError('any.required', value, null, state, prefs)], state, prefs);
+        }
+
+        if (presence === 'optional') {
+            if (schema._type !== 'object' ||
+                schema._flags.default !== Common.symbols.deepDefault) {
 
-                value = {};
-            }
-            else {
                 return internals.finalize(value, schema, original, null, state, prefs);
             }
-        }
-    }
-    else if (presence === 'required' &&
-        value === undefined) {
 
-        return internals.finalize(value, schema, original, [schema.createError('any.required', value, null, state, prefs)], state, prefs);
+            value = {};
+        }
     }
     else if (presence === 'forbidden') {
-        if (value === undefined) {
-            return internals.finalize(value, schema, original, null, state, prefs);
-        }
-
         return internals.finalize(value, schema, original, [schema.createError('any.unknown', value, null, state, prefs)], state, prefs);
     }
 

diff --git a/package.json b/package.json
@@ -10,8 +10,9 @@
     "validation"
   ],
   "dependencies": {
-    "@hapi/hoek": "7.x.x",
     "@hapi/address": "2.x.x",
+    "@hapi/bourne": "1.x.x",
+    "@hapi/hoek": "7.x.x",
     "@hapi/marker": "1.x.x",
     "@hapi/topo": "3.x.x"
   },

diff --git a/test/types/array.js b/test/types/array.js
@@ -50,6 +50,17 @@ describe('array', () => {
         }]);
     });
 
+    it('errors on invalid array string', async () => {
+
+        const err = await expect(Joi.array().validate(' \n\r\t[ \n\r\t1 \n\r\t, \n\r\t2,3 \n\r\t')).to.reject('"value" must be an array');
+        expect(err.details).to.equal([{
+            message: '"value" must be an array',
+            path: [],
+            type: 'array.base',
+            context: { label: 'value', value: ' \n\r\t[ \n\r\t1 \n\r\t, \n\r\t2,3 \n\r\t' }
+        }]);
+    });
+
     it('errors on number', async () => {
 
         const err = await expect(Joi.array().validate(3)).to.reject('"value" must be an array');