https connection doesn't work behind a socks5 proxy

[hensansi]

🐛 Bug Report

The feature added to replace createNodeAgent doesnt work as expected
#810

When proxying the requests over socks5 through an ssh tunnel the library fails to find the host

After some debugging I found where the issue is but I don't have enough understanding of the library to apply it.

On the connection pool urlToHost new URL will transform https://myelastic:443 and remove the port, since it is the https protocol

When running the request the host is:

host:
      [ 'Host',
        'myelastic:80' ] } }

If I do the following the request is done successfully:

const requestParams = this.buildRequestObject(params)
requestParams.port = undefined

If requestParams.port = '' , https.request host port will default to 80

To Reproduce

Version 6.X

const { Client } = require('@elastic/elasticsearch')
const SocksProxyAgent = require('socks-proxy-agent')

const client = new Client({
  node: 'https://myelasticsearch:443',
  agent: () => new SocksProxyAgent('socks5://127.0.0.1:7766')
})

client.search().then(response => {
  console.log(response)
})

Expected behavior

Version 16.X

const { Client } = require('elasticsearch')
const SocksProxyAgent = require('socks-proxy-agent')

const client = new Client({
  host: 'https://myelasticsearch:443',
  createNodeAgent: () => new SocksProxyAgent('socks5://127.0.0.1:7766')
});

client.search().then(response => {
  console.log(response)
})

or the same as in the reproduce with:

requestParams.port = undefined

Your Environment

• node version: v10.14.0
• @elastic/elasticsearch version: >=6.4.3
• os: Mac

[delvedor]

Hello! Thank you for reporting!
Just to be sure, the code was working with the legacy client while it does not with the new one?

Can you provide a full example to reproduce the issue?
I repository would be awesome. Thanks!

[hensansi]

This example is enough to show/reproduce the issue:

const { Client } = require('@elastic/elasticsearch')
const SocksProxyAgent = require('socks-proxy-agent')

const client = new Client({
  node: 'https://myelasticsearch:443',
  agent: () => new SocksProxyAgent('socks5://127.0.0.1:7766')
})

client.search().then(response => {
  console.log(response)
})

If you console.log the request: (Connection.js)

debug('Starting a new request', params)
console.log(requestParams)
const request = this.makeRequest(requestParams)
console.log(request)

Even without having elasticsearch running or the proxy you will get:

{ 'user-agent':
      [ 'User-Agent',
        'elasticsearch-js/6.7.0-rc.3 (darwin 18.5.0-x64; Node.js v10.14.0)' ],
     'content-type': [ 'Content-Type', 'application/json' ],
     'content-length': [ 'Content-Length', '0' ],
     host: [ 'Host', 'myelasticsearch:80' ] } }

This doesn't work but if I unset the following:

debug('Starting a new request', params)
console.log(requestParams)
requestParams.port = undefined
const request = this.makeRequest(requestParams)
console.log(request)

I get:

{ 'user-agent':
      [ 'User-Agent',
        'elasticsearch-js/6.7.0-rc.3 (darwin 18.5.0-x64; Node.js v10.14.0)' ],
     'content-type': [ 'Content-Type', 'application/json' ],
     'content-length': [ 'Content-Length', '0' ],
     host: [ 'Host', 'myelasticsearch:443' ] } }

[hensansi]

I will try to provide a repo example

[delvedor]

Hello! Thank you for investigating!
The bug is due to how we build the request params object internally, and more specifically how we handle the port.
We are using the URL constructor to parse the url of the node, in the legacy client we were using url.parse, which now is deprecated.
The new URL parser defaults the port to an empty string for some protocols (https is one of them), and the http/s.request function in case of empty string defaults to 80.

You can see how Node.js core is handling the URL here.
I'll open a pr to fix the issue.

Thank you again for reporting and investigating!

[hensansi]

https://github.com/hensansi/testing-socks