Improve validation for invalid attribute and parameter values

Author: Christopher Jones

src/njsCommon.cpp

@@ -432,9 +432,23 @@ bool njsBaton::GetBoolFromJSON(Local<Object> obj, const char *key, int index,
 
     if (!error.empty())
         return false;
+
     jsValue = obj->Get(Nan::New<v8::String>(key).ToLocalChecked());
-    if (!jsValue->IsUndefined())
-        *value = jsValue->ToBoolean()->Value();
+
+    /* Undefined implies value not provided or equivalent */
+    if (!jsValue->IsUndefined()) {
+        if(jsValue->IsBoolean()) {
+            // Get the boolean value
+            *value = jsValue->ToBoolean()->Value();
+        }
+        else {
+            // Non-Boolean value provided, report error
+            error = njsMessages::Get ( errInvalidPropertyValueInParam, key,
+                                       index + 1 ) ;
+            return false;
+        }
+    }
+
     return true;
 }
 
@@ -457,15 +471,17 @@ bool njsBaton::GetIntFromJSON(Local<Object> obj, const char *key,
     if (jsValue->IsInt32()) {
         *value = Nan::To<int32_t>(jsValue).FromJust();
         return true;
-    } else if (jsValue->IsUndefined() || jsValue->IsNull()) {
+    } else if (jsValue->IsUndefined()) {
         return true;
-    } else if (jsValue->IsNumber()) {
+    } else if (jsValue->IsNumber() || jsValue->IsNull()) {
         error = njsMessages::Get(errInvalidPropertyValueInParam, key,
                 index + 1);
         return false;
+    } else {
+        error = njsMessages::Get(errInvalidPropertyTypeInParam, key,
+                                 index + 1);
+        return false;
     }
-    error = njsMessages::Get(errInvalidPropertyTypeInParam, key, index + 1);
-    return false;
 }
 
 
@@ -488,10 +504,17 @@ bool njsBaton::GetStringFromJSON(Local<Object> obj, const char *key, int index,
         v8::String::Utf8Value utf8str(jsValue->ToString());
         value = std::string(*utf8str, utf8str.length());
         return true;
-    } else if (jsValue->IsUndefined() || jsValue->IsNull())
+    } else if (jsValue->IsUndefined()) {
         return true;
-    error = njsMessages::Get(errInvalidPropertyTypeInParam, key, index + 1);
-    return false;
+    } else if ( jsValue->IsNull()) {
+        error = njsMessages::Get(errInvalidPropertyValueInParam, key,
+                               index + 1 );
+        return false;
+    } else  {
+        error = njsMessages::Get(errInvalidPropertyTypeInParam, key,
+                                 index + 1);
+        return false;
+    }
 }
 
 
@@ -515,15 +538,17 @@ bool njsBaton::GetUnsignedIntFromJSON(Local<Object> obj, const char *key,
     if (jsValue->IsUint32()) {
         *value = Nan::To<uint32_t>(jsValue).FromJust();
         return true;
-    } else if (jsValue->IsUndefined() || jsValue->IsNull()) {
+    } else if (jsValue->IsUndefined()) {
         return true;
-    } else if (jsValue->IsNumber()) {
+    } else if (jsValue->IsNumber() || jsValue->IsNull()) {
         error = njsMessages::Get(errInvalidPropertyValueInParam, key,
                 index + 1);
         return false;
+    } else {
+        error = njsMessages::Get(errInvalidPropertyTypeInParam, key,
+                                 index + 1);
+        return false;
     }
-    error = njsMessages::Get(errInvalidPropertyTypeInParam, key, index + 1);
-    return false;
 }
 
 

src/njsConnection.cpp

@@ -914,6 +914,12 @@ bool njsConnection::ProcessBindValue(Local<Value> value, njsVariable *var,
         return false;
     }
 
+    /* Expecting an array value here */
+    if ( !value->IsArray ()) {
+        baton->error = njsMessages::Get ( errNonArrayProvided ) ;
+        return false;
+    }
+
     // set the number of actual elements in the variable
     Nan::HandleScope scope;
     Local<Array> arrayVal = Local<Array>::Cast(value);

