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(#1448): Fix proxy configuration confusion #1883

Closed
Closed
Changes from all commits
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
fix(#1448): Fix proxy configuration confusion
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
```
samuel-deal-tisseo committed Mar 21, 2024
commit a11ec725def365bf6edb96a70ebc24a76bfdaec1
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
@@ -161,6 +161,7 @@ const runSingleRequest = async function (
} else {
proxyUri = `${proxyProtocol}://${proxyHostname}${uriPort}`;
}
request.proxy = {};

if (socksEnabled) {
request.httpsAgent = new SocksProxyAgent(
4 changes: 3 additions & 1 deletion packages/bruno-cli/src/utils/proxy-util.js
Original file line number Diff line number Diff line change
@@ -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);
}
}
1 change: 1 addition & 0 deletions packages/bruno-electron/src/ipc/network/index.js
Original file line number Diff line number Diff line change
@@ -174,6 +174,7 @@ const configureRequest = async (
} else {
proxyUri = `${proxyProtocol}://${proxyHostname}${uriPort}`;
}
request.proxy = {};

if (socksEnabled) {
request.httpsAgent = new SocksProxyAgent(
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,
@@ -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 {
@@ -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);
}
}