Skip to content

Commit

Permalink
Add clientFactory option to ProxyAgent (nodejs#2003)
Browse files Browse the repository at this point in the history
* Add clientFactory option to ProxyAgent

The default use of a Client means that HTTP CONNECT requests to the
Proxy will block successive requests. Giving consumers the option to
supply a factory allows them use a Pool which ensures that these
requests will not block.

fixes: nodejs#2001

* fixup! Use pool by default and change clientFactory return type

* Add clientFactory to ProxyAgent docs

---------

Co-authored-by: Andrew Fecenko <afecenko@atlassian.com>
  • Loading branch information
2 people authored and crysmags committed Feb 27, 2024
1 parent c0b67c7 commit 5a2582c
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 4 deletions.
1 change: 1 addition & 0 deletions docs/api/ProxyAgent.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ Extends: [`AgentOptions`](Agent.md#parameter-agentoptions)
* **uri** `string` (required) - It can be passed either by a string or a object containing `uri` as string.
* **token** `string` (optional) - It can be passed by a string of token for authentication.
* **auth** `string` (**deprecated**) - Use token.
* **clientFactory** `(origin: URL, opts: Object) => Dispatcher` - Default: `(origin, opts) => new Pool(origin, opts)`

Examples:

Expand Down
14 changes: 12 additions & 2 deletions lib/proxy-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
const { kProxy, kClose, kDestroy, kInterceptors } = require('./core/symbols')
const { URL } = require('url')
const Agent = require('./agent')
const Client = require('./client')
const Pool = require('./pool')
const DispatcherBase = require('./dispatcher-base')
const { InvalidArgumentError, RequestAbortedError } = require('./core/errors')
const buildConnector = require('./core/connect')
Expand Down Expand Up @@ -34,6 +34,10 @@ function buildProxyOptions (opts) {
}
}

function defaultFactory (origin, opts) {
return new Pool(origin, opts)
}

class ProxyAgent extends DispatcherBase {
constructor (opts) {
super(opts)
Expand All @@ -51,6 +55,12 @@ class ProxyAgent extends DispatcherBase {
throw new InvalidArgumentError('Proxy opts.uri is mandatory')
}

const { clientFactory = defaultFactory } = opts

if (typeof clientFactory !== 'function') {
throw new InvalidArgumentError('Proxy opts.clientFactory must be a function.')
}

this[kRequestTls] = opts.requestTls
this[kProxyTls] = opts.proxyTls
this[kProxyHeaders] = opts.headers || {}
Expand All @@ -69,7 +79,7 @@ class ProxyAgent extends DispatcherBase {

const connect = buildConnector({ ...opts.proxyTls })
this[kConnectEndpoint] = buildConnector({ ...opts.requestTls })
this[kClient] = new Client(resolvedUrl, { connect })
this[kClient] = clientFactory(resolvedUrl, { connect })
this[kAgent] = new Agent({
...opts,
connect: async (opts, callback) => {
Expand Down
40 changes: 40 additions & 0 deletions test/proxy-agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ const { nodeMajor } = require('../lib/core/util')
const { readFileSync } = require('fs')
const { join } = require('path')
const ProxyAgent = require('../lib/proxy-agent')
const Pool = require('../lib/pool')
const { createServer } = require('http')
const https = require('https')
const proxy = require('proxy')
Expand Down Expand Up @@ -72,6 +73,45 @@ test('use proxy-agent to connect through proxy', async (t) => {
proxyAgent.close()
})

test('use proxy agent to connect through proxy using Pool', async (t) => {
t.plan(3)
const server = await buildServer()
const proxy = await buildProxy()
let resolveFirstConnect
let connectCount = 0

proxy.authenticate = async function (req, fn) {
if (++connectCount === 2) {
t.pass('second connect should arrive while first is still inflight')
resolveFirstConnect()
fn(null, true)
} else {
await new Promise((resolve) => {
resolveFirstConnect = resolve
})
fn(null, true)
}
}

server.on('request', (req, res) => {
res.end()
})

const serverUrl = `http://localhost:${server.address().port}`
const proxyUrl = `http://localhost:${proxy.address().port}`
const clientFactory = (url, options) => {
return new Pool(url, options)
}
const proxyAgent = new ProxyAgent({ auth: Buffer.from('user:pass').toString('base64'), uri: proxyUrl, clientFactory })
const firstRequest = request(`${serverUrl}`, { dispatcher: proxyAgent })
const secondRequest = await request(`${serverUrl}`, { dispatcher: proxyAgent })
t.equal((await firstRequest).statusCode, 200)
t.equal(secondRequest.statusCode, 200)
server.close()
proxy.close()
proxyAgent.close()
})

test('use proxy-agent to connect through proxy using path with params', async (t) => {
t.plan(6)
const server = await buildServer()
Expand Down
5 changes: 3 additions & 2 deletions test/types/proxy-agent.test-d.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { expectAssignable } from 'tsd'
import { URL } from 'url'
import { ProxyAgent, setGlobalDispatcher, getGlobalDispatcher, Agent } from '../..'
import { ProxyAgent, setGlobalDispatcher, getGlobalDispatcher, Agent, Pool } from '../..'

expectAssignable<ProxyAgent>(new ProxyAgent(''))
expectAssignable<ProxyAgent>(new ProxyAgent({ uri: '' }))
Expand All @@ -25,7 +25,8 @@ expectAssignable<ProxyAgent>(
cert: '',
servername: '',
timeout: 1
}
},
clientFactory: (origin: URL, opts: object) => new Pool(origin, opts)
})
)

Expand Down
3 changes: 3 additions & 0 deletions types/proxy-agent.d.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import Agent from './agent'
import buildConnector from './connector';
import Client from './client'
import Dispatcher from './dispatcher'
import { IncomingHttpHeaders } from './header'
import Pool from './pool'

export default ProxyAgent

Expand All @@ -23,5 +25,6 @@ declare namespace ProxyAgent {
headers?: IncomingHttpHeaders;
requestTls?: buildConnector.BuildOptions;
proxyTls?: buildConnector.BuildOptions;
clientFactory?(origin: URL, opts: object): Dispatcher;
}
}

0 comments on commit 5a2582c

Please sign in to comment.