[SEMVER-MAJOR] Fix object type conversion

3.0-RELEASE-NOTES.md

@@ -173,6 +173,7 @@ Most notable breaking changes:
    the value as a number now. That way the value "0" always produces
    "1970-01-01T00:00:00.000Z" instead of some date around 1999/2000/2001
    depending on server timezone.
+ - Array values are not allowed for arguments of type object.
 
 Hopefully this change should leave most LoopBack applications (and clients)
 unaffected. If your start seeing unusual amount of 400 error responses after

lib/looks-like-json.js

@@ -0,0 +1,27 @@
+// Copyright IBM Corp. 2016. All Rights Reserved.
+// Node module: strong-remoting
+// This file is licensed under the Artistic License 2.0.
+// License text available at https://opensource.org/licenses/Artistic-2.0
+
+'use strict';
+
+module.exports = {
+  looksLikeJson: looksLikeJson,
+  looksLikeJsonArray: looksLikeJsonArray,
+  looksLikeJsonObject: looksLikeJsonObject,
+};
+
+function looksLikeJson(value) {
+  return looksLikeJsonObject(value) || looksLikeJsonArray(value);
+}
+
+function looksLikeJsonArray(value) {
+  return typeof value === 'string' &&
+    value[0] === '[' && value[value.length - 1] === ']';
+}
+
+function looksLikeJsonObject(value) {
+  return typeof value === 'string' &&
+    value[0] === '{' && value[value.length - 1] === '}';
+}
+

lib/types/any.js

@@ -8,6 +8,7 @@
 var debug = require('debug')('strong-remoting:http-coercion');
 var g = require('strong-globalize')();
 var numberChecks = require('../number-checks');
+var looksLikeJson = require('../looks-like-json').looksLikeJson;
 
 var MAX_SAFE_INTEGER = numberChecks.MAX_SAFE_INTEGER;
 var MIN_SAFE_INTEGER = numberChecks.MIN_SAFE_INTEGER;
@@ -46,12 +47,18 @@ module.exports = {
         value = num;
     }
 
-    var objectConverter = ctx.typeRegistry.getConverter('object');
-    var objectResult = objectConverter.fromSloppyValue(ctx, value);
-    if (!objectResult.error && objectResult.value)
-      return objectResult;
+    if (looksLikeJson(value)) {
+      try {
+        var result = JSON.parse(value);
+        debug('parsed %j as JSON: %j', value, result);
+        return this.fromTypedValue(ctx, result);
+      } catch (ex) {
+        debug('Cannot parse "any" value %j, assuming string. %s', value, ex);
+        // no-op, use the original string value
+      }
+    }
 
