Skip to content

Commit 4a35cfc

Browse files
sharadrajusharadraju
committed
Improve error messages and fix for issue #1561 
1 parent 53cf33c commit 4a35cfc

File tree

8 files changed

+133
-15
lines changed

8 files changed

+133
-15
lines changed

‎lib/errors.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ const ERR_WRONG_VALUE_FOR_DBOBJECT_ATTR = 134;
136136
const ERR_WRONG_VALUE_FOR_DBOBJECT_ELEM = 135;
137137
const ERR_WRONG_CRED_FOR_EXTAUTH = 136;
138138
const ERR_MISSING_BIND_VALUE = 137;
139+
const ERR_SERVER_VERSION_NOT_SUPPORTED = 138;
139140

140141
// Oracle Net layer errors start from 500
141142
const ERR_CONNECTION_CLOSED = 500;
@@ -393,6 +394,8 @@ messages.set(ERR_WRONG_CRED_FOR_EXTAUTH, // NJS-136
393394
'user name and password cannot be set when using external authentication');
394395
messages.set(ERR_MISSING_BIND_VALUE, // NJS-137
395396
'a bind variable replacement value for placeholder ":%s" was not provided');
397+
messages.set(ERR_SERVER_VERSION_NOT_SUPPORTED, // NJS-138
398+
'connections to this database server version are not supported by node-oracledb in Thin mode');
396399

397400
// Oracle Net layer errors
398401

