From b1733e51d6496f772e62834d9029b3e66d57f5c8 Mon Sep 17 00:00:00 2001 From: lutovich Date: Fri, 15 Jun 2018 17:45:25 +0200 Subject: [PATCH] Check parameter types in spatial and temporal types This commit adds type assertions for constructor parameters of all spatial and temporal types. Assertions make it easier to detect incorrect usage and packstream layer from serializing illegal structs. --- src/v1/internal/util.js | 18 ++++++++ src/v1/spatial-types.js | 9 ++-- src/v1/temporal-types.js | 61 +++++++++++++------------ test/internal/http/http-driver.test.js | 2 +- test/internal/util.test.js | 58 ++++++++++++++++++++++++ test/v1/spatial-types.test.js | 31 ++++++++++++- test/v1/temporal-types.test.js | 63 +++++++++++++++++++++++++- 7 files changed, 207 insertions(+), 35 deletions(-) diff --git a/src/v1/internal/util.js b/src/v1/internal/util.js index 640d544d8..437be20be 100644 --- a/src/v1/internal/util.js +++ b/src/v1/internal/util.js @@ -17,6 +17,8 @@ * limitations under the License. */ +import {isInt} from '../integer'; + const ENCRYPTION_ON = "ENCRYPTION_ON"; const ENCRYPTION_OFF = "ENCRYPTION_OFF"; @@ -71,6 +73,20 @@ function assertString(obj, objName) { return obj; } +function assertNumber(obj, objName) { + if (typeof obj !== 'number') { + throw new TypeError(objName + ' expected to be a number but was: ' + JSON.stringify(obj)); + } + return obj; +} + +function assertNumberOrInteger(obj, objName) { + if (typeof obj !== 'number' && !isInt(obj)) { + throw new TypeError(objName + ' expected to be either a number or an Integer object but was: ' + JSON.stringify(obj)); + } + return obj; +} + function assertCypherStatement(obj) { assertString(obj, 'Cypher statement'); if (obj.trim().length === 0) { @@ -94,6 +110,8 @@ export { isEmptyObjectOrNull, isString, assertString, + assertNumber, + assertNumberOrInteger, validateStatementAndParameters, ENCRYPTION_ON, ENCRYPTION_OFF diff --git a/src/v1/spatial-types.js b/src/v1/spatial-types.js index ca7efc0c7..9140a00ff 100644 --- a/src/v1/spatial-types.js +++ b/src/v1/spatial-types.js @@ -16,6 +16,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +import {assertNumber, assertNumberOrInteger} from './internal/util'; const POINT_IDENTIFIER_PROPERTY = '__isPoint__'; @@ -33,10 +34,10 @@ export class Point { * @param {number} [z=undefined] the y coordinate of the point or undefined if point has 2 dimensions. */ constructor(srid, x, y, z) { - this.srid = srid; - this.x = x; - this.y = y; - this.z = z; + this.srid = assertNumberOrInteger(srid, 'SRID'); + this.x = assertNumber(x, 'X coordinate'); + this.y = assertNumber(y, 'Y coordinate'); + this.z = (z === null || z === undefined) ? z : assertNumber(z, 'Z coordinate'); Object.freeze(this); } diff --git a/src/v1/temporal-types.js b/src/v1/temporal-types.js index 0cb4a9ea7..0ca11b7df 100644 --- a/src/v1/temporal-types.js +++ b/src/v1/temporal-types.js @@ -18,6 +18,7 @@ */ import * as util from './internal/temporal-util'; +import {assertNumberOrInteger, assertString} from './internal/util'; import {newError} from './error'; const IDENTIFIER_PROPERTY_ATTRIBUTES = { @@ -47,8 +48,10 @@ export class Duration { * @param {Integer|number} nanoseconds the number of nanoseconds for the new duration. */ constructor(months, days, seconds, nanoseconds) { - this.months = months; - this.days = days; + this.months = assertNumberOrInteger(months, 'Months'); + this.days = assertNumberOrInteger(days, 'Days'); + assertNumberOrInteger(seconds, 'Seconds'); + assertNumberOrInteger(nanoseconds, 'Nanoseconds'); this.seconds = util.normalizeSecondsForDuration(seconds, nanoseconds); this.nanoseconds = util.normalizeNanosecondsForDuration(nanoseconds); Object.freeze(this); @@ -84,10 +87,10 @@ export class LocalTime { * @param {Integer|number} nanosecond the nanosecond for the new local time. */ constructor(hour, minute, second, nanosecond) { - this.hour = hour; - this.minute = minute; - this.second = second; - this.nanosecond = nanosecond; + this.hour = assertNumberOrInteger(hour, 'Hour'); + this.minute = assertNumberOrInteger(minute, 'Minute'); + this.second = assertNumberOrInteger(second, 'Second'); + this.nanosecond = assertNumberOrInteger(nanosecond, 'Nanosecond'); Object.freeze(this); } @@ -122,11 +125,11 @@ export class Time { * @param {Integer|number} timeZoneOffsetSeconds the time zone offset in seconds. */ constructor(hour, minute, second, nanosecond, timeZoneOffsetSeconds) { - this.hour = hour; - this.minute = minute; - this.second = second; - this.nanosecond = nanosecond; - this.timeZoneOffsetSeconds = timeZoneOffsetSeconds; + this.hour = assertNumberOrInteger(hour, 'Hour'); + this.minute = assertNumberOrInteger(minute, 'Minute'); + this.second = assertNumberOrInteger(second, 'Second'); + this.nanosecond = assertNumberOrInteger(nanosecond, 'Nanosecond'); + this.timeZoneOffsetSeconds = assertNumberOrInteger(timeZoneOffsetSeconds, 'Time zone offset in seconds'); Object.freeze(this); } @@ -159,9 +162,9 @@ export class Date { * @param {Integer|number} day the day for the new local date. */ constructor(year, month, day) { - this.year = year; - this.month = month; - this.day = day; + this.year = assertNumberOrInteger(year, 'Year'); + this.month = assertNumberOrInteger(month, 'Month'); + this.day = assertNumberOrInteger(day, 'Day'); Object.freeze(this); } @@ -198,13 +201,13 @@ export class LocalDateTime { * @param {Integer|number} nanosecond the nanosecond for the new local time. */ constructor(year, month, day, hour, minute, second, nanosecond) { - this.year = year; - this.month = month; - this.day = day; - this.hour = hour; - this.minute = minute; - this.second = second; - this.nanosecond = nanosecond; + this.year = assertNumberOrInteger(year, 'Year'); + this.month = assertNumberOrInteger(month, 'Month'); + this.day = assertNumberOrInteger(day, 'Day'); + this.hour = assertNumberOrInteger(hour, 'Hour'); + this.minute = assertNumberOrInteger(minute, 'Minute'); + this.second = assertNumberOrInteger(second, 'Second'); + this.nanosecond = assertNumberOrInteger(nanosecond, 'Nanosecond'); Object.freeze(this); } @@ -243,13 +246,13 @@ export class DateTime { * @param {string|null} timeZoneId the time zone id for the new date-time. Either this argument or timeZoneOffsetSeconds should be defined. */ constructor(year, month, day, hour, minute, second, nanosecond, timeZoneOffsetSeconds, timeZoneId) { - this.year = year; - this.month = month; - this.day = day; - this.hour = hour; - this.minute = minute; - this.second = second; - this.nanosecond = nanosecond; + this.year = assertNumberOrInteger(year, 'Year'); + this.month = assertNumberOrInteger(month, 'Month'); + this.day = assertNumberOrInteger(day, 'Day'); + this.hour = assertNumberOrInteger(hour, 'Hour'); + this.minute = assertNumberOrInteger(minute, 'Minute'); + this.second = assertNumberOrInteger(second, 'Second'); + this.nanosecond = assertNumberOrInteger(nanosecond, 'Nanosecond'); const [offset, id] = verifyTimeZoneArguments(timeZoneOffsetSeconds, timeZoneId); this.timeZoneOffsetSeconds = offset; @@ -289,8 +292,10 @@ function verifyTimeZoneArguments(timeZoneOffsetSeconds, timeZoneId) { const idDefined = timeZoneId && timeZoneId !== ''; if (offsetDefined && !idDefined) { + assertNumberOrInteger(timeZoneOffsetSeconds, 'Time zone offset in seconds'); return [timeZoneOffsetSeconds, null]; } else if (!offsetDefined && idDefined) { + assertString(timeZoneId, 'Time zone ID'); return [null, timeZoneId]; } else if (offsetDefined && idDefined) { throw newError(`Unable to create DateTime with both time zone offset and id. Please specify either of them. Given offset: ${timeZoneOffsetSeconds} and id: ${timeZoneId}`); diff --git a/test/internal/http/http-driver.test.js b/test/internal/http/http-driver.test.js index 3d183b750..a82f1a054 100644 --- a/test/internal/http/http-driver.test.js +++ b/test/internal/http/http-driver.test.js @@ -346,7 +346,7 @@ describe('http driver', () => { return; } - testUnsupportedQueryParameterWithHttpDriver(new neo4j.types.LocalDateTime(2000, 10, 12, 10, 10, 10), done); + testUnsupportedQueryParameterWithHttpDriver(new neo4j.types.LocalDateTime(2000, 10, 12, 10, 10, 10, 10), done); }); it('should fail to pass local time as a query parameter', done => { diff --git a/test/internal/util.test.js b/test/internal/util.test.js index bd25fab32..58cc873d6 100644 --- a/test/internal/util.test.js +++ b/test/internal/util.test.js @@ -18,6 +18,7 @@ */ import * as util from '../../src/v1/internal/util'; +import {int} from '../../src/v1'; describe('util', () => { @@ -93,6 +94,47 @@ describe('util', () => { verifyInvalidQueryParameters(() => null); }); + it('should check numbers and integers', () => { + verifyValidNumber(0); + verifyValidNumber(1); + verifyValidNumber(42); + verifyValidNumber(-42); + verifyValidNumber(12.001); + verifyValidNumber(-493.423); + + verifyInvalidNumber(int(0)); + verifyInvalidNumber(int(1)); + verifyInvalidNumber(int(147123)); + verifyInvalidNumber(''); + verifyInvalidNumber('42'); + verifyInvalidNumber([]); + verifyInvalidNumber([42]); + verifyInvalidNumber({}); + verifyInvalidNumber({value: 42}); + }); + + it('should check numbers and integers', () => { + verifyValidNumberOrInteger(0); + verifyValidNumberOrInteger(1); + verifyValidNumberOrInteger(42); + verifyValidNumberOrInteger(-42); + verifyValidNumberOrInteger(12.001); + verifyValidNumberOrInteger(-493.423); + + verifyValidNumberOrInteger(int(0)); + verifyValidNumberOrInteger(int(42)); + verifyValidNumberOrInteger(int(1241)); + verifyValidNumberOrInteger(int(441)); + verifyValidNumberOrInteger(int(-100500)); + + verifyInvalidNumberOrInteger(''); + verifyInvalidNumberOrInteger('42'); + verifyInvalidNumberOrInteger([]); + verifyInvalidNumberOrInteger([42]); + verifyInvalidNumberOrInteger({}); + verifyInvalidNumberOrInteger({value: 42}); + }); + function verifyValidString(str) { expect(util.assertString(str, 'Test string')).toBe(str); } @@ -101,6 +143,22 @@ describe('util', () => { expect(() => util.assertString(str, 'Test string')).toThrowError(TypeError); } + function verifyValidNumber(obj) { + expect(util.assertNumber(obj, 'Test number')).toBe(obj); + } + + function verifyInvalidNumber(obj) { + expect(() => util.assertNumber(obj, 'Test number')).toThrowError(TypeError); + } + + function verifyValidNumberOrInteger(obj) { + expect(util.assertNumberOrInteger(obj, 'Test object')).toEqual(obj); + } + + function verifyInvalidNumberOrInteger(obj) { + expect(() => util.assertNumberOrInteger(obj, 'Test object')).toThrowError(TypeError); + } + function verifyInvalidCypherStatement(str) { expect(() => util.validateStatementAndParameters(str, {})).toThrowError(TypeError); } diff --git a/test/v1/spatial-types.test.js b/test/v1/spatial-types.test.js index 62dd7a439..5ae9d5578 100644 --- a/test/v1/spatial-types.test.js +++ b/test/v1/spatial-types.test.js @@ -17,7 +17,7 @@ * limitations under the License. */ -import neo4j from '../../src/v1'; +import neo4j, {int} from '../../src/v1'; import sharedNeo4j from '../internal/shared-neo4j'; import {ServerVersion, VERSION_3_4_0} from '../../src/v1/internal/server-version'; import {isPoint, Point} from '../../src/v1/spatial-types'; @@ -210,6 +210,35 @@ describe('spatial-types', () => { expect(point6.toString()).toEqual('Point{srid=7203, x=23.9378123, y=67.3891}'); }); + it('should validate types of constructor arguments for Point', () => { + expect(() => new Point('1', 2, 3)).toThrowError(TypeError); + expect(() => new Point(1, '2', 3)).toThrowError(TypeError); + expect(() => new Point(1, 2, '3')).toThrowError(TypeError); + expect(() => new Point(1, 2, '3')).toThrowError(TypeError); + + expect(() => new Point('1', 2, 3, null)).toThrowError(TypeError); + expect(() => new Point(1, '2', 3, null)).toThrowError(TypeError); + expect(() => new Point(1, 2, '3', null)).toThrowError(TypeError); + expect(() => new Point(1, 2, '3', null)).toThrowError(TypeError); + + expect(() => new Point('1', 2, 3, 4)).toThrowError(TypeError); + expect(() => new Point(1, '2', 3, 4)).toThrowError(TypeError); + expect(() => new Point(1, 2, '3', 4)).toThrowError(TypeError); + expect(() => new Point(1, 2, '3', 4)).toThrowError(TypeError); + + expect(() => new Point(1, int(2), 3, 4)).toThrowError(TypeError); + expect(() => new Point(1, 2, int(3), 4)).toThrowError(TypeError); + expect(() => new Point(1, 2, 3, int(4))).toThrowError(TypeError); + + expect(new Point(1, 2, 3, 4)).toBeDefined(); + + expect(new Point(int(1), 2, 3, undefined)).toBeDefined(); + expect(new Point(1, 2, 3, undefined)).toBeDefined(); + + expect(new Point(1, 2, 3, null)).toBeDefined(); + expect(new Point(int(1), 2, 3, null)).toBeDefined(); + }); + function testReceivingOfPoints(done, query, pointChecker) { if (neo4jDoesNotSupportPoints(done)) { return; diff --git a/test/v1/temporal-types.test.js b/test/v1/temporal-types.test.js index d764ef07c..cbc926f12 100644 --- a/test/v1/temporal-types.test.js +++ b/test/v1/temporal-types.test.js @@ -513,7 +513,7 @@ describe('temporal-types', () => { }); it('should expose local date-time components in date-time with zone ID', () => { - const zonedDateTime = dateTimeWithZoneId(2356, 7, 29, 23, 32, 11, 9346458, 3600, randomZoneId()); + const zonedDateTime = dateTimeWithZoneId(2356, 7, 29, 23, 32, 11, 9346458, randomZoneId()); expect(zonedDateTime.year).toEqual(neo4j.int(2356)); expect(zonedDateTime.month).toEqual(neo4j.int(7)); @@ -597,6 +597,67 @@ describe('temporal-types', () => { expect(duration6.nanoseconds).toEqual(neo4j.int(876543001)); }); + it('should validate types of constructor arguments for Duration', () => { + expect(() => new neo4j.types.Duration('1', 2, 3, 4)).toThrowError(TypeError); + expect(() => new neo4j.types.Duration(1, '2', 3, 4)).toThrowError(TypeError); + expect(() => new neo4j.types.Duration(1, 2, [3], 4)).toThrowError(TypeError); + expect(() => new neo4j.types.Duration(1, 2, 3, {value: 4})).toThrowError(TypeError); + expect(() => new neo4j.types.Duration({months: 1, days: 2, seconds: 3, nanoseconds: 4})).toThrowError(TypeError); + }); + + it('should validate types of constructor arguments for LocalTime', () => { + expect(() => new neo4j.types.LocalTime('1', 2, 3, 4)).toThrowError(TypeError); + expect(() => new neo4j.types.LocalTime(1, '2', 3, 4)).toThrowError(TypeError); + expect(() => new neo4j.types.LocalTime(1, 2, [3], 4)).toThrowError(TypeError); + expect(() => new neo4j.types.LocalTime(1, 2, 3, {value: 4})).toThrowError(TypeError); + expect(() => new neo4j.types.LocalTime({hour: 1, minute: 2, seconds: 3, nanosecond: 4})).toThrowError(TypeError); + }); + + it('should validate types of constructor arguments for Time', () => { + expect(() => new neo4j.types.Time('1', 2, 3, 4, 5)).toThrowError(TypeError); + expect(() => new neo4j.types.Time(1, '2', 3, 4, 5)).toThrowError(TypeError); + expect(() => new neo4j.types.Time(1, 2, [3], 4, 5)).toThrowError(TypeError); + expect(() => new neo4j.types.Time(1, 2, 3, {value: 4}, 5)).toThrowError(TypeError); + expect(() => new neo4j.types.Time(1, 2, 3, 4, () => 5)).toThrowError(TypeError); + expect(() => new neo4j.types.Time({hour: 1, minute: 2, seconds: 3, nanosecond: 4, timeZoneOffsetSeconds: 5})).toThrowError(TypeError); + }); + + it('should validate types of constructor arguments for Date', () => { + expect(() => new neo4j.types.Date('1', 2, 3)).toThrowError(TypeError); + expect(() => new neo4j.types.Date(1, [2], 3)).toThrowError(TypeError); + expect(() => new neo4j.types.Date(1, 2, () => 3)).toThrowError(TypeError); + expect(() => new neo4j.types.Date({year: 1, month: 2, day: 3})).toThrowError(TypeError); + }); + + it('should validate types of constructor arguments for LocalDateTime', () => { + expect(() => new neo4j.types.LocalDateTime('1', 2, 3, 4, 5, 6, 7)).toThrowError(TypeError); + expect(() => new neo4j.types.LocalDateTime(1, '2', 3, 4, 5, 6, 7)).toThrowError(TypeError); + expect(() => new neo4j.types.LocalDateTime(1, 2, [3], 4, 5, 6, 7)).toThrowError(TypeError); + expect(() => new neo4j.types.LocalDateTime(1, 2, 3, [4], 5, 6, 7)).toThrowError(TypeError); + expect(() => new neo4j.types.LocalDateTime(1, 2, 3, 4, () => 5, 6, 7)).toThrowError(TypeError); + expect(() => new neo4j.types.LocalDateTime(1, 2, 3, 4, 5, () => 6, 7)).toThrowError(TypeError); + expect(() => new neo4j.types.LocalDateTime(1, 2, 3, 4, 5, 6, {value: 7})).toThrowError(TypeError); + expect(() => new neo4j.types.LocalDateTime({year: 1, month: 2, day: 3, hour: 4, minute: 5, second: 6, nanosecond: 7})).toThrowError(TypeError); + }); + + it('should validate types of constructor arguments for DateTime', () => { + expect(() => new neo4j.types.DateTime('1', 2, 3, 4, 5, 6, 7, 8)).toThrowError(TypeError); + expect(() => new neo4j.types.DateTime(1, '2', 3, 4, 5, 6, 7, 8)).toThrowError(TypeError); + expect(() => new neo4j.types.DateTime(1, 2, [3], 4, 5, 6, 7, 8)).toThrowError(TypeError); + expect(() => new neo4j.types.DateTime(1, 2, 3, [4], 5, 6, 7, 8)).toThrowError(TypeError); + expect(() => new neo4j.types.DateTime(1, 2, 3, 4, () => 5, 6, 7, 8)).toThrowError(TypeError); + expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, () => 6, 7, 8)).toThrowError(TypeError); + expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, {value: 7}, 8)).toThrowError(TypeError); + expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, {value: 8})).toThrowError(TypeError); + + expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, null, {timeZoneId: 'UK'})).toThrowError(TypeError); + expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, null, ['UK'])).toThrowError(TypeError); + + expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7)).toThrow(); + expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, null, null)).toThrow(); + expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, 8, 'UK')).toThrow(); + }); + function testSendAndReceiveRandomTemporalValues(valueGenerator, done) { const asyncFunction = (index, callback) => { const next = () => callback();