Skip to content

Commit b1733e5

Browse files
lutovichlutovich
committed
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.
1 parent 41e459a commit b1733e5

File tree

7 files changed

+207
-35
lines changed

7 files changed

+207
-35
lines changed

‎src/v1/internal/util.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@
1717
* limitations under the License.
1818
*/
1919

20+
import {isInt} from '../integer';
21+
2022
const ENCRYPTION_ON = "ENCRYPTION_ON";
2123
const ENCRYPTION_OFF = "ENCRYPTION_OFF";
2224

@@ -71,6 +73,20 @@ function assertString(obj, objName) {
7173
return obj;
7274
}
7375

76+
function assertNumber(obj, objName) {
77+
if (typeof obj !== 'number') {
78+
throw new TypeError(objName + ' expected to be a number but was: ' + JSON.stringify(obj));
79+
}
80+
return obj;
81+
}
82+
83+
function assertNumberOrInteger(obj, objName) {
84+
if (typeof obj !== 'number' && !isInt(obj)) {
85+
throw new TypeError(objName + ' expected to be either a number or an Integer object but was: ' + JSON.stringify(obj));
86+
}
87+
return obj;
88+
}
89+
7490
function assertCypherStatement(obj) {
7591
assertString(obj, 'Cypher statement');
7692
if (obj.trim().length === 0) {
@@ -94,6 +110,8 @@ export {
94110
isEmptyObjectOrNull,
95111
isString,
96112
assertString,
113+
assertNumber,
114+
assertNumberOrInteger,
97115
validateStatementAndParameters,
98116
ENCRYPTION_ON,
99117
ENCRYPTION_OFF

‎src/v1/spatial-types.js‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* See the License for the specific language governing permissions and
1717
* limitations under the License.
1818
*/
19+
import {assertNumber, assertNumberOrInteger} from './internal/util';
1920

2021
const POINT_IDENTIFIER_PROPERTY = '__isPoint__';
2122

@@ -33,10 +34,10 @@ export class Point {
3334
* @param {number} [z=undefined] the <code>y</code> coordinate of the point or <code>undefined</code> if point has 2 dimensions.
3435
*/
3536
constructor(srid, x, y, z) {
36-
this.srid = srid;
37-
this.x = x;
38-
this.y = y;
39-
this.z = z;
37+
this.srid = assertNumberOrInteger(srid, 'SRID');
38+
this.x = assertNumber(x, 'X coordinate');
39+
this.y = assertNumber(y, 'Y coordinate');
40+
this.z = (z === null || z === undefined) ? z : assertNumber(z, 'Z coordinate');
4041
Object.freeze(this);
4142
}
4243

‎src/v1/temporal-types.js‎

Lines changed: 33 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919

2020
import * as util from './internal/temporal-util';
21+
import {assertNumberOrInteger, assertString} from './internal/util';
2122
import {newError} from './error';
2223

2324
const IDENTIFIER_PROPERTY_ATTRIBUTES = {
@@ -47,8 +48,10 @@ export class Duration {
4748
* @param {Integer|number} nanoseconds the number of nanoseconds for the new duration.
4849
*/
4950
constructor(months, days, seconds, nanoseconds) {
50-
this.months = months;
51-
this.days = days;
51+
this.months = assertNumberOrInteger(months, 'Months');
52+
this.days = assertNumberOrInteger(days, 'Days');
53+
assertNumberOrInteger(seconds, 'Seconds');
54+
assertNumberOrInteger(nanoseconds, 'Nanoseconds');
5255
this.seconds = util.normalizeSecondsForDuration(seconds, nanoseconds);
5356
this.nanoseconds = util.normalizeNanosecondsForDuration(nanoseconds);
5457
Object.freeze(this);
@@ -84,10 +87,10 @@ export class LocalTime {
8487
* @param {Integer|number} nanosecond the nanosecond for the new local time.
8588
*/
8689
constructor(hour, minute, second, nanosecond) {
87-
this.hour = hour;
88-
this.minute = minute;
89-
this.second = second;
90-
this.nanosecond = nanosecond;
90+
this.hour = assertNumberOrInteger(hour, 'Hour');
91+
this.minute = assertNumberOrInteger(minute, 'Minute');
92+
this.second = assertNumberOrInteger(second, 'Second');
93+
this.nanosecond = assertNumberOrInteger(nanosecond, 'Nanosecond');
9194
Object.freeze(this);
9295
}
9396

@@ -122,11 +125,11 @@ export class Time {
122125
* @param {Integer|number} timeZoneOffsetSeconds the time zone offset in seconds.
123126
*/
124127
constructor(hour, minute, second, nanosecond, timeZoneOffsetSeconds) {
125-
this.hour = hour;
126-
this.minute = minute;
127-
this.second = second;
128-
this.nanosecond = nanosecond;
129-
this.timeZoneOffsetSeconds = timeZoneOffsetSeconds;
128+
this.hour = assertNumberOrInteger(hour, 'Hour');
129+
this.minute = assertNumberOrInteger(minute, 'Minute');
130+
this.second = assertNumberOrInteger(second, 'Second');
131+
this.nanosecond = assertNumberOrInteger(nanosecond, 'Nanosecond');
132+
this.timeZoneOffsetSeconds = assertNumberOrInteger(timeZoneOffsetSeconds, 'Time zone offset in seconds');
130133
Object.freeze(this);
131134
}
132135

@@ -159,9 +162,9 @@ export class Date {
159162
* @param {Integer|number} day the day for the new local date.
160163
*/
161164
constructor(year, month, day) {
162-
this.year = year;
163-
this.month = month;
164-
this.day = day;
165+
this.year = assertNumberOrInteger(year, 'Year');
166+
this.month = assertNumberOrInteger(month, 'Month');
167+
this.day = assertNumberOrInteger(day, 'Day');
165168
Object.freeze(this);
166169
}
167170

@@ -198,13 +201,13 @@ export class LocalDateTime {
198201
* @param {Integer|number} nanosecond the nanosecond for the new local time.
199202
*/
200203
constructor(year, month, day, hour, minute, second, nanosecond) {
201-
this.year = year;
202-
this.month = month;
203-
this.day = day;
204-
this.hour = hour;
205-
this.minute = minute;
206-
this.second = second;
207-
this.nanosecond = nanosecond;
204+
this.year = assertNumberOrInteger(year, 'Year');
205+
this.month = assertNumberOrInteger(month, 'Month');
206+
this.day = assertNumberOrInteger(day, 'Day');
207+
this.hour = assertNumberOrInteger(hour, 'Hour');
208+
this.minute = assertNumberOrInteger(minute, 'Minute');
209+
this.second = assertNumberOrInteger(second, 'Second');
210+
this.nanosecond = assertNumberOrInteger(nanosecond, 'Nanosecond');
208211
Object.freeze(this);
209212
}
210213

@@ -243,13 +246,13 @@ export class DateTime {
243246
* @param {string|null} timeZoneId the time zone id for the new date-time. Either this argument or <code>timeZoneOffsetSeconds</code> should be defined.
244247
*/
245248
constructor(year, month, day, hour, minute, second, nanosecond, timeZoneOffsetSeconds, timeZoneId) {
246-
this.year = year;
247-
this.month = month;
248-
this.day = day;
249-
this.hour = hour;
250-
this.minute = minute;
251-
this.second = second;
252-
this.nanosecond = nanosecond;
249+
this.year = assertNumberOrInteger(year, 'Year');
250+
this.month = assertNumberOrInteger(month, 'Month');
251+
this.day = assertNumberOrInteger(day, 'Day');
252+
this.hour = assertNumberOrInteger(hour, 'Hour');
253+
this.minute = assertNumberOrInteger(minute, 'Minute');
254+
this.second = assertNumberOrInteger(second, 'Second');
255+
this.nanosecond = assertNumberOrInteger(nanosecond, 'Nanosecond');
253256

254257
const [offset, id] = verifyTimeZoneArguments(timeZoneOffsetSeconds, timeZoneId);
255258
this.timeZoneOffsetSeconds = offset;
@@ -289,8 +292,10 @@ function verifyTimeZoneArguments(timeZoneOffsetSeconds, timeZoneId) {
289292
const idDefined = timeZoneId && timeZoneId !== '';
290293

291294
if (offsetDefined && !idDefined) {
295+
assertNumberOrInteger(timeZoneOffsetSeconds, 'Time zone offset in seconds');
292296
return [timeZoneOffsetSeconds, null];
293297
} else if (!offsetDefined && idDefined) {
298+
assertString(timeZoneId, 'Time zone ID');
294299
return [null, timeZoneId];
295300
} else if (offsetDefined && idDefined) {
296301
throw newError(`Unable to create DateTime with both time zone offset and id. Please specify either of them. Given offset: ${timeZoneOffsetSeconds} and id: ${timeZoneId}`);

‎test/internal/http/http-driver.test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -346,7 +346,7 @@ describe('http driver', () => {
346346
return;
347347
}
348348

349-
testUnsupportedQueryParameterWithHttpDriver(new neo4j.types.LocalDateTime(2000, 10, 12, 10, 10, 10), done);
349+
testUnsupportedQueryParameterWithHttpDriver(new neo4j.types.LocalDateTime(2000, 10, 12, 10, 10, 10, 10), done);
350350
});
351351

352352
it('should fail to pass local time as a query parameter', done => {

‎test/internal/util.test.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
*/
1919

2020
import * as util from '../../src/v1/internal/util';
21+
import {int} from '../../src/v1';
2122

2223
describe('util', () => {
2324

@@ -93,6 +94,47 @@ describe('util', () => {
9394
verifyInvalidQueryParameters(() => null);
9495
});
9596

97+
it('should check numbers and integers', () => {
98+
verifyValidNumber(0);
99+
verifyValidNumber(1);
100+
verifyValidNumber(42);
101+
verifyValidNumber(-42);
102+
verifyValidNumber(12.001);
103+
verifyValidNumber(-493.423);
104+
105+
verifyInvalidNumber(int(0));
106+
verifyInvalidNumber(int(1));
107+
verifyInvalidNumber(int(147123));
108+
verifyInvalidNumber('');
109+
verifyInvalidNumber('42');
110+
verifyInvalidNumber([]);
111+
verifyInvalidNumber([42]);
112+
verifyInvalidNumber({});
113+
verifyInvalidNumber({value: 42});
114+
});
115+
116+
it('should check numbers and integers', () => {
117+
verifyValidNumberOrInteger(0);
118+
verifyValidNumberOrInteger(1);
119+
verifyValidNumberOrInteger(42);
120+
verifyValidNumberOrInteger(-42);
121+
verifyValidNumberOrInteger(12.001);
122+
verifyValidNumberOrInteger(-493.423);
123+
124+
verifyValidNumberOrInteger(int(0));
125+
verifyValidNumberOrInteger(int(42));
126+
verifyValidNumberOrInteger(int(1241));
127+
verifyValidNumberOrInteger(int(441));
128+
verifyValidNumberOrInteger(int(-100500));
129+
130+
verifyInvalidNumberOrInteger('');
131+
verifyInvalidNumberOrInteger('42');
132+
verifyInvalidNumberOrInteger([]);
133+
verifyInvalidNumberOrInteger([42]);
134+
verifyInvalidNumberOrInteger({});
135+
verifyInvalidNumberOrInteger({value: 42});
136+
});
137+
96138
function verifyValidString(str) {
97139
expect(util.assertString(str, 'Test string')).toBe(str);
98140
}
@@ -101,6 +143,22 @@ describe('util', () => {
101143
expect(() => util.assertString(str, 'Test string')).toThrowError(TypeError);
102144
}
103145

146+
function verifyValidNumber(obj) {
147+
expect(util.assertNumber(obj, 'Test number')).toBe(obj);
148+
}
149+
150+
function verifyInvalidNumber(obj) {
151+
expect(() => util.assertNumber(obj, 'Test number')).toThrowError(TypeError);
152+
}
153+
154+
function verifyValidNumberOrInteger(obj) {
155+
expect(util.assertNumberOrInteger(obj, 'Test object')).toEqual(obj);
156+
}
157+
158+
function verifyInvalidNumberOrInteger(obj) {
159+
expect(() => util.assertNumberOrInteger(obj, 'Test object')).toThrowError(TypeError);
160+
}
161+
104162
function verifyInvalidCypherStatement(str) {
105163
expect(() => util.validateStatementAndParameters(str, {})).toThrowError(TypeError);
106164
}

‎test/v1/spatial-types.test.js‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
* limitations under the License.
1818
*/
1919

20-
import neo4j from '../../src/v1';
20+
import neo4j, {int} from '../../src/v1';
2121
import sharedNeo4j from '../internal/shared-neo4j';
2222
import {ServerVersion, VERSION_3_4_0} from '../../src/v1/internal/server-version';
2323
import {isPoint, Point} from '../../src/v1/spatial-types';
@@ -210,6 +210,35 @@ describe('spatial-types', () => {
210210
expect(point6.toString()).toEqual('Point{srid=7203, x=23.9378123, y=67.3891}');
211211
});
212212

213+
it('should validate types of constructor arguments for Point', () => {
214+
expect(() => new Point('1', 2, 3)).toThrowError(TypeError);
215+
expect(() => new Point(1, '2', 3)).toThrowError(TypeError);
216+
expect(() => new Point(1, 2, '3')).toThrowError(TypeError);
217+
expect(() => new Point(1, 2, '3')).toThrowError(TypeError);
218+
219+
expect(() => new Point('1', 2, 3, null)).toThrowError(TypeError);
220+
expect(() => new Point(1, '2', 3, null)).toThrowError(TypeError);
221+
expect(() => new Point(1, 2, '3', null)).toThrowError(TypeError);
222+
expect(() => new Point(1, 2, '3', null)).toThrowError(TypeError);
223+
224+
expect(() => new Point('1', 2, 3, 4)).toThrowError(TypeError);
225+
expect(() => new Point(1, '2', 3, 4)).toThrowError(TypeError);
226+
expect(() => new Point(1, 2, '3', 4)).toThrowError(TypeError);
227+
expect(() => new Point(1, 2, '3', 4)).toThrowError(TypeError);
228+
229+
expect(() => new Point(1, int(2), 3, 4)).toThrowError(TypeError);
230+
expect(() => new Point(1, 2, int(3), 4)).toThrowError(TypeError);
231+
expect(() => new Point(1, 2, 3, int(4))).toThrowError(TypeError);
232+
233+
expect(new Point(1, 2, 3, 4)).toBeDefined();
234+
235+
expect(new Point(int(1), 2, 3, undefined)).toBeDefined();
236+
expect(new Point(1, 2, 3, undefined)).toBeDefined();
237+
238+
expect(new Point(1, 2, 3, null)).toBeDefined();
239+
expect(new Point(int(1), 2, 3, null)).toBeDefined();
240+
});
241+
213242
function testReceivingOfPoints(done, query, pointChecker) {
214243
if (neo4jDoesNotSupportPoints(done)) {
215244
return;

‎test/v1/temporal-types.test.js‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -513,7 +513,7 @@ describe('temporal-types', () => {
513513
});
514514

515515
it('should expose local date-time components in date-time with zone ID', () => {
516-
const zonedDateTime = dateTimeWithZoneId(2356, 7, 29, 23, 32, 11, 9346458, 3600, randomZoneId());
516+
const zonedDateTime = dateTimeWithZoneId(2356, 7, 29, 23, 32, 11, 9346458, randomZoneId());
517517

518518
expect(zonedDateTime.year).toEqual(neo4j.int(2356));
519519
expect(zonedDateTime.month).toEqual(neo4j.int(7));
@@ -597,6 +597,67 @@ describe('temporal-types', () => {
597597
expect(duration6.nanoseconds).toEqual(neo4j.int(876543001));
598598
});
599599

600+
it('should validate types of constructor arguments for Duration', () => {
601+
expect(() => new neo4j.types.Duration('1', 2, 3, 4)).toThrowError(TypeError);
602+
expect(() => new neo4j.types.Duration(1, '2', 3, 4)).toThrowError(TypeError);
603+
expect(() => new neo4j.types.Duration(1, 2, [3], 4)).toThrowError(TypeError);
604+
expect(() => new neo4j.types.Duration(1, 2, 3, {value: 4})).toThrowError(TypeError);
605+
expect(() => new neo4j.types.Duration({months: 1, days: 2, seconds: 3, nanoseconds: 4})).toThrowError(TypeError);
606+
});
607+
608+
it('should validate types of constructor arguments for LocalTime', () => {
609+
expect(() => new neo4j.types.LocalTime('1', 2, 3, 4)).toThrowError(TypeError);
610+
expect(() => new neo4j.types.LocalTime(1, '2', 3, 4)).toThrowError(TypeError);
611+
expect(() => new neo4j.types.LocalTime(1, 2, [3], 4)).toThrowError(TypeError);
612+
expect(() => new neo4j.types.LocalTime(1, 2, 3, {value: 4})).toThrowError(TypeError);
613+
expect(() => new neo4j.types.LocalTime({hour: 1, minute: 2, seconds: 3, nanosecond: 4})).toThrowError(TypeError);
614+
});
615+
616+
it('should validate types of constructor arguments for Time', () => {
617+
expect(() => new neo4j.types.Time('1', 2, 3, 4, 5)).toThrowError(TypeError);
618+
expect(() => new neo4j.types.Time(1, '2', 3, 4, 5)).toThrowError(TypeError);
619+
expect(() => new neo4j.types.Time(1, 2, [3], 4, 5)).toThrowError(TypeError);
620+
expect(() => new neo4j.types.Time(1, 2, 3, {value: 4}, 5)).toThrowError(TypeError);
621+
expect(() => new neo4j.types.Time(1, 2, 3, 4, () => 5)).toThrowError(TypeError);
622+
expect(() => new neo4j.types.Time({hour: 1, minute: 2, seconds: 3, nanosecond: 4, timeZoneOffsetSeconds: 5})).toThrowError(TypeError);
623+
});
624+
625+
it('should validate types of constructor arguments for Date', () => {
626+
expect(() => new neo4j.types.Date('1', 2, 3)).toThrowError(TypeError);
627+
expect(() => new neo4j.types.Date(1, [2], 3)).toThrowError(TypeError);
628+
expect(() => new neo4j.types.Date(1, 2, () => 3)).toThrowError(TypeError);
629+
expect(() => new neo4j.types.Date({year: 1, month: 2, day: 3})).toThrowError(TypeError);
630+
});
631+
632+
it('should validate types of constructor arguments for LocalDateTime', () => {
633+
expect(() => new neo4j.types.LocalDateTime('1', 2, 3, 4, 5, 6, 7)).toThrowError(TypeError);
634+
expect(() => new neo4j.types.LocalDateTime(1, '2', 3, 4, 5, 6, 7)).toThrowError(TypeError);
635+
expect(() => new neo4j.types.LocalDateTime(1, 2, [3], 4, 5, 6, 7)).toThrowError(TypeError);
636+
expect(() => new neo4j.types.LocalDateTime(1, 2, 3, [4], 5, 6, 7)).toThrowError(TypeError);
637+
expect(() => new neo4j.types.LocalDateTime(1, 2, 3, 4, () => 5, 6, 7)).toThrowError(TypeError);
638+
expect(() => new neo4j.types.LocalDateTime(1, 2, 3, 4, 5, () => 6, 7)).toThrowError(TypeError);
639+
expect(() => new neo4j.types.LocalDateTime(1, 2, 3, 4, 5, 6, {value: 7})).toThrowError(TypeError);
640+
expect(() => new neo4j.types.LocalDateTime({year: 1, month: 2, day: 3, hour: 4, minute: 5, second: 6, nanosecond: 7})).toThrowError(TypeError);
641+
});
642+
643+
it('should validate types of constructor arguments for DateTime', () => {
644+
expect(() => new neo4j.types.DateTime('1', 2, 3, 4, 5, 6, 7, 8)).toThrowError(TypeError);
645+
expect(() => new neo4j.types.DateTime(1, '2', 3, 4, 5, 6, 7, 8)).toThrowError(TypeError);
646+
expect(() => new neo4j.types.DateTime(1, 2, [3], 4, 5, 6, 7, 8)).toThrowError(TypeError);
647+
expect(() => new neo4j.types.DateTime(1, 2, 3, [4], 5, 6, 7, 8)).toThrowError(TypeError);
648+
expect(() => new neo4j.types.DateTime(1, 2, 3, 4, () => 5, 6, 7, 8)).toThrowError(TypeError);
649+
expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, () => 6, 7, 8)).toThrowError(TypeError);
650+
expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, {value: 7}, 8)).toThrowError(TypeError);
651+
expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, {value: 8})).toThrowError(TypeError);
652+
653+
expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, null, {timeZoneId: 'UK'})).toThrowError(TypeError);
654+
expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, null, ['UK'])).toThrowError(TypeError);
655+
656+
expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7)).toThrow();
657+
expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, null, null)).toThrow();
658+
expect(() => new neo4j.types.DateTime(1, 2, 3, 4, 5, 6, 7, 8, 'UK')).toThrow();
659+
});
660+
600661
function testSendAndReceiveRandomTemporalValues(valueGenerator, done) {
601662
const asyncFunction = (index, callback) => {
602663
const next = () => callback();

0 commit comments

Comments
 (0)