Skip to content

Commit

Permalink
fix(usebruno#1448): Fix proxy configuration confusion
Browse files Browse the repository at this point in the history
This patched have been tested with http proxy:
* on http requests
* on https requests
* on proxy auth with basic auth
* on proxy without auth
* on tinnyproxy proxy
* on squid proxy

It should have no effects on SOCK proxy (And I don't know if the issue exists on those config anyway)

Given the following configuration
* System proxy: http://system.proxy:8080
* Bruno proxy: http://explicit.proxy:9000

On http request (to http://neverssl.com): it try to send the http request
to http://neverssl.com:8080 (wrong port) via the proxy http://explicit.proxy:9000 (correct proxy)

On https request (to https://www.example.com): it try send a CONNECT ::1 request (can be seen with wireshark)

An issue is opened on the proxy-agent repo: TooTallNate/proxy-agents#298
even tho I don't know if it's an axios issue, a proxy-agent issue or a bruno issue

```javascript
import { parse as parseURL } from "url";
import axios from 'axios';
import { HttpsProxyAgent } from 'https-proxy-agent';
import { HttpProxyAgent } from 'http-proxy-agent';

class PatchedHttpsProxyAgent extends HttpsProxyAgent {
	async connect(req, opts) {
		const url = parseURL(req.path);
		url.port = Number(url.port) || (url.protocol.includes('https') ? 443 : 80);
		return super.connect(req, { ...opts, ...url });
	}
}

function proxyObjToStr(proxyConf) {
	return proxyConf.protocol+"://"+proxyConf.host+":"+proxyConf.port.toString();
}

function test_req(url, proxyConf, applyPatch) {
	let request = {
		method: 'get',
		url: url,
		responseType: 'string',
		httpAgent: new HttpProxyAgent(proxyObjToStr(proxyConf)),
		httpsAgent: new HttpsProxyAgent(proxyObjToStr(proxyConf)),
		proxy: proxyConf
	};
	if (applyPatch) {
		request.proxy = {}
		request.httpsAgent = new PatchedHttpsProxyAgent(proxyObjToStr(proxyConf));
	}
	const axiosInstance = axios.create();
	const test_descr= url + (applyPatch ? " [patched]: " : " [current]: ");
	return axiosInstance(request).then(function (response) {
		console.log(test_descr + 'Ok');
	}).catch(function (error) {
		console.log(test_descr + "KO");
		if (error.response) {
			console.log({
				http_head: error.response.request._header.split("\r\n")[0],
				path: error.response.request.path,
				host: error.response.request.host
			});
		}
	});
}

async function main() {
	process.env["HTTPS_PROXY"] = "http://implicit.proxy:8080";
	const proxyConf = {
		protocol: 'http',
		host: 'explicit',
		port: 9000
	};
	await test_req("http://neverssl.com", proxyConf, false);
	await test_req("https://www.example.com/", proxyConf, false);
	await test_req("http://neverssl.com", proxyConf, true);
	await test_req("https://www.example.com/", proxyConf, true);
}

await main();
```

Here the following result:

```
http://neverssl.com: KO
{
  http_head: 'GET http://neverssl.com:8080/ HTTP/1.1',
  path: 'http://neverssl.com:8080/',
  host: 'implicit.proxy'
}
https://www.example.com/: KO
{
  http_head: 'GET https://www.example.com:8080/ HTTP/1.1',
  path: 'https://www.example.com:8080/',
  host: 'implicit.proxy'
}
http://neverssl.com [patched]: Ok
https://www.example.com/ [patched]: Ok
```
  • Loading branch information
samuel-deal-tisseo committed Mar 21, 2024
1 parent f8ba781 commit a11ec72
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 4 deletions.
1 change: 1 addition & 0 deletions packages/bruno-cli/src/runner/run-single-request.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,7 @@ const runSingleRequest = async function (
} else {
proxyUri = `${proxyProtocol}://${proxyHostname}${uriPort}`;
}
request.proxy = {};

if (socksEnabled) {
request.httpsAgent = new SocksProxyAgent(
Expand Down
4 changes: 3 additions & 1 deletion packages/bruno-cli/src/utils/proxy-util.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ class PatchedHttpsProxyAgent extends HttpsProxyAgent {
}

async connect(req, opts) {
const combinedOpts = { ...this.constructorOpts, ...opts };
const url = parseUrl(req.path);
url.port = Number(url.port) || (url.protocol.includes('https') ? 443 : 80);
const combinedOpts = { ...this.constructorOpts, ...opts, ...url };
return super.connect(req, combinedOpts);
}
}
Expand Down
1 change: 1 addition & 0 deletions packages/bruno-electron/src/ipc/network/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ const configureRequest = async (
} else {
proxyUri = `${proxyProtocol}://${proxyHostname}${uriPort}`;
}
request.proxy = {};

if (socksEnabled) {
request.httpsAgent = new SocksProxyAgent(
Expand Down
9 changes: 6 additions & 3 deletions packages/bruno-electron/src/utils/proxy-util.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const parseUrl = require('url').parse;
const { isEmpty } = require('lodash');
const { HttpsProxyAgent } = require('https-proxy-agent');
const { HttpProxyAgent } = require('http-proxy-agent');

const DEFAULT_PORTS = {
ftp: 21,
Expand Down Expand Up @@ -63,8 +64,8 @@ const shouldUseProxy = (url, proxyBypass) => {
};

/**
* Patched version of HttpsProxyAgent to get around a bug that ignores options
* such as ca and rejectUnauthorized when upgrading the proxied socket to TLS:
* Patched version of HttpsProxyAgent to get around a bug that ignores
* options like ca and rejectUnauthorized when upgrading the socket to TLS:
* https://github.com/TooTallNate/proxy-agents/issues/194
*/
class PatchedHttpsProxyAgent extends HttpsProxyAgent {
Expand All @@ -74,7 +75,9 @@ class PatchedHttpsProxyAgent extends HttpsProxyAgent {
}

async connect(req, opts) {
const combinedOpts = { ...this.constructorOpts, ...opts };
const url = parseUrl(req.path);
url.port = Number(url.port) || (url.protocol.includes('https') ? 443 : 80);
const combinedOpts = { ...this.constructorOpts, ...opts, ...url };
return super.connect(req, combinedOpts);
}
}
Expand Down

0 comments on commit a11ec72

Please sign in to comment.