Correctly sanitize pool configs

Author: lutovich

src/v1/driver.js

@@ -25,7 +25,9 @@ import {newError, SERVICE_UNAVAILABLE} from './error';
 import {DirectConnectionProvider} from './internal/connection-providers';
 import Bookmark from './internal/bookmark';
 import ConnectivityVerifier from './internal/connectivity-verifier';
-import PoolConfig from './internal/pool-config';
+import PoolConfig, {DEFAULT_ACQUISITION_TIMEOUT, DEFAULT_MAX_SIZE} from './internal/pool-config';
+
+const DEFAULT_MAX_CONNECTION_LIFETIME = 60 * 60 * 1000; // 1 hour
 
 const READ = 'READ', WRITE = 'WRITE';
 /**
@@ -115,14 +117,8 @@ class Driver {
     }
 
     const maxConnectionLifetime = this._config.maxConnectionLifetime;
-    if (maxConnectionLifetime) {
-      const lifetime = Date.now() - conn.creationTimestamp;
-      if (lifetime > maxConnectionLifetime) {
-        return false;
-      }
-    }
-
-    return true;
+    const lifetime = Date.now() - conn.creationTimestamp;
+    return lifetime <= maxConnectionLifetime;
   }
 
   /**
@@ -238,19 +234,20 @@ class _ConnectionStreamObserver extends StreamObserver {
  * @private
  */
 function sanitizeConfig(config) {
-  config.maxConnectionLifetime = sanitizeIntValue(config.maxConnectionLifetime, 60*60*1000);
-  config.maxConnectionPoolSize = sanitizeIntValue(config.maxConnectionPoolSize);
-  config.connectionAcquisitionTimeout = sanitizeIntValue(config.connectionAcquisitionTimeout);
+  config.maxConnectionLifetime = sanitizeIntValue(config.maxConnectionLifetime, DEFAULT_MAX_CONNECTION_LIFETIME);
+  config.maxConnectionPoolSize = sanitizeIntValue(config.maxConnectionPoolSize, DEFAULT_MAX_SIZE);
+  config.connectionAcquisitionTimeout = sanitizeIntValue(config.connectionAcquisitionTimeout, DEFAULT_ACQUISITION_TIMEOUT);
 }
 
-function sanitizeIntValue(value, defaultValue=null) {
-  if (value) {
-      const sanitizedValue = parseInt(value, 10);
-      if (sanitizedValue && sanitizedValue > 0) {
-        return sanitizedValue;
-      }
+function sanitizeIntValue(rawValue, defaultWhenAbsent) {
+  const sanitizedValue = parseInt(rawValue, 10);
+  if (sanitizedValue > 0 || sanitizedValue === 0) {
+    return sanitizedValue;
+  } else if (sanitizedValue < 0) {
+    return Number.MAX_SAFE_INTEGER;
+  } else {
+    return defaultWhenAbsent;
   }
-  return defaultValue;
 }
 
 export {Driver, READ, WRITE}

src/v1/internal/pool-config.js

@@ -18,7 +18,7 @@
  */
 
 const DEFAULT_MAX_SIZE = 100;
-const DEFAULT_ACQUISITION_TIMEOUT = 60000;
+const DEFAULT_ACQUISITION_TIMEOUT = 60 * 1000; // 60 seconds
 
 export default class PoolConfig {
 

test/v1/driver.test.js

@@ -21,6 +21,7 @@ import neo4j from '../../src/v1';
 import sharedNeo4j from '../internal/shared-neo4j';
 import FakeConnection from '../internal/fake-connection';
 import lolex from 'lolex';
+import {DEFAULT_ACQUISITION_TIMEOUT, DEFAULT_MAX_SIZE} from '../../src/v1/internal/pool-config';
 
 describe('driver', () => {
 
@@ -191,13 +192,10 @@ describe('driver', () => {
     expect(() => neo4j.driver('bolt://localhost:7687/?policy=my_policy')).toThrow();
   });
 
-  it('should sanitize maxConnectionLifetime in the config', () => {
-    validateMaxConnectionLifetime({}, null);
-    validateMaxConnectionLifetime({maxConnectionLifetime: 42}, 42);
-    validateMaxConnectionLifetime({maxConnectionLifetime: 0}, null);
-    validateMaxConnectionLifetime({maxConnectionLifetime: '42'}, 42);
-    validateMaxConnectionLifetime({maxConnectionLifetime: '042'}, 42);
-    validateMaxConnectionLifetime({maxConnectionLifetime: -42}, null);
+  it('should sanitize pool setting values in the config', () => {
+    testConfigSanitizing('maxConnectionLifetime', 60 * 60 * 1000);
+    testConfigSanitizing('maxConnectionPoolSize', DEFAULT_MAX_SIZE);
+    testConfigSanitizing('connectionAcquisitionTimeout', DEFAULT_ACQUISITION_TIMEOUT);
   });
 
   it('should treat closed connections as invalid', () => {
@@ -321,10 +319,19 @@ describe('driver', () => {
     return neo4j.auth.basic('neo4j', 'who would use such a password');
   }
 
-  function validateMaxConnectionLifetime(config, expectedValue) {
+  function testConfigSanitizing(configProperty, defaultValue) {
+    validateConfigSanitizing({}, defaultValue);
+    validateConfigSanitizing({[configProperty]: 42}, 42);
+    validateConfigSanitizing({[configProperty]: 0}, 0);
+    validateConfigSanitizing({[configProperty]: '42'}, 42);
+    validateConfigSanitizing({[configProperty]: '042'}, 42);
+    validateConfigSanitizing({[configProperty]: -42}, Number.MAX_SAFE_INTEGER);
+  }
+
+  function validateConfigSanitizing(config, configProperty, expectedValue) {
     const driver = neo4j.driver('bolt://localhost', sharedNeo4j.authToken, config);
     try {
-      expect(driver._config.maxConnectionLifetime).toEqual(expectedValue);
+      expect(driver._config[configProperty]).toEqual(expectedValue);
     } finally {
       driver.close();
     }