Support HTTP_PROXY and NO_PROXY env var

[shttrstckak]

A somewhat standard env var used in proxying applications is the HTTP_PROXY and NO_PROXY variables.

Undici should support automatically setting a global dispatcher if HTTP_PROXY env var is used. Currently Undici supports using setGlobalDispatcher() method to set a specific proxy endpoint.

However there is a sister environment variable to HTTP_PROXY that supports contextually non-proxying domains: NO_PROXY

NO_PROXY requires a list of domain names to be NOT proxied and should make a request normally.

If any of the above cannot be supported, then Undici (and ultimately Node's Fetch-Undici) should allow per-request proxy settings or function hook to allow anyone to develop the same effect as HTTP_PROXY and NO_PROXY env var in their application that uses Undici/Node Fetch.

Thoughts?

[mcollina]

I don't know if we should support HTTP_PROXY. How is widespread the use of HTTP_PROXY in the ecosystem.

[shttrstckak]

The "ecosystem" as in the JS ecosystem or the larger software ecosystem?

For the latter, HTTP_PROXY is used everywhere.

For JS it's a quite heavily used I imagine for the same reasons. Popular frameworks tend to implement it.

Node.http module already supports it out of the box.

Got, Axios, Node-fetch (npm package) require a module global-agent to utilize. Global-agent does not appear to have any plans to support Undici.

Popular JS based testing framework Cypress supports it.

On the other side, browsers tend to support some sort of proxy configuration.

Overall I think if there is no interest in supporting HTTP_PROXY in Undici, then it would be better to support to being able to implement these env vars. HTTP_PROXY can already be supported easily however NO_PROXY doesn't seem currently possible.

[silverwind]

Use of these environment variables is in very widespread, see number of upvotes on nodejs/node#8381. Many high-security environments restrict outbound internet access via a allowlist of domains configured on the proxy.

I do maintain a fetch wrapper specifically for the purpose of automatic proxy discovery from environment. The crucial part is that if such proxy discovery is performed, there must be an opt-out option because sometimes one wants to fetch without proxy, irregardless of environment variables.

Parsing of these env vars is tricky, I recommend https://github.com/Rob--W/proxy-from-env for it.

[mcollina]

Then let's do it.

[shttrstckak]

I offer myself as sacrifice and will probably get started this weekend.

Does anyone have any good starters on developing with this repository? For example, What would be a good place in code to start investigating for this feature?

[mcollina]

The check for automatically supporting them should likely be in https://github.com/nodejs/undici/blob/main/lib/global.js.

[silverwind]

We probably should cleanly re-implement https://github.com/Rob--W/proxy-from-env because that module includes support for npm_config_* variables which I think has no place in module that is supposed to only support the standard environment variables. See discussion in https://github.com/Rob--W/proxy-from-env/issues/13.

[silverwind]

Also read https://about.gitlab.com/blog/2021/01/27/we-need-to-talk-no-proxy/. It gives an overview of the different implementations and the last two paragraphs give good advice on how to implement.

I plan to create a module that exactly implements such a parser.

[sla100]

There is my PoC:

import {URL} from 'node:url';
import {getGlobalDispatcher, setGlobalDispatcher, Dispatcher, ProxyAgent} from 'undici';

const proxyAgents = Object.fromEntries(
  ['http', 'https'].map( protocol => {
    const uri = process.env[ `${protocol}_proxy` ];
    if ( uri ){
      return [`${protocol}:`, new ProxyAgent(uri)];
    }
    return [];
  })
);

const noProxyRules = (process.env['no_proxy'] ?? '').split(',').map( rule => rule.trim() );

const defaultDispatcher = getGlobalDispatcher();

setGlobalDispatcher(new class extends Dispatcher {

  dispatch(options, handler) {
    if (options.origin ){
      const {host, protocol} = typeof options.origin === 'string' ? new URL(options.origin) : options.origin;
      if (! noProxyRules.some( rule => rule.startsWith('.') ? host.endsWith(rule) : host === rule )){
        const proxyAgent = proxyAgents[ protocol ];
        if ( proxyAgent ) {
          proxyAgent.dispatch( options, handler );
        }
      }
    }
    return defaultDispatcher.dispatch(options, handler);
  }

});

I think that this feature should be treated as plugin and loaded by node "preload" argument:

node -r undici/register-http-proxy ./script.js

[silverwind]

Seems like a start, but it's definitely missing some features like wildcard (*) support in no_proxy.

I think that this feature should be treated as plugin and loaded by node "preload"

I disagree. Numerous environments support these variables by default, like Deno, Python, Golang and many more. It should of course be considered a breaking change, but I don't see why it should not be the default. Some kind of opt-out would be good to have, but I guess users could always overwrite the env with no_proxy=* I guess.

Maybe the implementation here could be "inspired" by Deno's 😉.

[jaecktec]

@sla100 my 2 cents on this line:

 return [`${protocol}:`, new ProxyAgent(uri)];
 ...
 const proxyAgent = proxyAgents[ protocol ];

you can also do http over https, that's why most frameworks specify the https option as optional. Meaning that if you haven't specified https_proxy it would do HTTP over the http-proxy :)

that's why I propose smth like this:

const getProxyAgent = (proto: 'http' | 'https') => {
  let agent: ProxyAgent | undefined;
  if (proto === 'http') {
    agent = process.env['https_proxy'] ? new ProxyAgent(process.env['https_proxy']) : undefined;
  }
  if (!agent) {
    agent = process.env['http_proxy'] ? new ProxyAgent(process.env['http_proxy']) : undefined;
  }
  return agent;
};

const noProxyRules = (process.env['no_proxy'] ?? '').split(',').map(rule => rule.trim());

const defaultDispatcher = getGlobalDispatcher();

setGlobalDispatcher(new class extends Dispatcher {

  dispatch(options, handler) {
    if (options.origin) {
      const {
        host,
        protocol,
      } = typeof options.origin === 'string' ? new URL(options.origin) : options.origin;
      if (!noProxyRules.some(rule => rule.startsWith('.') ? host.endsWith(rule) : host === rule)) {
        const proxyAgent = getProxyAgent(protocol);
        if (proxyAgent) {
          proxyAgent.dispatch(options, handler);
        }
      }
    }
    return defaultDispatcher.dispatch(options, handler);
  }
});

[mcollina]

I'd be ok to support:

import { setGlobalDispatcher, EnvHttpProxyAgent } from 'undici'

setGlobalDispatcher(new EnvHttpProxyAgent())

[mcollina]

Here is an additional data point in favor of this: Deno supports it by default.

https://deno.land/manual@v1.29.4/getting_started/setup_your_environment#environment-variables

We should check if Python, Ruby and other runtimes supports this by default.