Skip to content

Commit f6a4204

Browse files
committed
Improve validation for invalid attribute and parameter values
1 parent d930b6c commit f6a4204

File tree

10 files changed

+282
-145
lines changed

10 files changed

+282
-145
lines changed

src/njsCommon.cpp

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -432,9 +432,23 @@ bool njsBaton::GetBoolFromJSON(Local<Object> obj, const char *key, int index,
432432

433433
if (!error.empty())
434434
return false;
435+
435436
jsValue = obj->Get(Nan::New<v8::String>(key).ToLocalChecked());
436-
if (!jsValue->IsUndefined())
437-
*value = jsValue->ToBoolean()->Value();
437+
438+
/* Undefined implies value not provided or equivalent */
439+
if (!jsValue->IsUndefined()) {
440+
if(jsValue->IsBoolean()) {
441+
// Get the boolean value
442+
*value = jsValue->ToBoolean()->Value();
443+
}
444+
else {
445+
// Non-Boolean value provided, report error
446+
error = njsMessages::Get ( errInvalidPropertyValueInParam, key,
447+
index + 1 ) ;
448+
return false;
449+
}
450+
}
451+
438452
return true;
439453
}
440454

@@ -457,15 +471,17 @@ bool njsBaton::GetIntFromJSON(Local<Object> obj, const char *key,
457471
if (jsValue->IsInt32()) {
458472
*value = Nan::To<int32_t>(jsValue).FromJust();
459473
return true;
460-
} else if (jsValue->IsUndefined() || jsValue->IsNull()) {
474+
} else if (jsValue->IsUndefined()) {
461475
return true;
462-
} else if (jsValue->IsNumber()) {
476+
} else if (jsValue->IsNumber() || jsValue->IsNull()) {
463477
error = njsMessages::Get(errInvalidPropertyValueInParam, key,
464478
index + 1);
465479
return false;
480+
} else {
481+
error = njsMessages::Get(errInvalidPropertyTypeInParam, key,
482+
index + 1);
483+
return false;
466484
}
467-
error = njsMessages::Get(errInvalidPropertyTypeInParam, key, index + 1);
468-
return false;
469485
}
470486

471487

@@ -488,10 +504,17 @@ bool njsBaton::GetStringFromJSON(Local<Object> obj, const char *key, int index,
488504
v8::String::Utf8Value utf8str(jsValue->ToString());
489505
value = std::string(*utf8str, utf8str.length());
490506
return true;
491-
} else if (jsValue->IsUndefined() || jsValue->IsNull())
507+
} else if (jsValue->IsUndefined()) {
492508
return true;
493-
error = njsMessages::Get(errInvalidPropertyTypeInParam, key, index + 1);
494-
return false;
509+
} else if ( jsValue->IsNull()) {
510+
error = njsMessages::Get(errInvalidPropertyValueInParam, key,
511+
index + 1 );
512+
return false;
513+
} else {
514+
error = njsMessages::Get(errInvalidPropertyTypeInParam, key,
515+
index + 1);
516+
return false;
517+
}
495518
}
496519

497520

@@ -515,15 +538,17 @@ bool njsBaton::GetUnsignedIntFromJSON(Local<Object> obj, const char *key,
515538
if (jsValue->IsUint32()) {
516539
*value = Nan::To<uint32_t>(jsValue).FromJust();
517540
return true;
518-
} else if (jsValue->IsUndefined() || jsValue->IsNull()) {
541+
} else if (jsValue->IsUndefined()) {
519542
return true;
520-
} else if (jsValue->IsNumber()) {
543+
} else if (jsValue->IsNumber() || jsValue->IsNull()) {
521544
error = njsMessages::Get(errInvalidPropertyValueInParam, key,
522545
index + 1);
523546
return false;
547+
} else {
548+
error = njsMessages::Get(errInvalidPropertyTypeInParam, key,
549+
index + 1);
550+
return false;
524551
}
525-
error = njsMessages::Get(errInvalidPropertyTypeInParam, key, index + 1);
526-
return false;
527552
}
528553

529554

src/njsConnection.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,12 @@ bool njsConnection::ProcessBindValue(Local<Value> value, njsVariable *var,
914914
return false;
915915
}
916916

917+
/* Expecting an array value here */
918+
if ( !value->IsArray ()) {
919+
baton->error = njsMessages::Get ( errNonArrayProvided ) ;
920+
return false;
921+
}
922+
917923
// set the number of actual elements in the variable
918924
Nan::HandleScope scope;
919925
Local<Array> arrayVal = Local<Array>::Cast(value);

