Skip to content

Commit 41cab45

Browse files
author
Christopher Jones
committed
Fix REF CURSOR mem leak in error handling scenario. Also, bail out earlier when querying unsupported column types.
1 parent 96e64c2 commit 41cab45

File tree

5 files changed

+160
-166
lines changed

5 files changed

+160
-166
lines changed

‎src/njs/src/njsConnection.cpp‎

Lines changed: 106 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,13 @@ void Connection::ProcessOptions (Nan::NAN_METHOD_ARGS_TYPE args, unsigned int in
563563
options = args[index]->ToObject();
564564
NJS_GET_UINT_FROM_JSON ( executeBaton->maxRows, executeBaton->error,
565565
options, "maxRows", 2, exitProcessOptions );
566+
567+
if ( executeBaton->maxRows <= 0 )
568+
{
569+
executeBaton->error = NJSMessages::getErrorMsg ( errInvalidmaxRows );
570+
goto exitProcessOptions;
571+
}
572+
566573
NJS_GET_UINT_FROM_JSON ( executeBaton->prefetchRows, executeBaton->error,
567574
options, "prefetchRows", 2, exitProcessOptions );
568575
NJS_GET_UINT_FROM_JSON ( executeBaton->outFormat, executeBaton->error,
@@ -1628,6 +1635,8 @@ void Connection::Async_Execute (uv_work_t *req)
16281635

16291636
Connection::CopyMetaData ( executeBaton->mInfo, executeBaton, mData,
16301637
executeBaton->numCols );
1638+
if ( !executeBaton->error.empty() )
1639+
goto exitAsyncExecute;
16311640

16321641
if ( executeBaton->getRS )
16331642
{
@@ -1763,6 +1772,9 @@ void Connection::Async_Execute (uv_work_t *req)
17631772
}
17641773
Connection::CopyMetaData ( extBind->mInfo, executeBaton, mData,
17651774
extBind->numCols );
1775+
if ( !executeBaton->error.empty() )
1776+
goto exitAsyncExecute;
1777+
17661778
executeBaton->extBinds [ b ] = extBind;
17671779
}
17681780
else
@@ -1807,6 +1819,21 @@ void Connection::Async_Execute (uv_work_t *req)
18071819
{
18081820
executeBaton->dpistmt->release ();
18091821
}
1822+
1823+
// In case of error, release the statement handles allocated (REF CURSOR)
1824+
if ( !(executeBaton->error).empty() )
1825+
{
1826+
for( unsigned int index = 0 ;index < executeBaton->binds.size();
1827+
index++ )
1828+
{
1829+
if( executeBaton->binds[index]->value &&
1830+
( executeBaton->binds[index]->type == DpiRSet ) )
1831+
{
1832+
((Stmt*)executeBaton->binds[index]->value)->release ();
1833+
executeBaton->binds[index]->value = NULL;
1834+
}
1835+
}
1836+
}
18101837
;
18111838
}
18121839

@@ -1961,7 +1988,9 @@ void Connection::CopyMetaData ( MetaInfo *mInfo,
19611988
const MetaData* mData,
19621989
const unsigned int numCols )
19631990
{
1964-
for ( unsigned int col = 0; col < numCols; col++ )
1991+
bool error = false;
1992+
1993+
for ( unsigned int col = 0; !error && ( col < numCols ); col++ )
19651994
{
19661995
mInfo[col].name = string( (const char*)mData[col].colName,
19671996
mData[col].colNameLen );
@@ -2045,6 +2074,16 @@ void Connection::CopyMetaData ( MetaInfo *mInfo,
20452074
mInfo[col].dbType = NJS_DATATYPE_UNKNOWN;
20462075
break;
20472076
}
2077+
2078+
if ( mInfo[col].njsFetchType == NJS_DATATYPE_UNKNOWN )
2079+
{
2080+
error = true;
2081+
}
2082+
}
2083+
2084+
if ( error )
2085+
{
2086+
executeBaton->error = NJSMessages::getErrorMsg ( errUnsupportedDatType );
20482087
}
20492088
}
20502089