@@ -763,6 +766,7 @@ module.exports = {
763766
ERR_WRONG_VALUE_FOR_DBOBJECT_ELEM,
764767
ERR_WRONG_CRED_FOR_EXTAUTH,
765768
ERR_MISSING_BIND_VALUE,
769+
ERR_SERVER_VERSION_NOT_SUPPORTED,
766770
ERR_CONNECTION_CLOSED_CODE: `${ERR_PREFIX}-${ERR_CONNECTION_CLOSED}`,
767771
assert,
768772
assertArgCount,

‎lib/thin/connection.js

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,13 @@ class ThinConnectionImpl extends ConnectionImpl {
528528
}
529529

530530
this._protocol = new Protocol(this);
531+
532+
// check if the protocol version supported by the database is high
533+
// enough; if not, reject the connection immediately
534+
if (this._protocol.caps.protocolVersion < constants.TNS_VERSION_MIN_ACCEPTED) {
535+
errors.throwErr(errors.ERR_SERVER_VERSION_NOT_SUPPORTED);
536+
}
537+
531538
try {
532539
let message = new messages.ProtocolMessage(this);
533540
await this._protocol._processMessage(message);

‎lib/thin/protocol/buffer.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,7 +686,9 @@ class BaseBuffer {
686686
let numBytes = value.length;
687687
if (numBytes <= constants.TNS_MAX_SHORT_LENGTH) {
688688
this.writeUInt8(numBytes);
689-
this.writeBytes(value);
689+
if (numBytes > 0) {
690+
this.writeBytes(value);
691+
}
690692
} else {
691693
let start = 0;
692694
this.writeUInt8(constants.TNS_LONG_LENGTH_INDICATOR);

‎lib/thin/protocol/capabilities.js

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ class Capabilities {
4646
this.runtimeCaps = Buffer.alloc(constants.TNS_RCAP_MAX);
4747
this.initCompileCaps();
4848
this.initRuntimeCaps();
49+
this.maxStringSize = 0;
4950
}
5051

5152
adjustForServerCompileCaps(serverCaps) {
@@ -56,8 +57,12 @@ class Capabilities {
5657
}
5758
}
5859

59-
adjustForServerRuntimeCaps() {
60-
// nothing to do currently
60+
adjustForServerRuntimeCaps(serverCaps) {
61+
if (serverCaps[constants.TNS_RCAP_TTC] & constants.TNS_RCAP_TTC_32K) {
62+
this.maxStringSize = 32767;
63+
} else {
64+
this.maxStringSize = 4000;
65+
}
6166
}
6267

6368
initCompileCaps() {

‎lib/thin/protocol/constants.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,7 @@ module.exports = {
557557
// versions
558558
TNS_VERSION_DESIRED: 316,
559559
TNS_VERSION_MINIMUM: 300,
560+
TNS_VERSION_MIN_ACCEPTED: 315,
560561
TNS_VERSION_MIN_LARGE_SDU: 315,
561562

562563
// other connection constants
@@ -684,7 +685,6 @@ module.exports = {
684685
TNS_DURATION_MID: 0x80000000,
685686
TNS_DURATION_OFFSET: 60,
686687
TNS_DURATION_SESSION: 10,
687-
TNS_MIN_LONG_LENGTH: 0x8000,
688688
TNS_MAX_LONG_LENGTH: 0x7fffffff,
689689
TNS_SDU: 8192,
690690
TNS_TDU: 65535,

‎lib/thin/protocol/messages/withData.js

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ class MessageWithData extends Message {
628628
// expects that and complains if any other value is sent!
629629
buf.writeUInt8(0);
630630
buf.writeUInt8(0);
631-
if (maxSize >= constants.TNS_MIN_LONG_LENGTH) {
631+
if (maxSize > buf.caps.maxStringSize) {
632632
buf.writeUB4(constants.TNS_MAX_LONG_LENGTH);
633633
} else {
634634
buf.writeUB4(maxSize);
@@ -676,7 +676,7 @@ class MessageWithData extends Message {
676676
this.writeBindParamsColumn(buf, variable, variable.values[i]);
677677
}
678678
} else {
679-
if (variable.maxSize >= constants.TNS_MIN_LONG_LENGTH) {
679+
if (variable.maxSize > buf.caps.maxStringSize) {
680680
foundLong = true;
681681
} else {
682682
this.writeBindParamsColumn(buf, variable,
@@ -689,10 +689,9 @@ class MessageWithData extends Message {
689689
if (bindInfo.isReturnBind)
690690
continue;
691691
const variable = bindInfo.bindVar;
692-
if (variable.maxSize >= constants.TNS_MIN_LONG_LENGTH) {
693-
this.writeBindParamsColumn(buf, variable,
694-
variable.values[pos + offset]);
695-
}
692+
if (variable.maxSize <= buf.caps.maxStringSize)
693+
continue;
694+
this.writeBindParamsColumn(buf, variable, variable.values[pos + offset]);
696695
}
697696
}
698697
}

‎lib/thin/statement.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ const BIND_PATTERN = /(?<=:)\s*((?:".*?")|(?:[\p{L}][\p{L}\p{Nd}_\$#]*)|\p{Nd}+)
4848
const DML_RETURNING_KEY = /(?<=\bRETURN(?:ING)?\b)/si;
4949
const DML_RETURNING_INTO_KEY = /(?<=\bINTO\b)/gsi;
5050

51-
const SINGLE_LINE_COMMENT_PATTERN = /--.*/;
52-
const MULTI_LINE_COMMENT_PATTERN = /\/\*.*?\*\//s;
51+
const SINGLE_LINE_COMMENT_PATTERN = /--.*/g;
52+
const MULTI_LINE_COMMENT_PATTERN = /\/\*.*?\*\//sg;
5353
const CONSTANT_STRING_PATTERN = /'.*?'/sg;
5454

5555
/**

‎test/getStmtInfo.js

Lines changed: 104 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -365,9 +365,110 @@ describe('162. getStmtInfo.js', function() {
365365
'OBJECT_GRANT_AS_GRANTOR',
366366
:schema_name_in
367367
from dual`;
368-
const connection = await oracledb.getConnection(dbConfig);
369-
const info = await connection.getStatementInfo(sql);
368+
const info = await conn.getStatementInfo(sql);
370369
assert.deepStrictEqual(info.bindNames, ['ÖOBJECT_NAME_IN', 'SCHEMA_NAME_IN']);
371-
await connection.close();
372370
}); // 162.28
371+
372+
it('162.29 Ignore multiple single line comments having : in sql', async function() {
373+
// Issue 1561
374+
const sql = `select
375+
1
376+
from
377+
-- :
378+
dual
379+
where
380+
-- :
381+
1=1`;
382+
const info = await conn.getStatementInfo(sql);
383+
assert.deepStrictEqual(info.bindNames, []);
384+
const result = await conn.execute(sql);
385+
assert.deepStrictEqual(result.rows[0], [1]);
386+
});
387+
388+
it('162.30 Ignore multiple single and multi line comments having : in sql', async function() {
389+
const sql = `select
390+
1
391+
from
392+
-- :
393+
/* adding
394+
multi line comments :1 */
395+
dual
396+
where
397+
/* adding :bind */
398+
-- :
399+
1=1`;
400+
const info = await conn.getStatementInfo(sql);
401+
assert.deepStrictEqual(info.bindNames, []);
402+
const result = await conn.execute(sql);
403+
assert.deepStrictEqual(result.rows[0], [1]);
404+
});
405+
406+
it('162.31 Multiple single line comments with binds before the comment in sql', async function() {
407+
const sql = `select
408+
-- :
409+
:1 -- :
410+
-- :
411+
from
412+
-- :
413+
dual
414+
where
415+
-- :
416+
1=1`;
417+
const info = await conn.getStatementInfo(sql);
418+
assert.deepStrictEqual(info.bindNames, ['1']);
419+
const result = await conn.execute(sql, [1]);
420+
assert.deepStrictEqual(result.rows[0], [1]);
421+
});
422+
423+
it('162.32 Multiple single line comments with binds after the comment in sql', async function() {
424+
const sql = `select
425+
-- :
426+
-- :
427+
:1
428+
-- :
429+
from
430+
-- :
431+
dual
432+
where
433+
-- :
434+
1=1`;
435+
const info = await conn.getStatementInfo(sql);
436+
assert.deepStrictEqual(info.bindNames, ['1']);
437+
const result = await conn.execute(sql, [1]);
438+
assert.deepStrictEqual(result.rows[0], [1]);
439+
});
440+
441+
it('162.33 Bind Variable before, inbetween and after comments in sql', async function() {
442+
const sql = `select
443+
:1,
444+
-- :
445+
:2
446+
-- :
447+
from
448+
-- :
449+
dual
450+
where
451+
-- :
452+
:3=1`;
453+
const info = await conn.getStatementInfo(sql);
454+
assert.deepStrictEqual(info.bindNames, ['1', '2', '3']);
455+
const result = await conn.execute(sql, [1, 1, 1]);
456+
assert.deepStrictEqual(result.rows[0], [1, 1]);
457+
});
458+
459+
it('162.34 ignore literals only during bind processing', async function() {
460+
const sql = `select 'HELLO' from
461+
-- :
462+
/* adding
463+
multi line comments :1 */
464+
dual
465+
where
466+
/* adding :bind */
467+
1=1`;
468+
const info = await conn.getStatementInfo(sql);
469+
assert.deepStrictEqual(info.bindNames, []);
470+
const result = await conn.execute(sql);
471+
assert.deepStrictEqual(result.rows[0], ['HELLO']);
472+
});
473+
373474
});

0 commit comments

Comments
 (0)