delvedor / hpagent

A ready to use http and https agent for working with proxies that keeps connections alive!
MIT License
182 stars 36 forks source link

Don't send proxy-authorization header with empty username & password. #18

Closed Tomalak closed 3 years ago

Tomalak commented 3 years ago

When creating the agent in the described way:

const agent = new HttpProxyAgent({
  keepAlive: true,
  keepAliveMsecs: 1000,
  maxSockets: 256,
  maxFreeSockets: 256,
  proxy: 'http://localhost:8080'
});

hpagent will use new URL() to parse 'http://localhost:8080', which generates this data structure:

URL {
  href: 'http://localhost:8080/',
  origin: 'http://localhost:8080',
  protocol: 'http:',
  username: '',
  password: '',
  host: 'localhost:8080',
  hostname: 'localhost',
  port: '8080',
  pathname: '/',
  search: '',
  searchParams: URLSearchParams {},
  hash: ''
}

The agent will then generate an empty proxy-authorization header because of this section:

if (this.proxy.username != null && this.proxy.password != null) {
  const base64 = Buffer.from(`${this.proxy.username}:${this.proxy.password}`).toString('base64')
  requestOptions.headers['proxy-authorization'] = `Basic ${base64}`
}

Apart from being not particularly useful (why send a header that contains no data?), it also throws off cntlm.exe (a utility for transparently authenticating anonymous traffic to an upstream proxy that requires Microsoft authentication, and which expects that header not to be present unless you want to specify a certain username). Such proxies react with 407 Proxy Authorization Required responses to empty username fields.

Once I do this:

const agent = new HttpProxyAgent({
  keepAlive: true,
  keepAliveMsecs: 1000,
  maxSockets: 256,
  maxFreeSockets: 256,
  proxy: {
    protocol: 'http:',
    hostname: '127.0.0.1',
    port: 8080,
    username: null,
    password: null
  }
});

it starts to function properly. This is a work-around, but it's needlessly obscure.

Since sending an empty proxy-authorization header is not particularly useful, I think it would make sense to change the above check to:

if (this.proxy.username > '' && this.proxy.password >= '') {
  const base64 = Buffer.from(`${this.proxy.username}:${this.proxy.password}`).toString('base64')
  requestOptions.headers['proxy-authorization'] = `Basic ${base64}`
}

This way the authorization is only sent for non-empty usernames, with possibly-empty passwords, while the absence of a username will result in ommiting this header.