src/njsMessages.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ static const char *errMsg[] = {
8383
"NJS-046: pool alias \"%s\" already exists in the connection pool cache", // errPoolWithAliasAlreadyExists
8484
"NJS-047: pool alias \"%s\" not found in connection pool cache", // errPoolWithAliasNotFound
8585
"NJS-052: invalid data type at array index %d for bind position %d", // errIncompatibleTypeArrayIndexBind
86+
"NJS-053: array value expected, a non-array value provided", //errNonArrayProvided
8687
};
8788

8889

src/njsMessages.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ typedef enum {
8686
errPoolWithAliasAlreadyExists,
8787
errPoolWithAliasNotFound,
8888
errIncompatibleTypeArrayIndexBind,
89+
errNonArrayProvided,
8990

9091
// New ones should be added here
9192

src/njsOracle.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,8 @@ NAN_SETTER(njsOracledb::SetExtendedMetaData)
491491
{
492492
njsOracledb *oracledb = (njsOracledb*) ValidateSetter(info);
493493
if (oracledb)
494-
oracledb->extendedMetaData = value->ToBoolean()->Value();
494+
oracledb->SetPropBool ( value, &oracledb->extendedMetaData,
495+
"extendedMetaData" ) ;
495496
}
496497

497498

test/autoCommit.js

Lines changed: 86 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2015, 2016, Oracle and/or its affiliates. All rights reserved. */
1+
/* Copyright (c) 2015, 2017, Oracle and/or its affiliates. All rights reserved. */
22

33
/******************************************************************************
44
*
@@ -394,4 +394,89 @@ describe('7. autoCommit.js', function() {
394394
], done);
395395
});
396396

397+
describe('7.4 global option - oracledb.autoCommit', function() {
398+
var defaultValue;
399+
beforeEach(function() {
400+
defaultValue = oracledb.autoCommit;
401+
});
402+
afterEach(function() {
403+
oracledb.autoCommit = defaultValue;
404+
});
405+
406+
it('7.4.1 Negative - 0', function(done) {
407+
setAsGlobalOption(0, done);
408+
});
409+
410+
it('7.4.2 Negative - negative number', function(done) {
411+
setAsGlobalOption(-1, done);
412+
});
413+
414+
it('7.4.3 Negative - positive number', function(done) {
415+
setAsGlobalOption(-1, done);
416+
});
417+
418+
it('7.4.4 Negative - NaN', function(done) {
419+
setAsGlobalOption(NaN, done);
420+
});
421+
422+
it('7.4.5 Negative - undefined', function(done) {
423+
setAsGlobalOption(undefined, done);
424+
});
425+
426+
var setAsGlobalOption = function(setValue, callback) {
427+
should.throws(
428+
function() {
429+
oracledb.autoCommit = setValue;
430+
},
431+
/NJS-004: invalid value for property autoCommit/
432+
);
433+
callback();
434+
};
435+
});
436+
437+
describe('7.5 set autoCommit as an execute() option', function() {
438+
439+
it('7.5.1 Negative - 0', function(done) {
440+
setAsExecOption(0, done);
441+
});
442+
443+
it('7.5.2 Negative - negative number', function(done) {
444+
setAsExecOption(-1, done);
445+
});
446+
447+
it('7.5.3 Negative - positive number', function(done) {
448+
setAsExecOption(-1, done);
449+
});
450+
451+
it('7.5.4 Negative - NaN', function(done) {
452+
setAsExecOption(NaN, done);
453+
});
454+
455+
it("7.5.5 works as 'false' when setting to 'undefined'", function(done) {
456+
connection.execute(
457+
"select user from dual",
458+
[],
459+
{ autoCommit: undefined },
460+
function(err, result) {
461+
should.not.exist(err);
462+
should.exist(result);
463+
done();
464+
}
465+
);
466+
});
467+
468+
var setAsExecOption = function(setValue, callback) {
469+
connection.execute(
470+
"select user from dual",
471+
{},
472+
{ autoCommit: setValue },
473+
function(err, result) {
474+
should.not.exist(result);
475+
should.exist(err);
476+
should.strictEqual(err.message, "NJS-007: invalid value for \"autoCommit\" in parameter 3");
477+
callback();
478+
});
479+
};
480+
});
481+
397482
});

0 commit comments

Comments
 (0)