[Extensions Bindings] Allow native custom signature parsing

A few APIs validate arguments against a signature specified in an event
call, rather than as an API method. Allow this validation to happen
natively when native bindings are enabled by adding a JS utility to add
a custom signature and validate against it.

Use this for certificateProvider, printerProvider, and webRequest custom
bindings instead of the JS validation.

Add unittests for the new JS utility method.

Bug: 810487

Change-Id: Ifbc8480ae15acbdaa9ecdffcfb7bfe159e201510
Reviewed-on: https://chromium-review.googlesource.com/914808Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Commit-Queue: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#545806}

chrome/renderer/resources/extensions/certificate_provider_custom_bindings.js

@@ -16,7 +16,16 @@ var registerArgumentMassager = bindingUtil ?
 var certificateProviderSchema =
     requireNative('schema_registry').GetSchema('certificateProvider')
 var utils = require('utils');
-var validate = require('schemaUtils').validate;
+var validate = bindingUtil ? undefined : require('schemaUtils').validate;
+
+// Validates that the result passed by the extension to the event callback
+// matches the callback schema. Throws an exception in case of an error.
+function validateListenerResponse(eventName, expectedSchema, listenerResponse) {
+  if (bindingUtil)
+    bindingUtil.validateCustomSignature(eventName, listenerResponse);
+  else
+    validate(listenerResponse, expectedSchema);
+}
 
 // Custom bindings for chrome.certificateProvider API.
 // The bindings are used to implement callbacks for the API events. Internally
