Support for non-friendly chars in url username and password (#858)

* Support for non-friendly chars in url username and password
- Added auth option to Connection class
- Updated pool.addConnection

* Updated test

Author: delvedor

lib/Connection.js

@@ -35,6 +35,7 @@ class Connection {
     this.ssl = opts.ssl || null
     this.id = opts.id || stripAuth(opts.url.href)
     this.headers = opts.headers || null
+    this.auth = opts.auth || { username: null, password: null }
     this.deadCount = 0
     this.resurrectTimeout = 0
 
@@ -180,6 +181,7 @@ class Connection {
 
   buildRequestObject (params) {
     const url = this.url
+    const { username, password } = this.auth
     const request = {
       protocol: url.protocol,
       hostname: url.hostname[0] === '['
@@ -194,8 +196,8 @@ class Connection {
       // https://github.com/elastic/elasticsearch-js/issues/843
       port: url.port !== '' ? url.port : undefined,
       headers: this.headers,
-      auth: !!url.username === true || !!url.password === true
-        ? `${url.username}:${url.password}`
+      auth: username != null && password != null
+        ? `${username}:${password}`
         : undefined,
       agent: this.agent
     }
@@ -224,7 +226,7 @@ class Connection {
   }
 
   // Handles console.log and utils.inspect invocations.
-  // We want to hide `agent` and `ssl` since they made
+  // We want to hide `auth`, `agent` and `ssl` since they made
   // the logs very hard to read. The user can still
   // access them with `instance.agent` and `instance.ssl`.
   [inspect.custom] (depth, options) {

lib/ConnectionPool.js

@@ -222,21 +222,24 @@ class ConnectionPool {
     // we can add it to them once the connection instance has been created
     if (opts.url.username !== '' && opts.url.password !== '') {
       this._auth = {
-        username: opts.url.username,
-        password: opts.url.password
+        username: decodeURIComponent(opts.url.username),
+        password: decodeURIComponent(opts.url.password)
       }
+      opts.auth = this._auth
     }
+
+    if (this._auth != null) {
+      if (opts.auth == null || (opts.auth.username == null && opts.auth.password == null)) {
+        opts.auth = this._auth
+        opts.url.username = this._auth.username
+        opts.url.password = this._auth.password
+      }
+    }
+
     if (opts.ssl == null) opts.ssl = this._ssl
     if (opts.agent == null) opts.agent = this._agent
 
     const connection = new this.Connection(opts)
-    if (connection.url.username === '' &&
-        connection.url.password === '' &&
-        this._auth != null) {
-      connection.url.username = this._auth.username
-      connection.url.password = this._auth.password
-    }
-
     debug('Adding a new connection', connection)
     if (this.connections.has(connection.id)) {
       throw new Error(`Connection with id '${connection.id}' is already present`)

test/unit/connection-pool.test.js

@@ -61,8 +61,23 @@ test('API', t => {
     t.deepEqual(pool._auth, { username: 'foo', password: 'bar' })
 
     pool.addConnection('http://localhost:9201')
-    t.strictEqual(pool.connections.get('http://localhost:9201/').url.username, 'foo')
-    t.strictEqual(pool.connections.get('http://localhost:9201/').url.password, 'bar')
+    const conn = pool.connections.get('http://localhost:9201/')
+    t.strictEqual(conn.url.username, 'foo')
+    t.strictEqual(conn.url.password, 'bar')
+    t.strictEqual(conn.auth.username, 'foo')
+    t.strictEqual(conn.auth.password, 'bar')
+    t.end()
+  })
+
+  t.test('addConnection should handle not-friendly url parameters for user and password', t => {
+    const pool = new ConnectionPool({ Connection })
+    const href = 'http://us"er:p@assword@localhost:9200/'
+    pool.addConnection(href)
+    const conn = pool.getConnection()
+    t.strictEqual(conn.url.username, 'us%22er')
+    t.strictEqual(conn.url.password, 'p%40assword')
+    t.strictEqual(conn.auth.username, 'us"er')
+    t.strictEqual(conn.auth.password, 'p@assword')
     t.end()
   })
 

test/unit/connection.test.js

@@ -526,7 +526,8 @@ test('Url with auth', t => {
 
   buildServer(handler, ({ port }, server) => {
     const connection = new Connection({
-      url: new URL(`http://foo:bar@localhost:${port}`)
+      url: new URL(`http://foo:bar@localhost:${port}`),
+      auth: { username: 'foo', password: 'bar' }
     })
     connection.request({
       path: '/hello',