Add JS bindings validation logic for non-nullable types

mojo/public/js/bindings/validator.js

 // Copyright 2014 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 define("mojo/public/js/bindings/validator", [
   "mojo/public/js/bindings/codec",
-], function(codec) {
+  ], function(codec) {

[yzshen1, 2014/08/20 17:04:20]

I think the indent is a little unusual.
Maybe the previous is correct?

[hansmuller, 2014/08/20 17:46:30]

Done.

 
   var validationError = {
     NONE: 'VALIDATION_ERROR_NONE',
     MISALIGNED_OBJECT: 'VALIDATION_ERROR_MISALIGNED_OBJECT',
     ILLEGAL_MEMORY_RANGE: 'VALIDATION_ERROR_ILLEGAL_MEMORY_RANGE',
     UNEXPECTED_STRUCT_HEADER: 'VALIDATION_ERROR_UNEXPECTED_STRUCT_HEADER',
     UNEXPECTED_ARRAY_HEADER: 'VALIDATION_ERROR_UNEXPECTED_ARRAY_HEADER',
     ILLEGAL_HANDLE: 'VALIDATION_ERROR_ILLEGAL_HANDLE',
+    UNEXPECTED_INVALID_HANDLE: 'VALIDATION_ERROR_UNEXPECTED_INVALID_HANDLE',
     ILLEGAL_POINTER: 'VALIDATION_ERROR_ILLEGAL_POINTER',
+    UNEXPECTED_NULL_POINTER: 'VALIDATION_ERROR_UNEXPECTED_NULL_POINTER',
     MESSAGE_HEADER_INVALID_FLAG_COMBINATION:
         'VALIDATION_ERROR_MESSAGE_HEADER_INVALID_FLAG_COMBINATION',
     MESSAGE_HEADER_MISSING_REQUEST_ID:
         'VALIDATION_ERROR_MESSAGE_HEADER_MISSING_REQUEST_ID'
   };
 
   var NULL_MOJO_POINTER = "NULL_MOJO_POINTER";
 
+  function isStringClass(cls) {
+    return cls === codec.String || cls === codec.NullableString;
+  }
+
+  function isHandleClass(cls) {
+    return cls === codec.Handle || cls === codec.NullableHandle;
+  }
+
+  function isNullable(type) {
+    return type === codec.NullableString || type === codec.NullableHandle ||
+      type instanceof codec.NullableArrayOf ||

[yzshen1, 2014/08/20 17:04:20]

wrong indent. (37 - 39)

[hansmuller, 2014/08/20 17:46:31]

Done.

+      type instanceof codec.NullablePointerTo;
+}
+
   function Validator(message) {
     this.message = message;
     this.offset = 0;
     this.handleIndex = 0;
   }
 
   Object.defineProperty(Validator.prototype, "offsetLimit", {
     get: function() { return this.message.buffer.byteLength; }
   });
 
@@ skipping 31 matching lines @@
       return true;
 
     if (index < this.handleIndex || index >= this.handleIndexLimit)
       return false;
 
     // This is safe because handle indices are uint32.
     this.handleIndex = index + 1;
     return true;
   }
 