@@ -2270,19 +2309,17 @@ unsigned short Connection::GetTargetType ( eBaton *executeBaton,
22702309
*/
22712310
void Connection::DoDefines ( eBaton* executeBaton )
22722311
{
2273-
unsigned int numCols = executeBaton->numCols;
2274-
Define *defines = executeBaton->defines = new Define[numCols];
2275-
int csratio = executeBaton->dpiconn->getByteExpansionRatio ();
2276-
2277-
// Check for maxRows must be greater than zero in case of non-resultSet
2278-
if ( executeBaton->maxRows == 0 )
2279-
{
2280-
executeBaton->error = NJSMessages::getErrorMsg ( errInvalidmaxRows );
2281-
return;
2282-
}
2312+
unsigned int numCols = executeBaton->numCols;
2313+
Define *defines = executeBaton->defines = new Define[numCols];
2314+
int csratio = executeBaton->dpiconn->getByteExpansionRatio ();
2315+
bool error = false;
22832316

2284-
for (unsigned int col = 0; col < numCols; col++)
2317+
for (unsigned int col = 0; !error && ( col < numCols ); col++)
22852318
{
2319+
/*
2320+
* Only supported DB column types handled here and others would have
2321+
* reported error while processing meta-data
2322+
*/
22862323
switch( executeBaton->mInfo[col].dbType )
22872324
{
22882325
case dpi::DpiNumber :
@@ -2297,7 +2334,7 @@ void Connection::DoDefines ( eBaton* executeBaton )
22972334
executeBaton->maxRows ) )
22982335
{
22992336
executeBaton->error = NJSMessages::getErrorMsg( errResultsTooLarge );
2300-
return;
2337+
error = true;
23012338
}
23022339
else
23032340
{
@@ -2308,7 +2345,7 @@ void Connection::DoDefines ( eBaton* executeBaton )
23082345
{
23092346
executeBaton->error = NJSMessages::getErrorMsg(
23102347
errInsufficientMemory );
2311-
return;
2348+
error = true;
23122349
}
23132350
}
23142351

@@ -2320,7 +2357,7 @@ void Connection::DoDefines ( eBaton* executeBaton )
23202357
/*
23212358
* the buffer size is increased to account for possible character
23222359
* size expansion when data is converted from the DB character set
2323-
* to AL32UTF8
2360+
* to client character set
23242361
*/
23252362

23262363
if ( executeBaton->mInfo[col].byteSize != 0 )
@@ -2333,7 +2370,7 @@ void Connection::DoDefines ( eBaton* executeBaton )
23332370
{
23342371
executeBaton->error = NJSMessages::getErrorMsg(
23352372
errResultsTooLarge );
2336-
return;
2373+
error = true;
23372374
}
23382375
else
23392376
{
@@ -2343,32 +2380,21 @@ void Connection::DoDefines ( eBaton* executeBaton )
23432380
{
23442381
executeBaton->error = NJSMessages::getErrorMsg(
23452382
errInsufficientMemory );
2346-
return;
2383+
error = true;
23472384
}
23482385
}
23492386
}
23502387
/* The null scenario will have indicator as -1, so memory allocation
23512388
* not required.
23522389
*/
23532390
break;
2391+
case dpi::DpiTimestampTZ:
2392+
// TIMESTAMPTZ WITH TIMEZONE (TZ) supported only as STRING value
23542393
case dpi::DpiDate :
23552394
case dpi::DpiTimestamp:
2356-
case dpi::DpiTimestampTZ:
23572395
case dpi::DpiTimestampLTZ:
23582396
defines[col].fetchType = executeBaton->mInfo[col].dpiFetchType;
23592397

2360-
if ( ( executeBaton->mInfo[col].dbType == dpi::DpiTimestampTZ ) &&
2361-
( defines[col].fetchType != dpi::DpiVarChar ))
2362-
{
2363-
/*
2364-
* TIMESTAMP WITH TIMEZONE (TZ) column type supported only as
2365-
* STRING value.
2366-
*/
2367-
executeBaton->error = NJSMessages::getErrorMsg (
2368-
errUnsupportedDatType ) ;
2369-
return;
2370-
}
2371-
23722398
if ( defines[col].fetchType != dpi::DpiVarChar )
23732399
{
23742400
defines[col].dttmarr = executeBaton->dpienv->getDateTimeArray (
@@ -2385,8 +2411,9 @@ void Connection::DoDefines ( eBaton* executeBaton )
23852411
if ( NJS_SIZE_T_OVERFLOW ( defines[col].maxSize,
23862412
executeBaton->maxRows ) )
23872413
{
2388-
executeBaton->error = NJSMessages::getErrorMsg( errResultsTooLarge );
2389-
return;
2414+
executeBaton->error =
2415+
NJSMessages::getErrorMsg( errResultsTooLarge );
2416+
error = true;
23902417
}
23912418
else
23922419
{
@@ -2397,7 +2424,7 @@ void Connection::DoDefines ( eBaton* executeBaton )
23972424
{
23982425
executeBaton->error = NJSMessages::getErrorMsg(
23992426
errInsufficientMemory);
2400-
return;
2427+
error = true;
24012428
}
24022429
}
24032430

@@ -2412,15 +2439,18 @@ void Connection::DoDefines ( eBaton* executeBaton )
24122439
{
24132440
executeBaton->error = NJSMessages::getErrorMsg (
24142441
errResultsTooLarge ) ;
2415-
return;
2442+
error = true;
24162443
}
2417-
defines[col].buf = (char *)malloc(defines[col].maxSize *
2418-
(size_t) executeBaton->maxRows) ;
2419-
if ( !defines[col].buf )
2444+
else
24202445
{
2421-
executeBaton->error = NJSMessages::getErrorMsg (
2422-
errInsufficientMemory );
2423-
return;
2446+
defines[col].buf = (char *)malloc(defines[col].maxSize *
2447+
(size_t) executeBaton->maxRows) ;
2448+
if ( !defines[col].buf )
2449+
{
2450+
executeBaton->error = NJSMessages::getErrorMsg (
2451+
errInsufficientMemory );
2452+
error = true;
2453+
}
24242454
}
24252455
break;
24262456

@@ -2434,7 +2464,7 @@ void Connection::DoDefines ( eBaton* executeBaton )
24342464
executeBaton->maxRows ) )
24352465
{
24362466
executeBaton->error = NJSMessages::getErrorMsg( errResultsTooLarge );
2437-
return;
2467+
error = true;
24382468
}
24392469
else
24402470
{
@@ -2443,34 +2473,33 @@ void Connection::DoDefines ( eBaton* executeBaton )
24432473

24442474
if( !defines[col].buf )
24452475
{
2446-
executeBaton->error = NJSMessages::getErrorMsg( errInsufficientMemory );
2447-
return;
2476+
executeBaton->error =
2477+
NJSMessages::getErrorMsg( errInsufficientMemory );
2478+
error = true;
24482479
}
24492480
}
24502481

2451-
for (unsigned int j = 0; j < executeBaton->maxRows; j++)
2482+
if ( !error )
24522483
{
2453-
((Descriptor **)(defines[col].buf))[j] =
2454-
executeBaton->dpienv->allocDescriptor(LobDescriptorType);
2484+
for (unsigned int j = 0; j < executeBaton->maxRows; j++)
2485+
{
2486+
((Descriptor **)(defines[col].buf))[j] =
2487+
executeBaton->dpienv->allocDescriptor(LobDescriptorType);
2488+
}
24552489
}
24562490
break;
24572491

24582492
case dpi::DpiRowid:
24592493
defines[col].fetchType = executeBaton->mInfo[col].dpiFetchType;
24602494

2461-
if ( defines[col].fetchType != dpi::DpiVarChar )
2462-
{
2463-
executeBaton->error = NJSMessages::getErrorMsg (
2464-
errUnsupportedDatType);
2465-
return;
2466-
}
2495+
// ROWID supported only as STRING value
24672496
defines[col].maxSize = NJS_MAX_FETCH_AS_STRING_SIZE;
24682497

24692498
if ( NJS_SIZE_T_OVERFLOW ( defines[col].maxSize,
24702499
executeBaton->maxRows ) )
24712500
{
24722501
executeBaton->error = NJSMessages::getErrorMsg( errResultsTooLarge );
2473-
return;
2502+
error = true;
24742503
}
24752504
else
24762505
{
@@ -2481,36 +2510,41 @@ void Connection::DoDefines ( eBaton* executeBaton )
24812510
{
24822511
executeBaton->error = NJSMessages::getErrorMsg(
24832512
errInsufficientMemory);
2484-
return;
2513+
error = true;
24852514
}
24862515
}
24872516
break;
24882517

24892518
default :
2490-
executeBaton->error = NJSMessages::getErrorMsg(errUnsupportedDatType);
2491-
return;
2519+
// For unsupported column types, an error is reported earlier itself
2520+
executeBaton->error = NJSMessages::getErrorMsg( errInternalError,
2521+
"default:", "DoDefines" );
2522+
error = true;
24922523
break;
24932524
}
24942525