@@ -41,10 +50,14 @@ var validate = require('schemaUtils').validate;
 function handleEvent(eventName, internalReportFunc) {
   var eventSchema =
       utils.lookup(certificateProviderSchema.events, 'name', eventName);
-  var callbackSchema = utils.lookup(eventSchema.parameters, 'type', 'function');
+  var callbackSchema =
+      utils.lookup(eventSchema.parameters, 'type', 'function').parameters;
+  var fullEventName = 'certificateProvider.' + eventName;
+
+  if (bindingUtil)
+    bindingUtil.addCustomSignature(fullEventName, callbackSchema);
 
-  registerArgumentMassager('certificateProvider.' + eventName,
-                           function(args, dispatch) {
+  registerArgumentMassager(fullEventName, function(args, dispatch) {
     var responded = false;
 
     // Function provided to the extension as the event callback argument.
@@ -63,7 +76,7 @@ function handleEvent(eventName, internalReportFunc) {
         // Validates that the results reported by the extension matche the
         // callback schema of the event. Throws an exception in case of an
         // error.
-        validate(reportArgs, callbackSchema.parameters);
+        validateListenerResponse(fullEventName, callbackSchema, reportArgs);
         finalArgs = reportArgs;
       } finally {
         responded = true;

chrome/test/data/extensions/api_test/webrequest/test_api.js

@@ -60,6 +60,12 @@ chrome.test.runTests([
         function(details) {},
         goodFilter, goodExtraInfo);
 
+    function isArgumentParseError(error) {
+      // Native and JS bindings have slightly different errors surfaced.
+      return error.search('Invalid value') >= 0 ||
+             error.search('Error at parameter') >= 0;
+    }
+
     // Try a bad RequestFilter.
     try {
       chrome.webRequest.onBeforeRequest.addListener(
@@ -67,7 +73,7 @@ chrome.test.runTests([
           {badFilter: 42, urls: ["<all_urls>"]}, goodExtraInfo);
       chrome.test.fail();
     } catch (e) {
-      chrome.test.assertTrue(e.message.search("Invalid value") >= 0);
+      chrome.test.assertTrue(isArgumentParseError(e.message), e.message);
     }
 
     // Try a bad ExtraInfoSpec.
@@ -77,7 +83,7 @@ chrome.test.runTests([
           goodFilter, ["badExtraInfo"]);
       chrome.test.fail();
     } catch (e) {
-      chrome.test.assertTrue(e.message.search("Invalid value") >= 0);
+      chrome.test.assertTrue(isArgumentParseError(e.message), e.message);
     }
 
     // This extraInfoSpec should only work for onBeforeSendHeaders.
@@ -91,7 +97,7 @@ chrome.test.runTests([
           goodFilter, headersExtraInfo);
       chrome.test.fail();
     } catch (e) {
-      chrome.test.assertTrue(e.message.search("Invalid value") >= 0);
+      chrome.test.assertTrue(isArgumentParseError(e.message), e.message);
     }
 
     // ExtraInfoSpec with "responseHeaders" should work for onCompleted.
@@ -105,7 +111,7 @@ chrome.test.runTests([
           goodFilter, headersExtraInfo);
       chrome.test.fail();
     } catch (e) {
-      chrome.test.assertTrue(e.message.search("Invalid value") >= 0);
+      chrome.test.assertTrue(isArgumentParseError(e.message), e.message);
     }
 
     // Try a bad URL pattern. The error happens asynchronously. We're just

extensions/renderer/bindings/api_binding_js_util.cc

@@ -6,6 +6,7 @@
 
 #include "base/strings/stringprintf.h"
 #include "base/values.h"
+#include "content/public/renderer/v8_value_converter.h"
 #include "extensions/renderer/bindings/api_event_handler.h"
 #include "extensions/renderer/bindings/api_request_handler.h"
 #include "extensions/renderer/bindings/api_signature.h"
@@ -51,7 +52,10 @@ gin::ObjectTemplateBuilder APIBindingJSUtil::GetObjectTemplateBuilder(
                  &APIBindingJSUtil::RunCallbackWithLastError)
       .SetMethod("handleException", &APIBindingJSUtil::HandleException)
       .SetMethod("setExceptionHandler", &APIBindingJSUtil::SetExceptionHandler)
-      .SetMethod("validateType", &APIBindingJSUtil::ValidateType);
+      .SetMethod("validateType", &APIBindingJSUtil::ValidateType)
+      .SetMethod("validateCustomSignature",
+                 &APIBindingJSUtil::ValidateCustomSignature)
+      .SetMethod("addCustomSignature", &APIBindingJSUtil::AddCustomSignature);
 }
 
 void APIBindingJSUtil::SendRequest(
@@ -269,4 +273,62 @@ void APIBindingJSUtil::ValidateType(gin::Arguments* arguments,
   }
 }
 
+void APIBindingJSUtil::AddCustomSignature(
+    gin::Arguments* arguments,
+    const std::string& custom_signature_name,
+    v8::Local<v8::Value> signature) {
+  v8::Local<v8::Context> context = arguments->GetHolderCreationContext();
+
+  // JS bindings run per-context, so it's expected that they will call this
+  // multiple times in the lifetime of the renderer process. Only handle the
+  // first call.
+  if (type_refs_->GetCustomSignature(custom_signature_name))
+    return;
+
+  if (!signature->IsArray()) {
+    NOTREACHED();
+    return;
+  }
+
+  std::unique_ptr<base::Value> base_signature =
+      content::V8ValueConverter::Create()->FromV8Value(signature, context);
+  if (!base_signature->is_list()) {
+    NOTREACHED();
+    return;
+  }
+
+  type_refs_->AddCustomSignature(
+      custom_signature_name,
+      std::make_unique<APISignature>(
+          *base::ListValue::From(std::move(base_signature))));
+}
+
+void APIBindingJSUtil::ValidateCustomSignature(
+    gin::Arguments* arguments,
+    const std::string& custom_signature_name,
+    v8::Local<v8::Value> arguments_to_validate) {
+  v8::Isolate* isolate = arguments->isolate();
+  v8::HandleScope handle_scope(isolate);
+  v8::Local<v8::Context> context = arguments->GetHolderCreationContext();
+
+  const APISignature* signature =
+      type_refs_->GetCustomSignature(custom_signature_name);
+  if (!signature) {
+    NOTREACHED();
+  }
+
+  std::vector<v8::Local<v8::Value>> vector_arguments;
+  if (!gin::ConvertFromV8(isolate, arguments_to_validate, &vector_arguments)) {
+    NOTREACHED();
+    return;
+  }
+
+  std::vector<v8::Local<v8::Value>> args_out;
+  std::string parse_error;
+  if (!signature->ParseArgumentsToV8(context, vector_arguments, *type_refs_,
+                                     &args_out, &parse_error)) {
+    arguments->ThrowTypeError(parse_error);
+  }
+}
+
 }  // namespace extensions

extensions/renderer/bindings/api_binding_js_util.h

@@ -107,6 +107,21 @@ class APIBindingJSUtil final : public gin::Wrappable<APIBindingJSUtil> {
                     const std::string& type_name,
                     v8::Local<v8::Value> value);
 
+  // Allows custom bindings to add a signature with the given
+  // |custom_signature_name| to use later in argument validation. The signature
+  // is expected to be an array of expected types, that can be passed to
+  // construct an APISignature.
+  void AddCustomSignature(gin::Arguments* arguments,
+                          const std::string& custom_signature_name,
+                          v8::Local<v8::Value> signature);
+
+  // Looks up the signature with the given |custom_signature_name| and validates
+  // |arguments_to_validate| against it, throwing an error if the arguments
+  // don't match.
+  void ValidateCustomSignature(gin::Arguments* arguments,
+                               const std::string& custom_signature_name,
+                               v8::Local<v8::Value> arguments_to_validate);
+
   // Type references. Guaranteed to outlive this object.
   APITypeReferenceMap* const type_refs_;
 

extensions/renderer/bindings/api_binding_js_util_unittest.cc

@@ -329,4 +329,65 @@ TEST_F(APIBindingJSUtilUnittest, TestValidateType) {
       expected_error);
 }
 
+TEST_F(APIBindingJSUtilUnittest, TestValidateCustomSignature) {
+  v8::HandleScope handle_scope(isolate());
+  v8::Local<v8::Context> context = MainContext();
+
+  gin::Handle<APIBindingJSUtil> util = CreateUtil();
+  v8::Local<v8::Object> v8_util = util.ToV8().As<v8::Object>();
+
+  constexpr char kSignatureName[] = "custom_signature";
+  EXPECT_FALSE(bindings_system()->type_reference_map()->GetCustomSignature(
+      kSignatureName));
+
+  {
+    constexpr char kAddSignature[] =
+        R"((function(util) {
+             util.addCustomSignature(
+                 'custom_signature',
+                 [{type: 'integer'}, {'type': 'string'}]);
+           }))";
+    v8::Local<v8::Function> add_signature =
+        FunctionFromString(context, kAddSignature);
+    v8::Local<v8::Value> args[] = {v8_util};
+    RunFunction(add_signature, context, arraysize(args), args);
+  }
+
+  EXPECT_TRUE(bindings_system()->type_reference_map()->GetCustomSignature(
+      kSignatureName));
+
+  auto call_validate_signature =
+      [context, v8_util](const char* function,
+                         base::Optional<std::string> expected_error) {
+        v8::Local<v8::Function> v8_function =
+            FunctionFromString(context, function);
+        v8::Local<v8::Value> args[] = {v8_util};
+        if (expected_error) {
+          RunFunctionAndExpectError(v8_function, context, arraysize(args), args,
+                                    *expected_error);
+        } else {
+          RunFunction(v8_function, context, arraysize(args), args);
+        }
+      };
+
+  // Test a case that should succeed (a valid value).
+  call_validate_signature(
+      R"((function(util) {
+           util.validateCustomSignature('custom_signature', [1, 'foo']);
+         }))",
+      base::nullopt);
+
+  // Test a failing case (prop1 is supposed to be a string).
+  std::string expected_error =
+      "Uncaught TypeError: " +
+      api_errors::ArgumentError(
+          "", api_errors::InvalidType(api_errors::kTypeString,
+                                      api_errors::kTypeInteger));
+  call_validate_signature(
+      R"((function(util) {
+           util.validateCustomSignature('custom_signature', [1, 2]);
+         }))",
+      expected_error);
+}
+
 }  // namespace extensions

