From edafb7d566f602a2bc429720eb52bc488695e302 Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Tue, 19 Sep 2017 11:16:56 +0100 Subject: [PATCH 1/3] check if statement text is empty on Session.run and Transaction.run --- src/v1/session.js | 5 ++++- src/v1/transaction.js | 5 ++++- test/v1/session.test.js | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/v1/session.js b/src/v1/session.js index 5ff5f13e6..f6169ef70 100644 --- a/src/v1/session.js +++ b/src/v1/session.js @@ -53,7 +53,7 @@ class Session { /** * Run Cypher statement - * Could be called with a statement object i.e.: {statement: "MATCH ...", parameters: {param: 1}} + * Could be called with a statement object i.e.: {text: "MATCH ...", parameters: {param: 1}} * or with the statement and parameters as separate arguments. * @param {mixed} statement - Cypher statement to execute * @param {Object} parameters - Map with parameters to use in statement @@ -65,6 +65,9 @@ class Session { statement = statement.text; } assertString(statement, 'Cypher statement'); + if (statement.length == 0) { + throw new TypeError('Cypher statement is expected to be a non-empty string.'); + } return this._run(statement, parameters, (connection, streamObserver) => connection.run(statement, parameters, streamObserver) diff --git a/src/v1/transaction.js b/src/v1/transaction.js index 1c1ac34b3..cf2cd80cb 100644 --- a/src/v1/transaction.js +++ b/src/v1/transaction.js @@ -53,7 +53,7 @@ class Transaction { /** * Run Cypher statement - * Could be called with a statement object i.e.: {statement: "MATCH ...", parameters: {param: 1}} + * Could be called with a statement object i.e.: {text: "MATCH ...", parameters: {param: 1}} * or with the statement and parameters as separate arguments. * @param {mixed} statement - Cypher statement to execute * @param {Object} parameters - Map with parameters to use in statement @@ -65,6 +65,9 @@ class Transaction { statement = statement.text; } assertString(statement, "Cypher statement"); + if (statement.length == 0) { + throw new TypeError('Cypher statement is expected to be a non-empty string.'); + } return this._state.run(this._connectionHolder, new _TransactionStreamObserver(this), statement, parameters); } diff --git a/test/v1/session.test.js b/test/v1/session.test.js index b6dc104ad..51c358e15 100644 --- a/test/v1/session.test.js +++ b/test/v1/session.test.js @@ -427,6 +427,7 @@ describe('session', () => { expect(() => session.run({})).toThrowError(TypeError); expect(() => session.run(42)).toThrowError(TypeError); expect(() => session.run([])).toThrowError(TypeError); + expect(() => session.run('')).toThrowError(TypeError); expect(() => session.run(['CREATE ()'])).toThrowError(TypeError); expect(() => session.run({statement: 'CREATE ()'})).toThrowError(TypeError); From f54bdded716abb902a02c8c32391363ca5610e27 Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Wed, 20 Sep 2017 15:44:06 +0100 Subject: [PATCH 2/3] addressed review comments and ignored whitespaces during checks --- src/v1/internal/util.js | 9 +++++++++ src/v1/session.js | 7 ++----- src/v1/transaction.js | 7 ++----- test/internal/util.test.js | 29 ++++++++++++++++++++++++++++- 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/v1/internal/util.js b/src/v1/internal/util.js index 50811e23a..168052d77 100644 --- a/src/v1/internal/util.js +++ b/src/v1/internal/util.js @@ -58,6 +58,14 @@ function assertString(obj, objName) { return obj; } +function assertCypherStatement(obj) { + assertString(obj, 'Cypher statement'); + if (obj.trim().length == 0) { + throw new TypeError('Cypher statement is expected to be a non-empty string.'); + } + return obj; +} + function isString(str) { return Object.prototype.toString.call(str) === '[object String]'; } @@ -118,6 +126,7 @@ export { isEmptyObjectOrNull, isString, assertString, + assertCypherStatement, parseScheme, parseUrl, parseHost, diff --git a/src/v1/session.js b/src/v1/session.js index f6169ef70..cfd947082 100644 --- a/src/v1/session.js +++ b/src/v1/session.js @@ -20,7 +20,7 @@ import StreamObserver from './internal/stream-observer'; import Result from './result'; import Transaction from './transaction'; import {newError} from './error'; -import {assertString} from './internal/util'; +import {assertCypherStatement} from './internal/util'; import ConnectionHolder from './internal/connection-holder'; import Driver, {READ, WRITE} from './driver'; import TransactionExecutor from './internal/transaction-executor'; @@ -64,10 +64,7 @@ class Session { parameters = statement.parameters || {}; statement = statement.text; } - assertString(statement, 'Cypher statement'); - if (statement.length == 0) { - throw new TypeError('Cypher statement is expected to be a non-empty string.'); - } + assertCypherStatement(statement); return this._run(statement, parameters, (connection, streamObserver) => connection.run(statement, parameters, streamObserver) diff --git a/src/v1/transaction.js b/src/v1/transaction.js index cf2cd80cb..f904d9275 100644 --- a/src/v1/transaction.js +++ b/src/v1/transaction.js @@ -18,7 +18,7 @@ */ import StreamObserver from './internal/stream-observer'; import Result from './result'; -import {assertString} from './internal/util'; +import {assertCypherStatement} from './internal/util'; import {EMPTY_CONNECTION_HOLDER} from './internal/connection-holder'; import Bookmark from './internal/bookmark'; @@ -64,10 +64,7 @@ class Transaction { parameters = statement.parameters || {}; statement = statement.text; } - assertString(statement, "Cypher statement"); - if (statement.length == 0) { - throw new TypeError('Cypher statement is expected to be a non-empty string.'); - } + assertCypherStatement(statement); return this._state.run(this._connectionHolder, new _TransactionStreamObserver(this), statement, parameters); } diff --git a/test/internal/util.test.js b/test/internal/util.test.js index 907ad3421..de9de43f9 100644 --- a/test/internal/util.test.js +++ b/test/internal/util.test.js @@ -19,7 +19,7 @@ import * as util from '../../src/v1/internal/util'; -describe('util', () => { +fdescribe('util', () => { it('should check empty objects', () => { expect(util.isEmptyObjectOrNull(null)).toBeTruthy(); @@ -55,6 +55,25 @@ describe('util', () => { verifyInvalidString(console.log); }); + it('should check cypher statements (non-empty strings)', () => { + verifyValidString(new String('foo')); + verifyValidString(String('foo')); + verifyValidString("foo"); + + verifyInvalidCypherStatement(''); + verifyInvalidCypherStatement('\n'); + verifyInvalidCypherStatement('\t'); + verifyInvalidCypherStatement('\r'); + verifyInvalidCypherStatement(' '); + verifyInvalidCypherStatement(' \n\r'); + verifyInvalidCypherStatement({}); + verifyInvalidCypherStatement({foo: 1}); + verifyInvalidCypherStatement([]); + verifyInvalidCypherStatement(['1']); + verifyInvalidCypherStatement([1, '2']); + verifyInvalidCypherStatement(console.log); + }); + it('should parse scheme', () => { verifyScheme('bolt://', 'bolt://localhost'); verifyScheme('bolt://', 'bolt://localhost:7687'); @@ -169,6 +188,14 @@ describe('util', () => { expect(() => util.assertString(str, 'Test string')).toThrowError(TypeError); } + function verifyValidCypherStatement(str) { + expect(util.assertCypherStatement(str)).toBe(str); + } + + function verifyInvalidCypherStatement(str) { + expect(() => util.assertCypherStatement(str)).toThrowError(TypeError); + } + function verifyScheme(expectedScheme, url) { expect(util.parseScheme(url)).toEqual(expectedScheme); } From 0fe3760fcff757872cb5b62891ef1b1d94e14978 Mon Sep 17 00:00:00 2001 From: Ali Ince Date: Thu, 21 Sep 2017 08:36:38 +0100 Subject: [PATCH 3/3] change forgotten fdescribe to describe --- test/internal/util.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/internal/util.test.js b/test/internal/util.test.js index de9de43f9..56cabe389 100644 --- a/test/internal/util.test.js +++ b/test/internal/util.test.js @@ -19,7 +19,7 @@ import * as util from '../../src/v1/internal/util'; -fdescribe('util', () => { +describe('util', () => { it('should check empty objects', () => { expect(util.isEmptyObjectOrNull(null)).toBeTruthy();