2495-
defines[col].ind = (short*)malloc (sizeof(short)*(executeBaton->maxRows));
2496-
if(!defines[col].ind)
2526+
if ( !error )
24972527
{
2498-
executeBaton->error = NJSMessages::getErrorMsg( errInsufficientMemory );
2499-
return;
2500-
}
2501-
defines[col].len = (DPI_BUFLEN_TYPE *)malloc(sizeof(DPI_BUFLEN_TYPE)*
2502-
executeBaton->maxRows);
2503-
if(!defines[col].len)
2504-
{
2505-
executeBaton->error = NJSMessages::getErrorMsg( errInsufficientMemory );
2506-
return;
2507-
}
2528+
defines[col].ind = (short*)malloc ( sizeof( short ) *
2529+
( executeBaton->maxRows ) );
2530+
if(!defines[col].ind)
2531+
{
2532+
executeBaton->error = NJSMessages::getErrorMsg( errInsufficientMemory );
2533+
error = true;
2534+
}
2535+
defines[col].len = (DPI_BUFLEN_TYPE *)malloc(sizeof(DPI_BUFLEN_TYPE)*
2536+
executeBaton->maxRows);
2537+
if(!defines[col].len)
2538+
{
2539+
executeBaton->error = NJSMessages::getErrorMsg( errInsufficientMemory );
2540+
error = true;
2541+
}
25082542