-    return { value: value };
+    return this.fromTypedValue(ctx, value);
   },
 
   validate: function(ctx, value) {

lib/types/array.js

@@ -9,6 +9,7 @@ var assert = require('assert');
 var debug = require('debug')('strong-remoting:http-coercion');
 var escapeRegex = require('escape-string-regexp');
 var g = require('strong-globalize')();
+var looksLikeJsonArray = require('../looks-like-json').looksLikeJsonArray;
 
 module.exports = ArrayConverter;
 
@@ -56,10 +57,7 @@ ArrayConverter.prototype.fromSloppyValue = function(ctx, value) {
 };
 
 ArrayConverter.prototype._fromTypedValueString = function(ctx, value) {
-  var looksLikeJsonArray = typeof value === 'string' &&
-    value[0] === '[' && value[value.length - 1] === ']';
-
-  if (!looksLikeJsonArray)
+  if (!looksLikeJsonArray(value))
     return null;
 
   // If it looks like a JSON array, try to parse it.

lib/types/object.js

@@ -7,6 +7,7 @@
 
 var debug = require('debug')('strong-remoting:http-coercion');
 var g = require('strong-globalize')();
+var looksLikeJsonObject = require('../looks-like-json').looksLikeJsonObject;
 
 module.exports = {
   fromTypedValue: function(ctx, value) {
@@ -23,11 +24,7 @@ module.exports = {
     if (value === null || value === 'null')
       return { value: null };
 
-    var looksLikeJson = typeof value === 'string' && (
-      (value[0] === '[' && value[value.length - 1] === ']') ||
-      (value[0] === '{' && value[value.length - 1] === '}'));
-
-    if (looksLikeJson) {
+    if (looksLikeJsonObject(value)) {
       try {
         var result = JSON.parse(value);
         debug('parsed %j as JSON: %j', value, result);
@@ -45,11 +42,26 @@ module.exports = {
   },
 
   validate: function(ctx, value) {
-    if (value === undefined || typeof value === 'object')
+    if (value === undefined || value === null)
       return null;
 
-    var err = new Error(g.f('Value is not an object.'));
-    err.statusCode = 400;
-    return err;
+    if (typeof value !== 'object')
+      return errorNotAnObject();
+
+    // reject object-like values that have their own strong-remoting type
+
+    if (Array.isArray(value))
+      return errorNotAnObject();
+
+    if (value instanceof Date)
+      return errorNotAnObject();
+
+    return null;
   },
 };
+
+function errorNotAnObject() {
+  var err = new Error(g.f('Value is not an object.'));
+  err.statusCode = 400;
+  return err;
+}

test/rest-coercion.test.js

@@ -16,7 +16,7 @@ var RemoteObjects = require('..');
 
 describe('Coercion in RestAdapter', function() {
   var ctx = {
-    remoteObject: null,
+    remoteObjects: null,
     request: null,
     ERROR_BAD_REQUEST: new Error(400),
     prettyExpectation: prettyExpectation,
@@ -107,10 +107,24 @@ describe('Coercion in RestAdapter', function() {
       { value: actualValue } :
       { error: res.statusCode };
 
+    var actualCtor = actual.value && typeof actual.value === 'object' &&
+        actual.value.constructor;
+    if (actualCtor && actualCtor !== Object && actualCtor.name) {
+      actual = {};
+      actual[actualCtor.name] = actualValue;
+    }
+
     var expected = expectedResult instanceof Error ?
       { error: +expectedResult.message } :
       { value: expectedResult };
 
+    var expectedCtor = expected.value && typeof expected.value === 'object' &&
+        expected.value.constructor;
+    if (expectedCtor && expectedCtor !== Object && expectedCtor.name) {
+      expected = {};
+      expected[expectedCtor.name] = expectedResult;
+    }
+
     var suiteName = ctx.runtime.currentSuiteName;
     var input = ctx.runtime.currentInput;
     if (suiteName && input) {

test/rest-coercion/_custom-class.context.js

@@ -0,0 +1,51 @@
+// Copyright IBM Corp. 2014,2016. All Rights Reserved.
+// Node module: strong-remoting
+// This file is licensed under the Artistic License 2.0.
+// License text available at https://opensource.org/licenses/Artistic-2.0
+
+'use strict';
+
+var extend = require('util')._extend;
+
+function CustomClass(data) {
+  if (!(this instanceof CustomClass))
+    return new CustomClass(data);
+
+  if (data.invalid) {
+    var err = new Error('Invalid CustomClass value.');
+    err.statusCode = 400;
+    throw err;
+  }
+
+  if ('name' in data)
+    this.name = data.name;
+  else
+    this.empty = true;
+};
+
+module.exports = function createCustomClassContext(ctx) {
+  beforeEach(function registerCustomClass() {
+    if ('customclass' in ctx.remoteObjects._typeRegistry._types) {
+      // This happens when there are multiple instances of this beforEach hook
+      // registered. Typically when createCustomClassContext is called
+      // inside the top-level "describe" block.
+      return;
+    }
+    ctx.remoteObjects.defineObjectType('CustomClass', CustomClass);
+  });
+
+  return extend(Object.create(ctx), {
+    CustomClass: CustomClass,
+    verifyTestCases: verifyTestCases,
+  });
+
+  function verifyTestCases(argSpec, testCases) {
+    for (var ix in testCases) {
+      if (testCases[ix].length === 1) {
+        var data = testCases[ix][0];
+        testCases[ix] = [data, new CustomClass(data)];
+      }
+    }
+    ctx.verifyTestCases(argSpec, testCases);
+  }
+};

test/rest-coercion/_jsonbody.context.js

@@ -12,9 +12,9 @@ var format = util.format;
 var extend = util._extend;
 
 module.exports = function createJsonBodyContext(ctx) {
-  return extend({
+  return extend(Object.create(ctx), {
     verifyTestCases: verifyTestCases,
-  }, ctx);
+  });
 
   /**
    * Verify a set of test-cases for a given argument specification

test/rest-coercion/_jsonform.context.js

@@ -14,11 +14,11 @@ var extend = util._extend;
 var EMPTY_BODY = {};
 
 module.exports = function createJsonBodyContext(ctx) {
-  return extend({
+  return extend(Object.create(ctx), {
     /** Send a request with an empty body (that is still valid JSON) */
     EMPTY_BODY: EMPTY_BODY,
     verifyTestCases: verifyTestCases,
-  }, ctx);
+  });
 
   /**
    * Verify a set of test-cases for a given argument specification

test/rest-coercion/_urlencoded.context.js

@@ -16,11 +16,11 @@ var EMPTY_QUERY = '';
 module.exports = function createUrlEncodedContext(ctx, target) {
   var TARGET_QUERY_STRING = target === 'qs';
 
-  return extend({
+  return extend(Object.create(ctx), {
     /** Send empty data, i.e. empty request body or no query string */
     EMPTY_QUERY: EMPTY_QUERY,
     verifyTestCases: verifyTestCases,
-  }, ctx);
+  });
 
   /**
    * Verify a set of test-cases for a given argument specification

test/rest-coercion/jsonbody-object-type.suite.js

@@ -0,0 +1,72 @@
+// Copyright IBM Corp. 2014,2016. All Rights Reserved.
+// Node module: strong-remoting
+// This file is licensed under the Artistic License 2.0.
+// License text available at https://opensource.org/licenses/Artistic-2.0
+
+'use strict';
+
+var jsonBodyContext = require('./_jsonbody.context');
+var customClassContext = require('./_custom-class.context.js');
+
+module.exports = function(ctx) {
+  ctx = customClassContext(jsonBodyContext(ctx));
+  var ERROR_BAD_REQUEST = ctx.ERROR_BAD_REQUEST;
+  var CustomClass = ctx.CustomClass;
+  var verifyTestCases = ctx.verifyTestCases;
+
+  describe('json body - CustomClass - required', function() {
+    // See verifyTestCases' jsdoc for details about the format of test cases.
+    verifyTestCases({ arg: 'anyname', type: 'CustomClass', required: true }, [
+      // An empty object is a valid value
+      [{}],
+
+      [{ name: '' }],
+      [{ name: 'a-test-name' }],
+
+      // Invalid values trigger ERROR_BAD_REQUEST
+      [null, ERROR_BAD_REQUEST],
+      [{ invalid: true }, ERROR_BAD_REQUEST],
+
+      // Array values are not allowed
+      [[], ERROR_BAD_REQUEST],
+      [[1, 2], ERROR_BAD_REQUEST],
+    ]);
+  });
+
+  describe('json body - CustomClass - optional', function() {
+    // See verifyTestCases' jsdoc for details about the format of test cases.
+    verifyTestCases({ arg: 'anyname', type: 'CustomClass' }, [
+      // Empty values
+      [null, null],
+
+      // Valid values
+      [{}],
+      [{ name: 'a-test-name' }],
+
+      // Verify that deep coercion is not triggered
+      // and types specified in JSON are preserved
+
+      [{ name: '' }],
+      [{ name: null }],
+      [{ name: {}}],
+      [{ name: { key: null }}],
+      [{ name: 1 }],
+      [{ name: '1' }],
+      [{ name: -1 }],
+      [{ name: '-1' }],
+      [{ name: 1.2 }],
+      [{ name: '1.2' }],
+      [{ name: -1.2 }],
+      [{ name: '-1.2' }],
+      [{ name: ['tenamet'] }],
+      [{ name: [1, 2] }],
+
+      // Invalid values - arrays are rejected
+      [[], ERROR_BAD_REQUEST],
+      [[1, 2], ERROR_BAD_REQUEST],
+
+      // Verify that errors thrown by the factory function are handled
+      [{ invalid: true }, ERROR_BAD_REQUEST],
+    ]);
+  });
+};

test/rest-coercion/jsonbody-object.suite.js

@@ -16,14 +16,16 @@ module.exports = function(ctx) {
     // See verifyTestCases' jsdoc for details about the format of test cases.
     verifyTestCases({ arg: 'anyname', type: 'object', required: true }, [
       // Valid values, arrays are objects too
-      [[]], // an empty array is a valid value
       [{}], // an empty object is a valid value too
       [{ x: '' }],
       [{ x: null }],
-      [[1, 2]],
 
       // Invalid values trigger ERROR_BAD_REQUEST
       [null, ERROR_BAD_REQUEST],
+
+      // Arrays are not allowed
+      [[], ERROR_BAD_REQUEST],
+      [[1, 2], ERROR_BAD_REQUEST],
     ]);
   });
 
@@ -33,8 +35,7 @@ module.exports = function(ctx) {
       // Empty values
       [null, null],
 
-      // Valid values, arrays are objects too
-      [[]],
+      // Valid values
       [{}],
 
       // Verify that deep coercion is not triggered
@@ -68,6 +69,10 @@ module.exports = function(ctx) {
       [{ x: '2016-05-19T13:28:51.299Z' }],
       [{ x: '2016-05-19' }],
       [{ x: 'Thu May 19 2016 15:28:51 GMT 0200 (CEST)' }],
+
+      // Arrays are not allowed
+      [[], ERROR_BAD_REQUEST],
+      [[1, 2], ERROR_BAD_REQUEST],
     ]);
   });
 };