diff --git a/package-lock.json b/package-lock.json index fd0262cfe..3d5218ae3 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8233,11 +8233,6 @@ "integrity": "sha1-nsYfeQSYdXB9aUFFlv2Qek1xHnM=", "dev": true }, - "querystringify": { - "version": "1.0.0", - "resolved": "https://registry.npmjs.org/querystringify/-/querystringify-1.0.0.tgz", - "integrity": "sha1-YoYkIRLFtxL6ZU5SZlK/ahP/Bcs=" - }, "randomatic": { "version": "1.1.7", "resolved": "https://registry.npmjs.org/randomatic/-/randomatic-1.1.7.tgz", @@ -8554,7 +8549,8 @@ "requires-port": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/requires-port/-/requires-port-1.0.0.tgz", - "integrity": "sha1-kl0mAdOaxIXgkc8NpcbmlNw9yv8=" + "integrity": "sha1-kl0mAdOaxIXgkc8NpcbmlNw9yv8=", + "dev": true }, "resolve": { "version": "1.5.0", @@ -9932,6 +9928,21 @@ } } }, + "uri-js": { + "version": "4.2.1", + "resolved": "https://registry.npmjs.org/uri-js/-/uri-js-4.2.1.tgz", + "integrity": "sha512-jpKCA3HjsBfSDOEgxRDAxQCNyHfCPSbq57PqCkd3gAyBuPb3IWxw54EHncqESznIdqSetHfw3D7ylThu2Kcc9A==", + "requires": { + "punycode": "2.1.0" + }, + "dependencies": { + "punycode": { + "version": "2.1.0", + "resolved": "https://registry.npmjs.org/punycode/-/punycode-2.1.0.tgz", + "integrity": "sha1-X4Y+3Im5bbCQdLrXlHvwkFbKTn0=" + } + } + }, "urix": { "version": "0.1.0", "resolved": "https://registry.npmjs.org/urix/-/urix-0.1.0.tgz", @@ -9956,15 +9967,6 @@ } } }, - "url-parse": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/url-parse/-/url-parse-1.2.0.tgz", - "integrity": "sha512-DT1XbYAfmQP65M/mE6OALxmXzZ/z1+e5zk2TcSKe/KiYbNGZxgtttzC0mR/sjopbpOXcbniq7eIKmocJnUWlEw==", - "requires": { - "querystringify": "1.0.0", - "requires-port": "1.0.0" - } - }, "use": { "version": "2.0.2", "resolved": "https://registry.npmjs.org/use/-/use-2.0.2.tgz", diff --git a/package.json b/package.json index 41829beb5..f98150c40 100644 --- a/package.json +++ b/package.json @@ -73,6 +73,6 @@ }, "dependencies": { "babel-runtime": "^6.18.0", - "url-parse": "^1.2.0" + "uri-js": "^4.2.1" } } diff --git a/src/v1/driver.js b/src/v1/driver.js index 2351f7b0b..fcbc781c6 100644 --- a/src/v1/driver.js +++ b/src/v1/driver.js @@ -57,16 +57,16 @@ class Driver { /** * You should not be calling this directly, instead use {@link driver}. * @constructor - * @param {string} url + * @param {string} hostPort * @param {string} userAgent * @param {object} token * @param {object} config * @protected */ - constructor(url, userAgent, token = {}, config = {}) { + constructor(hostPort, userAgent, token = {}, config = {}) { sanitizeConfig(config); - this._url = url; + this._hostPort = hostPort; this._userAgent = userAgent; this._openSessions = {}; this._sessionIdGenerator = 0; @@ -117,13 +117,13 @@ class Driver { * @return {Connection} new connector-api session instance, a low level session API. * @access private */ - _createConnection(url, release) { + _createConnection(hostPort, release) { let sessionId = this._sessionIdGenerator++; - let conn = connect(url, this._config, this._connectionErrorCode()); + let conn = connect(hostPort, this._config, this._connectionErrorCode()); let streamObserver = new _ConnectionStreamObserver(this, conn); conn.initialize(this._userAgent, this._token, streamObserver); conn._id = sessionId; - conn._release = () => release(url, conn); + conn._release = () => release(hostPort, conn); this._openSessions[sessionId] = conn; return conn; @@ -186,8 +186,8 @@ class Driver { } // Extension point - _createConnectionProvider(address, connectionPool, driverOnErrorCallback) { - return new DirectConnectionProvider(address, connectionPool, driverOnErrorCallback); + _createConnectionProvider(hostPort, connectionPool, driverOnErrorCallback) { + return new DirectConnectionProvider(hostPort, connectionPool, driverOnErrorCallback); } // Extension point @@ -204,7 +204,7 @@ class Driver { _getOrCreateConnectionProvider() { if (!this._connectionProvider) { const driverOnErrorCallback = this._driverOnErrorCallback.bind(this); - this._connectionProvider = this._createConnectionProvider(this._url, this._pool, driverOnErrorCallback); + this._connectionProvider = this._createConnectionProvider(this._hostPort, this._pool, driverOnErrorCallback); } return this._connectionProvider; } diff --git a/src/v1/internal/connection-providers.js b/src/v1/internal/connection-providers.js index 02c46c0a0..106a50bf5 100644 --- a/src/v1/internal/connection-providers.js +++ b/src/v1/internal/connection-providers.js @@ -45,24 +45,24 @@ class ConnectionProvider { export class DirectConnectionProvider extends ConnectionProvider { - constructor(address, connectionPool, driverOnErrorCallback) { + constructor(hostPort, connectionPool, driverOnErrorCallback) { super(); - this._address = address; + this._hostPort = hostPort; this._connectionPool = connectionPool; this._driverOnErrorCallback = driverOnErrorCallback; } acquireConnection(mode) { - const connectionPromise = this._connectionPool.acquire(this._address); + const connectionPromise = this._connectionPool.acquire(this._hostPort); return this._withAdditionalOnErrorCallback(connectionPromise, this._driverOnErrorCallback); } } export class LoadBalancer extends ConnectionProvider { - constructor(address, routingContext, connectionPool, loadBalancingStrategy, driverOnErrorCallback) { + constructor(hostPort, routingContext, connectionPool, loadBalancingStrategy, driverOnErrorCallback) { super(); - this._seedRouter = address; + this._seedRouter = hostPort; this._routingTable = new RoutingTable([this._seedRouter]); this._rediscovery = new Rediscovery(new RoutingUtil(routingContext)); this._connectionPool = connectionPool; diff --git a/src/v1/internal/connector.js b/src/v1/internal/connector.js index 522a5edf9..82ab4b177 100644 --- a/src/v1/internal/connector.js +++ b/src/v1/internal/connector.js @@ -97,17 +97,17 @@ class Connection { /** * @constructor * @param {NodeChannel|WebSocketChannel} channel - channel with a 'write' function and a 'onmessage' callback property. - * @param {string} url - the hostname and port to connect to. + * @param {string} hostPort - the hostname and port to connect to. * @param {boolean} disableLosslessIntegers if this connection should convert all received integers to native JS numbers. */ - constructor(channel, url, disableLosslessIntegers = false) { + constructor(channel, hostPort, disableLosslessIntegers = false) { /** * An ordered queue of observers, each exchange response (zero or more * RECORD messages followed by a SUCCESS message) we receive will be routed * to the next pending observer. */ - this.url = url; - this.server = {address: url}; + this.hostPort = hostPort; + this.server = {address: hostPort}; this.creationTimestamp = Date.now(); this._disableLosslessIntegers = disableLosslessIntegers; this._pendingObservers = []; @@ -517,18 +517,18 @@ class ConnectionState { } /** - * Crete new connection to the provided url. + * Crete new connection to the provided address. * @access private - * @param {string} url - 'neo4j'-prefixed URL to Neo4j Bolt endpoint + * @param {string} hostPort - the Bolt endpoint to connect to * @param {object} config - this driver configuration * @param {string=null} connectionErrorCode - error code for errors raised on connection errors * @return {Connection} - New connection */ -function connect(url, config = {}, connectionErrorCode = null) { +function connect(hostPort, config = {}, connectionErrorCode = null) { const Ch = config.channel || Channel; - const parsedUrl = urlUtil.parseDatabaseUrl(url); - const channelConfig = new ChannelConfig(parsedUrl, config, connectionErrorCode); - return new Connection(new Ch(channelConfig), parsedUrl.hostAndPort, config.disableLosslessIntegers); + const parsedAddress = urlUtil.parseDatabaseUrl(hostPort); + const channelConfig = new ChannelConfig(parsedAddress, config, connectionErrorCode); + return new Connection(new Ch(channelConfig), parsedAddress.hostAndPort, config.disableLosslessIntegers); } export { diff --git a/src/v1/internal/http/http-driver.js b/src/v1/internal/http/http-driver.js index 3db3c2421..d4c934649 100644 --- a/src/v1/internal/http/http-driver.js +++ b/src/v1/internal/http/http-driver.js @@ -23,13 +23,13 @@ import HttpSessionTracker from './http-session-tracker'; export default class HttpDriver extends Driver { - constructor(url, userAgent, token, config) { - super(url, userAgent, token, config); + constructor(hostPort, userAgent, token, config) { + super(hostPort, userAgent, token, config); this._sessionTracker = new HttpSessionTracker(); } session() { - return new HttpSession(this._url, this._token, this._config, this._sessionTracker); + return new HttpSession(this._hostPort, this._token, this._config, this._sessionTracker); } close() { diff --git a/src/v1/internal/url-util.js b/src/v1/internal/url-util.js index d19433892..8b2c35f04 100644 --- a/src/v1/internal/url-util.js +++ b/src/v1/internal/url-util.js @@ -17,7 +17,7 @@ * limitations under the License. */ -import ParsedUrl from 'url-parse'; +import {parse as uriJsParse} from 'uri-js'; import {assertString} from './util'; const DEFAULT_BOLT_PORT = 7687; @@ -69,14 +69,14 @@ function parseDatabaseUrl(url) { assertString(url, 'URL'); const sanitized = sanitizeUrl(url); - const parsedUrl = new ParsedUrl(sanitized.url, {}, query => extractQuery(query, url)); + const parsedUrl = uriJsParse(sanitized.url); - const scheme = sanitized.schemeMissing ? null : extractScheme(parsedUrl.protocol); - const rawHost = extractHost(parsedUrl.hostname); // has square brackets for IPv6 - const host = unescapeIPv6Address(rawHost); // no square brackets for IPv6 + const scheme = sanitized.schemeMissing ? null : extractScheme(parsedUrl.scheme); + const host = extractHost(parsedUrl.host); // no square brackets for IPv6 + const formattedHost = formatHost(host); // has square brackets for IPv6 const port = extractPort(parsedUrl.port, scheme); - const hostAndPort = `${rawHost}:${port}`; - const query = parsedUrl.query; + const hostAndPort = `${formattedHost}:${port}`; + const query = extractQuery(parsedUrl.query, url); return new Url(scheme, host, port, hostAndPort, query); } @@ -85,8 +85,8 @@ function sanitizeUrl(url) { url = url.trim(); if (url.indexOf('://') === -1) { - // url does not contain scheme, add dummy 'http://' to make parser work correctly - return {schemeMissing: true, url: `http://${url}`}; + // url does not contain scheme, add dummy 'none://' to make parser work correctly + return {schemeMissing: true, url: `none://${url}`}; } return {schemeMissing: false, url: url}; @@ -169,17 +169,12 @@ function escapeIPv6Address(address) { } } -function unescapeIPv6Address(address) { - const startsWithSquareBracket = address.charAt(0) === '['; - const endsWithSquareBracket = address.charAt(address.length - 1) === ']'; - - if (!startsWithSquareBracket && !endsWithSquareBracket) { - return address; - } else if (startsWithSquareBracket && endsWithSquareBracket) { - return address.substring(1, address.length - 1); - } else { - throw new Error(`Illegal IPv6 address ${address}`); +function formatHost(host) { + if (!host) { + throw new Error(`Illegal host ${host}`); } + const isIPv6Address = host.indexOf(':') >= 0; + return isIPv6Address ? escapeIPv6Address(host) : host; } function formatIPv4Address(address, port) { diff --git a/src/v1/routing-driver.js b/src/v1/routing-driver.js index 6696a7356..e626dc86a 100644 --- a/src/v1/routing-driver.js +++ b/src/v1/routing-driver.js @@ -25,19 +25,19 @@ import LeastConnectedLoadBalancingStrategy, {LEAST_CONNECTED_STRATEGY_NAME} from import RoundRobinLoadBalancingStrategy, {ROUND_ROBIN_STRATEGY_NAME} from './internal/round-robin-load-balancing-strategy'; /** - * A driver that supports routing in a core-edge cluster. + * A driver that supports routing in a causal cluster. * @private */ class RoutingDriver extends Driver { - constructor(url, routingContext, userAgent, token = {}, config = {}) { - super(url, userAgent, token, validateConfig(config)); + constructor(hostPort, routingContext, userAgent, token = {}, config = {}) { + super(hostPort, userAgent, token, validateConfig(config)); this._routingContext = routingContext; } - _createConnectionProvider(address, connectionPool, driverOnErrorCallback) { + _createConnectionProvider(hostPort, connectionPool, driverOnErrorCallback) { const loadBalancingStrategy = RoutingDriver._createLoadBalancingStrategy(this._config, connectionPool); - return new LoadBalancer(address, this._routingContext, connectionPool, loadBalancingStrategy, driverOnErrorCallback); + return new LoadBalancer(hostPort, this._routingContext, connectionPool, loadBalancingStrategy, driverOnErrorCallback); } _createSession(mode, connectionProvider, bookmark, config) { @@ -47,14 +47,14 @@ class RoutingDriver extends Driver { return error; } - const url = conn.url; + const hostPort = conn.hostPort; if (error.code === SESSION_EXPIRED || isDatabaseUnavailable(error)) { - this._connectionProvider.forget(url); + this._connectionProvider.forget(hostPort); return error; } else if (isFailureToWrite(error)) { - this._connectionProvider.forgetWriter(url); - return newError('No longer possible to write to server at ' + url, SESSION_EXPIRED); + this._connectionProvider.forgetWriter(hostPort); + return newError('No longer possible to write to server at ' + hostPort, SESSION_EXPIRED); } else { return error; } diff --git a/test/internal/url-util.test.js b/test/internal/url-util.test.js index f1f62394a..6a8324a4b 100644 --- a/test/internal/url-util.test.js +++ b/test/internal/url-util.test.js @@ -723,6 +723,31 @@ describe('url-util', () => { expect(parse('https://localhost').port).toEqual(urlUtil.defaultPortForScheme('https')); }); + it('should parse URLs with port 80', () => { + ['http', 'https', 'ws', 'wss', 'bolt', 'bolt+routing'].forEach(scheme => { + verifyUrl(`${scheme}://localhost:80`, { + scheme: scheme, + host: 'localhost', + port: 80 + }); + }); + + ['localhost', '127.0.0.1', '192.168.10.29'].forEach(host => { + verifyUrl(`${host}:80`, { + host: host, + port: 80 + }); + }); + + ['::1', '1afc:0:a33:85a3::ff2f'].forEach(host => { + verifyUrl(`[${host}]:80`, { + host: host, + port: 80, + ipv6: true + }); + }); + }); + function verifyUrl(urlString, expectedUrl) { const url = parse(urlString); diff --git a/test/v1/driver.test.js b/test/v1/driver.test.js index 2e499bb1b..ae2d93b75 100644 --- a/test/v1/driver.test.js +++ b/test/v1/driver.test.js @@ -23,6 +23,7 @@ import FakeConnection from '../internal/fake-connection'; import lolex from 'lolex'; import {DEFAULT_ACQUISITION_TIMEOUT, DEFAULT_MAX_SIZE} from '../../src/v1/internal/pool-config'; import {ServerVersion, VERSION_3_1_0} from '../../src/v1/internal/server-version'; +import testUtils from '../internal/test-utils'; describe('driver', () => { @@ -78,6 +79,28 @@ describe('driver', () => { startNewTransaction(driver); }, 10000); + it('should fail with correct error message when connecting to port 80', done => { + if (testUtils.isClient()) { + // good error message is not available in browser + done(); + return; + } + + driver = neo4j.driver('bolt://localhost:80', sharedNeo4j.authToken); + + driver.session().run('RETURN 1').then(result => { + done.fail('Should not be able to connect. Result: ' + JSON.stringify(result)); + }).catch(error => { + const doesNotContainAddress = error.message.indexOf(':80') < 0; + if (doesNotContainAddress) { + done.fail(`Expected to contain ':80' but was: ${error.message}`); + } else { + expect(error.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE); + done(); + } + }); + }); + it('should handle wrong scheme', () => { expect(() => neo4j.driver("tank://localhost", sharedNeo4j.authToken)) .toThrow(new Error('Unknown scheme: tank')); diff --git a/test/v1/session.test.js b/test/v1/session.test.js index e0339aeff..f0b3542b5 100644 --- a/test/v1/session.test.js +++ b/test/v1/session.test.js @@ -1124,7 +1124,7 @@ describe('session', () => { function idleConnectionCount(driver) { const connectionProvider = driver._connectionProvider; - const address = connectionProvider._address; + const address = connectionProvider._hostPort; const connectionPool = connectionProvider._connectionPool; const idleConnections = connectionPool._pools[address]; return idleConnections.length;