2509-
executeBaton->dpistmt->define(col+1, defines[col].fetchType,
2510-
(defines[col].buf) ? defines[col].buf : defines[col].extbuf,
2511-
defines[col].maxSize, defines[col].ind, defines[col].len);
2543+
executeBaton->dpistmt->define(col+1, defines[col].fetchType,
2544+
(defines[col].buf) ? defines[col].buf : defines[col].extbuf,
2545+
defines[col].maxSize, defines[col].ind, defines[col].len);
2546+
}
25122547
}
2513-
executeBaton->numCols = numCols;
25142548
}
25152549

25162550
/*****************************************************************************/

‎src/njs/src/njsOracle.cpp‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,9 +383,21 @@ NAN_GETTER(Oracledb::GetMaxRows)
383383
*/
384384
NAN_SETTER(Oracledb::SetMaxRows)
385385
{
386-
Oracledb* oracledb = Nan::ObjectWrap::Unwrap<Oracledb>(info.Holder());
386+
unsigned int tempMaxRows = NJS_MAX_ROWS;
387+
Oracledb* oracledb = Nan::ObjectWrap::Unwrap<Oracledb>(info.Holder());
388+
387389
NJS_CHECK_OBJECT_VALID(oracledb);
388-
NJS_SET_PROP_UINT(oracledb->maxRows_, value, "maxRows");
390+
NJS_SET_PROP_UINT(tempMaxRows, value, "maxRows");
391+
392+
if ( tempMaxRows <= 0 )
393+
{
394+
string errMsg = NJSMessages::getErrorMsg ( errInvalidmaxRows );
395+
NJS_SET_EXCEPTION( errMsg.c_str(), errMsg.length() );
396+
}
397+
else
398+
{
399+
oracledb->maxRows_ = tempMaxRows;
400+
}
389401
}
390402

391403
/*****************************************************************************/

0 commit comments

Comments
 (0)