Skip to content

Commit

Permalink
fix(server): clean up vestigial code from proxy (#3640)
Browse files Browse the repository at this point in the history
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. This change cleans it up.
  • Loading branch information
Jonathan Ginsburg authored Jun 1, 2021
1 parent 94cf15e commit f4aeac3
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 37 deletions.
18 changes: 7 additions & 11 deletions lib/middleware/proxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,20 +34,16 @@ function parseProxyConfig (proxies, config) {

const hostname = proxyDetails.hostname || config.hostname
const protocol = proxyDetails.protocol || config.protocol
const https = proxyDetails.protocol === 'https:'
let port
if (proxyDetails.port) {
port = proxyDetails.port
} else if (proxyDetails.protocol) {
port = https ? '443' : '80'
} else {
port = config.port
const defaultPorts = {
'http:': '80',
'https:': '443'
}
const port = proxyDetails.port || defaultPorts[proxyDetails.protocol] || config.port
const changeOrigin = proxyConfiguration.changeOrigin || false
const Agent = https ? httpsAgent : httpAgent
const Agent = protocol === 'https:' ? httpsAgent : httpAgent
const agent = new Agent({ keepAlive: true })
const proxy = httpProxy.createProxyServer({
target: { host: hostname, port, https, protocol },
target: { host: hostname, port, protocol },
xfwd: true,
changeOrigin: changeOrigin,
secure: config.proxyValidateSSL,
Expand All @@ -71,7 +67,7 @@ function parseProxyConfig (proxies, config) {
res.destroy()
})

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

Expand Down
60 changes: 34 additions & 26 deletions test/unit/middleware/proxy.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,10 +146,16 @@ describe('middleware.proxy', () => {
host: 'localhost',
port: '8000',
baseUrl: '/',
path: '/base/',
https: false
path: '/base/'
})
expect(parsedProxyConfig[0].proxy).to.exist
expect(parsedProxyConfig[0].proxy).to.containSubset({
options: {
target: {
protocol: 'http:'
}
}
})
})

it('should set default http port', () => {
Expand All @@ -160,10 +166,16 @@ describe('middleware.proxy', () => {
host: 'localhost',
port: '80',
baseUrl: '/',
path: '/base/',
https: false
path: '/base/'
})
expect(parsedProxyConfig[0].proxy).to.exist
expect(parsedProxyConfig[0].proxy).to.containSubset({
options: {
target: {
protocol: 'http:'
}
}
})
})

it('should set default https port', () => {
Expand All @@ -174,8 +186,7 @@ describe('middleware.proxy', () => {
host: 'localhost',
port: '443',
baseUrl: '/',
path: '/base/',
https: true
path: '/base/'
})
expect(parsedProxyConfig[0].proxy).to.exist
expect(parsedProxyConfig[0].proxy).to.containSubset({
Expand All @@ -195,10 +206,16 @@ describe('middleware.proxy', () => {
host: 'localhost',
port: '8000',
baseUrl: '/proxy',
path: '/base',
https: false
path: '/base'
})
expect(parsedProxyConfig[0].proxy).to.exist
expect(parsedProxyConfig[0].proxy).to.containSubset({
options: {
target: {
protocol: 'http:'
}
}
})
})

it('should determine protocol', () => {
Expand All @@ -209,8 +226,7 @@ describe('middleware.proxy', () => {
host: 'localhost',
port: '8000',
baseUrl: '',
path: '/base',
https: true
path: '/base'
})
expect(parsedProxyConfig[0].proxy).to.exist
expect(parsedProxyConfig[0].proxy).to.containSubset({
Expand All @@ -231,8 +247,7 @@ describe('middleware.proxy', () => {
host: 'localhost',
port: 9877,
baseUrl: '/proxy/test',
path: '/base',
https: false
path: '/base'
})
expect(parsedProxyConfig[0].proxy).to.exist
})
Expand All @@ -246,8 +261,7 @@ describe('middleware.proxy', () => {
host: 'localhost',
port: 9877,
baseUrl: '/proxy/test/',
path: '/base/',
https: false
path: '/base/'
})
expect(parsedProxyConfig[0].proxy).to.exist
})
Expand All @@ -261,8 +275,7 @@ describe('middleware.proxy', () => {
host: 'krinkle.dev',
port: '80',
baseUrl: '/w',
path: '/w',
https: false
path: '/w'
})
expect(parsedProxyConfig[0].proxy).to.exist
})
Expand All @@ -276,8 +289,7 @@ describe('middleware.proxy', () => {
host: 'krinkle.dev',
port: '443',
baseUrl: '/w',
path: '/w',
https: true
path: '/w'
})
expect(parsedProxyConfig[0].proxy).to.exist
})
Expand All @@ -290,8 +302,7 @@ describe('middleware.proxy', () => {
host: 'localhost',
port: '8000',
baseUrl: '/proxy/test/',
path: '/base/',
https: false
path: '/base/'
})
expect(parsedProxyConfig[0].proxy).to.exist
})
Expand All @@ -307,16 +318,14 @@ describe('middleware.proxy', () => {
host: 'gstatic.com',
port: '80',
baseUrl: '/something',
path: '/sub/some',
https: false
path: '/sub/some'
})
expect(parsedProxyConfig[0].proxy).to.exist
expect(parsedProxyConfig[1]).to.containSubset({
host: 'localhost',
port: '9000',
baseUrl: '',
path: '/sub',
https: false
path: '/sub'
})
expect(parsedProxyConfig[1].proxy).to.exist
})
Expand All @@ -331,8 +340,7 @@ describe('middleware.proxy', () => {
host: 'localhost',
port: '8000',
baseUrl: '/',
path: '/base/',
https: false
path: '/base/'
})
expect(parsedProxyConfig[0].proxy).to.exist
})
Expand Down

0 comments on commit f4aeac3

Please sign in to comment.