Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(server): clean up vestigial code from proxy #3640

Merged
merged 2 commits into from
Jun 1, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fix(server): clean up vestigial code from proxy
When using the proxy feature of Karma, the target value can include the [scheme](https://tools.ietf.org/html/std66#section-3.1). It was used to determine the `https` variable to be sent to the [`http-proxy`](https://www.npmjs.com/package/http-proxy) `.createProxyServer` method. However, it is now disregarded by that package. Therefore, this PR cleans it up.
  • Loading branch information
Jonathan Ginsburg committed May 5, 2021
commit 3556ebd06ca287d0cff73f2a278c1fcc04a1a820
18 changes: 15 additions & 3 deletions lib/middleware/proxy.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const url = require('url')
const { Agent: httpAgent } = require('http')
const { Agent: httpsAgent } = require('https')
const httpProxy = require('http-proxy')
const _ = require('lodash')

Expand Down Expand Up @@ -38,11 +40,14 @@ function parseProxyConfig (proxies, config) {
}
const port = proxyDetails.port || defaultPorts[proxyDetails.protocol] || config.port
const changeOrigin = proxyConfiguration.changeOrigin || false
const Agent = protocol === 'https:' ? httpsAgent : httpAgent
const agent = new Agent({ keepAlive: true })
const proxy = httpProxy.createProxyServer({
target: { host: hostname, port, protocol },
xfwd: true,
changeOrigin: changeOrigin,
secure: config.proxyValidateSSL
secure: config.proxyValidateSSL,
agent
})

;['proxyReq', 'proxyRes'].forEach(function (name) {
Expand All @@ -62,7 +67,7 @@ function parseProxyConfig (proxies, config) {
res.destroy()
})

return { path: proxyPath, baseUrl: pathname, host: hostname, port, proxy }
return { path: proxyPath, baseUrl: pathname, host: hostname, port, proxy, agent }
}), 'path').reverse()
}

Expand Down Expand Up @@ -108,7 +113,14 @@ function createProxyHandler (proxies, urlRoot) {
return createProxy
}

exports.create = function (/* config */config, /* config.proxies */proxies) {
exports.create = function (/* config */config, /* config.proxies */proxies, /* emitter */emitter) {
const proxyRecords = parseProxyConfig(proxies, config)
emitter.on('exit', (done) => {
log.debug('Destroying proxy agents')
proxyRecords.forEach((proxyRecord) => {
proxyRecord.agent.destroy()
})
done()
})
return createProxyHandler(proxyRecords, config.urlRoot)
}
20 changes: 20 additions & 0 deletions test/unit/middleware/proxy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -360,4 +360,24 @@ describe('middleware.proxy', () => {
it('should handle empty proxy config', () => {
expect(m.parseProxyConfig({})).to.deep.equal([])
})

it('should use http agent with keepAlive=true', () => {
const proxy = { '/base': 'http://localhost:8000/proxy' }
const parsedProxyConfig = m.parseProxyConfig(proxy, {})
expect(parsedProxyConfig).to.have.length(1)
expect(parsedProxyConfig[0].proxy.options.agent).to.containSubset({
keepAlive: true,
protocol: 'http:'
})
})

it('should use https agent with keepAlive=true', () => {
const proxy = { '/base': 'https://localhost:8000/proxy' }
const parsedProxyConfig = m.parseProxyConfig(proxy, {})
expect(parsedProxyConfig).to.have.length(1)
expect(parsedProxyConfig[0].proxy.options.agent).to.containSubset({
keepAlive: true,
protocol: 'https:'
})
})
})