Skip to content

Commit bb0ff22

Browse files
committed
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
1 parent 9897ba8 commit bb0ff22

File tree

4 files changed

+36
-15
lines changed

4 files changed

+36
-15
lines changed

lib/Connection.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class Connection {
3535
this.ssl = opts.ssl || null
3636
this.id = opts.id || stripAuth(opts.url.href)
3737
this.headers = opts.headers || null
38+
this.auth = opts.auth || { username: null, password: null }
3839
this.deadCount = 0
3940
this.resurrectTimeout = 0
4041

@@ -180,6 +181,7 @@ class Connection {
180181

181182
buildRequestObject (params) {
182183
const url = this.url
184+
const { username, password } = this.auth
183185
const request = {
184186
protocol: url.protocol,
185187
hostname: url.hostname[0] === '['
@@ -194,8 +196,8 @@ class Connection {
194196
// https://github.com/elastic/elasticsearch-js/issues/843
195197
port: url.port !== '' ? url.port : undefined,
196198
headers: this.headers,
197-
auth: !!url.username === true || !!url.password === true
198-
? `${url.username}:${url.password}`
199+
auth: username != null && password != null
200+
? `${username}:${password}`
199201
: undefined,
200202
agent: this.agent
201203
}
@@ -224,7 +226,7 @@ class Connection {
224226
}
225227

226228
// Handles console.log and utils.inspect invocations.
227-
// We want to hide `agent` and `ssl` since they made
229+
// We want to hide `auth`, `agent` and `ssl` since they made
228230
// the logs very hard to read. The user can still
229231
// access them with `instance.agent` and `instance.ssl`.
230232
[inspect.custom] (depth, options) {

lib/ConnectionPool.js

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -222,21 +222,24 @@ class ConnectionPool {
222222
// we can add it to them once the connection instance has been created
223223
if (opts.url.username !== '' && opts.url.password !== '') {
224224
this._auth = {
225-
username: opts.url.username,
226-
password: opts.url.password
225+
username: decodeURIComponent(opts.url.username),
226+
password: decodeURIComponent(opts.url.password)
227227
}
228+
opts.auth = this._auth
228229
}
230+
231+
if (this._auth != null) {
232+
if (opts.auth == null || (opts.auth.username == null && opts.auth.password == null)) {
233+
opts.auth = this._auth
234+
opts.url.username = this._auth.username
235+
opts.url.password = this._auth.password
236+
}
237+
}
238+
229239
if (opts.ssl == null) opts.ssl = this._ssl
230240
if (opts.agent == null) opts.agent = this._agent
231241

232242
const connection = new this.Connection(opts)
233-
if (connection.url.username === '' &&
234-
connection.url.password === '' &&
235-
this._auth != null) {
236-
connection.url.username = this._auth.username
237-
connection.url.password = this._auth.password
238-
}
239-
240243
debug('Adding a new connection', connection)
241244
if (this.connections.has(connection.id)) {
242245
throw new Error(`Connection with id '${connection.id}' is already present`)

test/unit/connection-pool.test.js

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,23 @@ test('API', t => {
6161
t.deepEqual(pool._auth, { username: 'foo', password: 'bar' })
6262

6363
pool.addConnection('http://localhost:9201')
64-
t.strictEqual(pool.connections.get('http://localhost:9201/').url.username, 'foo')
65-
t.strictEqual(pool.connections.get('http://localhost:9201/').url.password, 'bar')
64+
const conn = pool.connections.get('http://localhost:9201/')
65+
t.strictEqual(conn.url.username, 'foo')
66+
t.strictEqual(conn.url.password, 'bar')
67+
t.strictEqual(conn.auth.username, 'foo')
68+
t.strictEqual(conn.auth.password, 'bar')
69+
t.end()
70+
})
71+
72+
t.test('addConnection should handle not-friendly url parameters for user and password', t => {
73+
const pool = new ConnectionPool({ Connection })
74+
const href = 'http://us"er:p@assword@localhost:9200/'
75+
pool.addConnection(href)
76+
const conn = pool.getConnection()
77+
t.strictEqual(conn.url.username, 'us%22er')
78+
t.strictEqual(conn.url.password, 'p%40assword')
79+
t.strictEqual(conn.auth.username, 'us"er')
80+
t.strictEqual(conn.auth.password, 'p@assword')
6681
t.end()
6782
})
6883

test/unit/connection.test.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,8 @@ test('Url with auth', t => {
526526

527527
buildServer(handler, ({ port }, server) => {
528528
const connection = new Connection({
529-
url: new URL(`http://foo:bar@localhost:${port}`)
529+
url: new URL(`http://foo:bar@localhost:${port}`),
530+
auth: { username: 'foo', password: 'bar' }
530531
})
531532
connection.request({
532533
path: '/hello',

0 commit comments

Comments
 (0)