-  Validator.prototype.validateHandle = function(offset) {
+  Validator.prototype.validateHandle = function(offset, nullable) {

[yzshen1, 2014/08/20 17:04:20]

One thing may worth considering:
Passing nullable at runtime may be unnecessary: we could have two separate
functions:
  validateHandle()
  isHandleValid()

For nullable handle field, we generate code to call validateHandle().
For non-nullable handle field, we generate code to call validateHandle() &&
isHandleValid().

We could consider similar things for structs/arrays, too.

In C++, I did such things because validation is on the performance critical
path. But I don't think I know about javascript performance very well. Maybe it
doesn't matter.

[hansmuller, 2014/08/20 17:46:31]

I don't know what the performance impact of an additional boolean function
parameter to these functions is for JS either. If it's OK with you, I'd like to
defer JS bindings performance tuning until it can be informed by benchmarks.

[yzshen1, 2014/08/20 17:54:08]

Sounds good.

     var index = this.message.buffer.getUint32(offset);
+
+    if (index === codec.kEncodedInvalidHandleValue)
+      return nullable ?
+          validationError.NONE : validationError.UNEXPECTED_INVALID_HANDLE;
+
     if (!this.claimHandle(index))
       return validationError.ILLEGAL_HANDLE;
     return validationError.NONE;
   }
 
   Validator.prototype.validateStructHeader =
       function(offset, minNumBytes, minNumFields) {
     if (!codec.isAligned(offset))
       return validationError.MISALIGNED_OBJECT;
 
@@ skipping 45 matching lines @@
   // NULL_MOJO_POINTER if the pointer represents a null, or JS null if the
   // pointer's value is not valid.
   Validator.prototype.decodePointer = function(offset) {
     var pointerValue = this.message.buffer.getUint64(offset);
     if (pointerValue === 0)
       return NULL_MOJO_POINTER;
     var bufferOffset = offset + pointerValue;
     return Number.isSafeInteger(bufferOffset) ? bufferOffset : null;
   }
 
-  Validator.prototype.validateArrayPointer =
-      function(offset, elementSize, expectedElementCount, elementType) {
+  Validator.prototype.validateArrayPointer = function(
+      offset, elementSize, expectedElementCount, elementType, nullable) {
     var arrayOffset = this.decodePointer(offset);
     if (arrayOffset === null)
       return validationError.ILLEGAL_POINTER;
+
     if (arrayOffset === NULL_MOJO_POINTER)
-      return validationError.NONE;
+      return nullable ?
+          validationError.NONE : validationError.UNEXPECTED_NULL_POINTER;
+
     return this.validateArray(
         arrayOffset, elementSize, expectedElementCount, elementType);
   }
 
-  Validator.prototype.validateStructPointer = function(offset, structClass) {
+  Validator.prototype.validateStructPointer = function(
+        offset, structClass, nullable) {
     var structOffset = this.decodePointer(offset);
     if (structOffset === null)
       return validationError.ILLEGAL_POINTER;
+
     if (structOffset === NULL_MOJO_POINTER)
-      return validationError.NONE;
+      return nullable ?
+          validationError.NONE : validationError.UNEXPECTED_NULL_POINTER;
+
     return structClass.validate(this, structOffset);
   }
 
   Validator.prototype.validateStringPointer = function(offset) {
     return this.validateArrayPointer(
         offset, codec.Uint8.encodedSize, 0, codec.Uint8);
   }
 
   // Similar to Array_Data<T>::Validate()
   // mojo/public/cpp/bindings/lib/array_internal.h
@@ skipping 19 matching lines @@
 
     if (expectedElementCount != 0 && numElements != expectedElementCount)
       return validationError.UNEXPECTED_ARRAY_HEADER;
 
     if (!this.claimRange(offset, numBytes))
       return validationError.ILLEGAL_MEMORY_RANGE;
 
     // Validate the array's elements if they are pointers or handles.
 
     var elementsOffset = offset + codec.kArrayHeaderSize;
-    if (elementType === codec.Handle)
-      return this.validateHandleElements(elementsOffset, numElements);
+    var nullable = isNullable(elementType);
+
+    if (isHandleClass(elementType))
+      return this.validateHandleElements(elementsOffset, numElements, nullable);
+    if (isStringClass(elementType))
+      return this.validateArrayElements(
+          elementsOffset, numElements, codec.Uint8, nullable)
     if (elementType instanceof codec.PointerTo)
       return this.validateStructElements(
-          elementsOffset, numElements, elementType.cls);
-    if (elementType instanceof codec.String)
-      return this.validateArrayElements(
-          elementsOffset, numElements, codec.Uint8);
+          elementsOffset, numElements, elementType.cls, nullable);
     if (elementType instanceof codec.ArrayOf)
       return this.validateArrayElements(
-          elementsOffset, numElements, elementType.cls);
+          elementsOffset, numElements, elementType.cls, nullable);
 
     return validationError.NONE;
   }
 
   // Note: the |offset + i * elementSize| computation in the validateFooElements
   // methods below is "safe" because elementSize <= 8, offset and
   // numElements are uint32, and 0 <= i < numElements.
 
-  Validator.prototype.validateHandleElements = function(offset, numElements) {
+  Validator.prototype.validateHandleElements =
+      function(offset, numElements, nullable) {
     var elementSize = codec.Handle.encodedSize;
     for (var i = 0; i < numElements; i++) {
-      var index = this.message.buffer.getUint32(offset + i * elementSize);
-      if (!this.claimHandle(index))
-        return validationError.ILLEGAL_HANDLE;
+      var elementOffset = offset + i * elementSize;
+      var err = this.validateHandle(elementOffset, nullable);
+      if (err != validationError.NONE)
+        return err;
     }
     return validationError.NONE;
   }
 
   // The elementClass parameter is the element type of the element arrays.
   Validator.prototype.validateArrayElements =
-      function(offset, numElements, elementClass) {
+      function(offset, numElements, elementClass, nullable) {
     var elementSize = codec.PointerTo.prototype.encodedSize;
     for (var i = 0; i < numElements; i++) {
       var elementOffset = offset + i * elementSize;
       var err = this.validateArrayPointer(
-          elementOffset, elementClass.encodedSize, 0, elementClass);
+        elementOffset, elementClass.encodedSize, 0, elementClass, nullable);

[yzshen1, 2014/08/20 17:04:20]

wrong indent.

[hansmuller, 2014/08/20 17:46:31]

Done.

       if (err != validationError.NONE)
         return err;
     }
     return validationError.NONE;
   }
 
   Validator.prototype.validateStructElements =
-      function(offset, numElements, structClass) {
+      function(offset, numElements, structClass, nullable) {
     var elementSize = codec.PointerTo.prototype.encodedSize;
     for (var i = 0; i < numElements; i++) {
       var elementOffset = offset + i * elementSize;
-      var err = this.validateStructPointer(elementOffset, structClass);
+      var err =
+          this.validateStructPointer(elementOffset, structClass, nullable);
       if (err != validationError.NONE)
         return err;
     }
     return validationError.NONE;
   }
 
   var exports = {};
   exports.validationError = validationError;
   exports.Validator = Validator;
   return exports;
 });