src/njsMessages.cpp

@@ -83,6 +83,7 @@ static const char *errMsg[] = {
     "NJS-046: pool alias \"%s\" already exists in the connection pool cache", // errPoolWithAliasAlreadyExists
     "NJS-047: pool alias \"%s\" not found in connection pool cache", // errPoolWithAliasNotFound
     "NJS-052: invalid data type at array index %d for bind position %d", // errIncompatibleTypeArrayIndexBind
+    "NJS-053: array value expected, a non-array value provided", //errNonArrayProvided
 };
 
 

src/njsMessages.h

@@ -86,6 +86,7 @@ typedef enum {
     errPoolWithAliasAlreadyExists,
     errPoolWithAliasNotFound,
     errIncompatibleTypeArrayIndexBind,
+    errNonArrayProvided,
 
     // New ones should be added here
 

src/njsOracle.cpp

@@ -491,7 +491,8 @@ NAN_SETTER(njsOracledb::SetExtendedMetaData)
 {
     njsOracledb *oracledb = (njsOracledb*) ValidateSetter(info);
     if (oracledb)
-        oracledb->extendedMetaData = value->ToBoolean()->Value();
+        oracledb->SetPropBool ( value, &oracledb->extendedMetaData,
+                            "extendedMetaData" ) ;
 }
 
 

test/autoCommit.js

@@ -1,4 +1,4 @@
-/* Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved. */
+/* Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved. */
 
 /******************************************************************************
  *
@@ -394,4 +394,89 @@ describe('7. autoCommit.js', function() {
     ], done);
   });
 
+  describe('7.4 global option - oracledb.autoCommit', function() {
+    var defaultValue;
+    beforeEach(function() {
+      defaultValue = oracledb.autoCommit;
+    });
+    afterEach(function() {
+      oracledb.autoCommit = defaultValue;
+    });
+
+    it('7.4.1 Negative - 0', function(done) {
+      setAsGlobalOption(0, done);
+    });
+
+    it('7.4.2 Negative - negative number', function(done) {
+      setAsGlobalOption(-1, done);
+    });
+
+    it('7.4.3 Negative - positive number', function(done) {
+      setAsGlobalOption(-1, done);
+    });
+
+    it('7.4.4 Negative - NaN', function(done) {
+      setAsGlobalOption(NaN, done);
+    });
+
+    it('7.4.5 Negative - undefined', function(done) {
+      setAsGlobalOption(undefined, done);
+    });
+
+    var setAsGlobalOption = function(setValue, callback) {
+      should.throws(
+        function() {
+          oracledb.autoCommit = setValue;
+        },
+        /NJS-004: invalid value for property autoCommit/
+      );
+      callback();
+    };
+  });
+
+  describe('7.5 set autoCommit as an execute() option', function() {
+
+    it('7.5.1 Negative - 0', function(done) {
+      setAsExecOption(0, done);
+    });
+
+    it('7.5.2 Negative - negative number', function(done) {
+      setAsExecOption(-1, done);
+    });
+
+    it('7.5.3 Negative - positive number', function(done) {
+      setAsExecOption(-1, done);
+    });
+
+    it('7.5.4 Negative - NaN', function(done) {
+      setAsExecOption(NaN, done);
+    });
+
+    it("7.5.5 works as 'false' when setting to 'undefined'", function(done) {
+      connection.execute(
+        "select user from dual",
+        [],
+        { autoCommit: undefined },
+        function(err, result) {
+          should.not.exist(err);
+          should.exist(result);
+          done();
+        }
+      );
+    });
+
+    var setAsExecOption = function(setValue, callback) {
+      connection.execute(
+        "select user from dual",
+        {},
+        { autoCommit: setValue },
+        function(err, result) {
+          should.not.exist(result);
+          should.exist(err);
+          should.strictEqual(err.message, "NJS-007: invalid value for \"autoCommit\" in parameter 3");
+          callback();
+        });
+    };
+  });
+
 });