fix(#1448): Fix proxy configuration confusion

[samuel-deal-tisseo]

Description

Fix a confusion between system proxy and explicit bruno proxy configuration

It fixes #1448

Contribution Checklist:

• The pull request only addresses one issue or adds one feature.
• The pull request does not introduce any breaking changes
• I have added screenshots or gifs to help explain the change if applicable.
• I have read the contribution guidelines.
• Create an issue and link to the pull request.

Status

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)

Description of the issue

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

Minimal example

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

[srvkb01]

did you have news about this merge pull requests ? @sanjai0py

[WiredNerd]

+1

[pablodr81]

+1

[srvkb01]

did someone have news ? (i have always error in v1.17.0)

[helloanoop]

This issue has already been fixed in latest version of Bruno. Closing this PR.
Please reopen the issue if the problem still persists,