extensions/renderer/bindings/api_type_reference_map.cc

@@ -76,4 +76,18 @@ bool APITypeReferenceMap::HasTypeMethodSignature(
   return type_methods_.find(name) != type_methods_.end();
 }
 
+void APITypeReferenceMap::AddCustomSignature(
+    const std::string& name,
+    std::unique_ptr<APISignature> signature) {
+  DCHECK(custom_signatures_.find(name) == custom_signatures_.end())
+      << "Cannot re-register signature for: " << name;
+  custom_signatures_[name] = std::move(signature);
+}
+
+const APISignature* APITypeReferenceMap::GetCustomSignature(
+    const std::string& name) const {
+  auto iter = custom_signatures_.find(name);
+  return iter != custom_signatures_.end() ? iter->second.get() : nullptr;
+}
+
 }  // namespace extensions

extensions/renderer/bindings/api_type_reference_map.h

@@ -57,6 +57,13 @@ class APITypeReferenceMap {
   // an API.
   bool HasTypeMethodSignature(const std::string& name) const;
 
+  // Adds a custom signature for bindings to use.
+  void AddCustomSignature(const std::string& name,
+                          std::unique_ptr<APISignature> signature);
+
+  // Looks up a custom signature that was previously added.
+  const APISignature* GetCustomSignature(const std::string& name) const;
+
   bool empty() const { return type_refs_.empty(); }
   size_t size() const { return type_refs_.size(); }
 
@@ -66,6 +73,7 @@ class APITypeReferenceMap {
   std::map<std::string, std::unique_ptr<ArgumentSpec>> type_refs_;
   std::map<std::string, std::unique_ptr<APISignature>> api_methods_;
   std::map<std::string, std::unique_ptr<APISignature>> type_methods_;
+  std::map<std::string, std::unique_ptr<APISignature>> custom_signatures_;
 
   DISALLOW_COPY_AND_ASSIGN(APITypeReferenceMap);
 };

extensions/renderer/resources/printer_provider_custom_bindings.js

@@ -14,8 +14,18 @@ var blobNatives = requireNative('blob_natives');
 
 var printerProviderSchema =
     requireNative('schema_registry').GetSchema('printerProvider')
+
 var utils = require('utils');
-var validate = require('schemaUtils').validate;
+var validate = bindingUtil ? undefined : require('schemaUtils').validate;
+
+// Validates that the result passed by the extension to the event callback
+// matches the callback schema. Throws an exception in case of an error.
+function validateListenerResponse(eventName, expectedSchema, listenerResponse) {
+  if (bindingUtil)
+    bindingUtil.validateCustomSignature(eventName, listenerResponse);
+  else
+    validate(listenerResponse, expectedSchema);
+}
 
 // Custom bindings for chrome.printerProvider API.
 // The bindings are used to implement callbacks for the API events. Internally
@@ -43,21 +53,17 @@ var validate = require('schemaUtils').validate;
 // |resultreporter|: The function that should be called to report event result.
 //     One of chrome.printerProviderInternal API functions.
 function handleEvent(eventName, prepareArgsForDispatch, resultReporter) {
-  registerArgumentMassager('printerProvider.' + eventName,
-                           function(args, dispatch) {
-    var responded = false;
+  var eventSchema =
+      utils.lookup(printerProviderSchema.events, 'name', eventName);
+  var callbackSchema =
+      utils.lookup(eventSchema.parameters, 'type', 'function').parameters;
+  var fullEventName = 'printerProvider.' + eventName;
 
-    // Validates that the result passed by the extension to the event
-    // callback matches the callback schema. Throws an exception in case of
-    // an error.
-    var validateResult = function(result) {
-      var eventSchema =
-          utils.lookup(printerProviderSchema.events, 'name', eventName);
-      var callbackSchema =
-          utils.lookup(eventSchema.parameters, 'type', 'function');
+  if (bindingUtil)
+    bindingUtil.addCustomSignature(fullEventName, callbackSchema);
 
-      validate([result], callbackSchema.parameters);
-    };
+  registerArgumentMassager(fullEventName, function(args, dispatch) {
+    var responded = false;
 
     // Function provided to the extension as the event callback argument.
     // It makes sure that the event result hasn't previously been returned
@@ -69,7 +75,8 @@ function handleEvent(eventName, prepareArgsForDispatch, resultReporter) {
 
       var finalResult = null;
       try {
-        validateResult(result);  // throws on failure
+        // throws on failure
+        validateListenerResponse(fullEventName, callbackSchema, [result]);
         finalResult = result;
       } finally {
         responded = true;
@@ -121,4 +128,5 @@ handleEvent('onGetUsbPrinterInfoRequested',
             function(args, callback) { callback(true); },
             printerProviderInternal.reportUsbPrinterInfo);
 
-exports.$set('binding', binding.generate());
+if (!apiBridge)
+  exports.$set('binding', binding.generate());

extensions/renderer/resources/web_request_event.js

@@ -6,11 +6,19 @@ var CHECK = requireNative('logging').CHECK;
 var eventBindings = bindingUtil ? undefined : require('event_bindings');
 var idGeneratorNatives = requireNative('id_generator');
 var utils = require('utils');
-var validate = require('schemaUtils').validate;
+var validate = bindingUtil ? undefined : require('schemaUtils').validate;
 var webRequestInternal = getInternalApi ?
     getInternalApi('webRequestInternal') :
     require('binding').Binding.create('webRequestInternal').generate();
 
+function validateListenerArguments(
+    eventName, extraArgSchemas, listenerArguments) {
+  if (bindingUtil)
+    bindingUtil.validateCustomSignature(eventName, listenerArguments);
+  else
+    validate(listenerArguments, extraArgSchemas);
+}
+
 function getUniqueSubEventName(eventName) {
   return eventName + '/' + idGeneratorNatives.GetNextId();
 }
@@ -41,6 +49,9 @@ function WebRequestEventImpl(eventName, opt_argSchemas, opt_extraArgSchemas,
   if (typeof eventName != 'string')
     throw new Error('chrome.WebRequestEvent requires an event name.');
 
+  if (bindingUtil)
+    bindingUtil.addCustomSignature(eventName, opt_extraArgSchemas);
+
   this.eventName = eventName;
   this.argSchemas = opt_argSchemas;
   this.extraArgSchemas = opt_extraArgSchemas;
@@ -77,7 +88,8 @@ WebRequestEventImpl.prototype.addListener =
   var subEventName = getUniqueSubEventName(this.eventName);
   // Note: this could fail to validate, in which case we would not add the
   // subEvent listener.
-  validate($Array.slice(arguments, 1), this.extraArgSchemas);
+  validateListenerArguments(this.eventName, this.extraArgSchemas,
+                            $Array.slice(arguments, 1));
   webRequestInternal.addEventListener(
       cb, opt_filter, opt_extraInfo, this.eventName, subEventName,
       this.webViewInstanceId);

extensions/test/data/api_test/printer_provider/request_capability/test.js

@@ -28,11 +28,15 @@ chrome.test.sendMessage('loaded', function(test) {
           }
 
           if (test == 'INVALID_VALUE') {
-            chrome.test.assertThrows(
-                callback,
-                ['XXX'],
-                'Invalid value for argument 1. '+
-                'Expected \'object\' but got \'string\'.');
+            var jsBindingsError =
+                'Invalid value for argument 1. Expected \'object\'' +
+                ' but got \'string\'.';
+            var nativeBindingsError =
+                'Error at parameter \'capabilities\': ' +
+                'Invalid type: expected object, found string.';
+            var regexp =
+                new RegExp(nativeBindingsError + '|' + jsBindingsError);
+            chrome.test.assertThrows(callback, ['XXX'], regexp);
           } else if (test == 'EMPTY') {
             callback({});
           } else {

extensions/test/data/api_test/printer_provider/request_print/test.js

@@ -45,12 +45,16 @@ chrome.test.sendMessage('loaded', function(test) {
           setTimeout(callback.bind(null, 'OK'), 0);
           break;
         case 'INVALID_VALUE':
-          chrome.test.assertThrows(
-              callback,
-              ['XXX'],
+          var jsBindingsError =
               'Invalid value for argument 1. ' +
               'Value must be one of: ' +
-              '[OK, FAILED, INVALID_TICKET, INVALID_DATA].');
+              '\\[OK, FAILED, INVALID_TICKET, INVALID_DATA\\].';
+          var nativeBindingsError =
+              'Error at parameter \'result\': Value must be one of ' +
+              'FAILED, INVALID_DATA, INVALID_TICKET, OK.';
+          chrome.test.assertThrows(
+              callback, ['XXX'],
+              new RegExp(jsBindingsError + '|' + nativeBindingsError));
           break;
         case 'FAILED':
         case 'INVALID_TICKET':

extensions/test/data/api_test/printer_provider/request_printers/test.js

@@ -2,6 +2,10 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+function eitherError(error1, error2) {
+  return new RegExp(error1 + '|' + error2);
+}
+
 chrome.test.sendMessage('loaded', function(test) {
   chrome.test.runTests([function printTest() {
     if (test == 'NO_LISTENER') {
@@ -31,12 +35,23 @@ chrome.test.sendMessage('loaded', function(test) {
           }
 
           if (test == 'NOT_ARRAY') {
-            chrome.test.assertThrows(
-                callback,
-                ['XXX'],
+            var jsBindingsError =
                 'Invalid value for argument 1. ' +
-                'Expected \'array\' but got \'string\'.');
+                'Expected \'array\' but got \'string\'.'
+            var nativeBindingsError =
+                'Error at parameter \'printerInfo\': ' +
+                'Invalid type: expected array, found string.'
+            chrome.test.assertThrows(
+                callback, ['XXX'],
+                eitherError(jsBindingsError, nativeBindingsError));
           } else if (test == 'INVALID_PRINTER_TYPE') {
+            var jsBindingsError =
+                'Invalid value for argument 1. ' +
+                'Property \'.1\': Expected \'object\' but got \'string\'.';
+            var nativeBindingsError =
+                'Error at parameter \'printerInfo\': Error at index 1: ' +
+                'Invalid type: expected printerProvider.PrinterInfo, ' +
+                'found string.';
             chrome.test.assertThrows(
                 callback,
                 [[{
@@ -44,9 +59,14 @@ chrome.test.sendMessage('loaded', function(test) {
                   name: 'Printer 1',
                   description: 'Test printer'
                 }, 'printer2']],
-                'Invalid value for argument 1. ' +
-                'Property \'.1\': Expected \'object\' but got \'string\'.');
+                eitherError(jsBindingsError, nativeBindingsError));
           } else if (test == 'INVALID_PRINTER') {
+            var jsBindingsError =
+                'Invalid value for argument 1. ' +
+                'Property \'.0.unsupported\': Unexpected property.';
+            var nativeBindingsError =
+                'Error at parameter \'printerInfo\': ' +
+                'Error at index 0: Unexpected property: \'unsupported\'.';
             chrome.test.assertThrows(
                 callback,
                 [[{
@@ -55,8 +75,7 @@ chrome.test.sendMessage('loaded', function(test) {
                   description: 'Test printer',
                   unsupported: 'print'
                 }]],
-                'Invalid value for argument 1. ' +
-                'Property \'.0.unsupported\': Unexpected property.');
+                eitherError(jsBindingsError, nativeBindingsError));
           } else {
             chrome.test.assertEq('OK', test);
             callback([{