fixup! http: add http.setGlobalProxyFromEnv()

Author: joyeecheung

doc/api/http.md

@@ -4361,15 +4361,16 @@ added:
 
 Set the maximum number of idle HTTP parsers.
 
-## `http.setGlobalProxyFromEnv(proxyEnv)`
+## `http.setGlobalProxyFromEnv([proxyEnv])`
 
 <!-- YAML
 added:
   - REPLACEME
 -->
 
 * `proxyEnv` {Object} An object containing proxy configuration. This accepts the
-  same options as the `proxyEnv` option accepted by [`Agent`][]
+  same options as the `proxyEnv` option accepted by [`Agent`][]. **Default:**
+  `process.env`.
 * Returns: {Function} A function that restores the original agent and dispatcher
   settings to the state before this `http.setGlobalProxyFromEnv()` is invoked.
 
@@ -4466,7 +4467,47 @@ Or the `--use-env-proxy` flag.
 HTTP_PROXY=http://proxy.example.com:8080 NO_PROXY=localhost,127.0.0.1 node --use-env-proxy client.js
 ```
 
-To enable proxy support dynamically and globally:
+To enable proxy support dynamically and globally with `process.env` (the default option of `http.setGlobalProxyFromEnv()`):
+
+```cjs
+const http = require('node:http');
+
+// Reads proxy-related environment variables from process.env
+const restore = http.setGlobalProxyFromEnv();
+
+// Subsequent requests will use the configured proxies from environment variables
+http.get('http://www.example.com', (res) => {
+  // This request will be proxied if HTTP_PROXY or http_proxy is set
+});
+
+fetch('https://www.example.com', (res) => {
+  // This request will be proxied if HTTPS_PROXY or https_proxy is set
+});
+
+// To restore the original global agent and dispatcher settings, call the returned function.
+// restore();
+```
+
+```mjs
+import http from 'node:http';
+
+// Reads proxy-related environment variables from process.env
+http.setGlobalProxyFromEnv();
+
+// Subsequent requests will use the configured proxies from environment variables
+http.get('http://www.example.com', (res) => {
+  // This request will be proxied if HTTP_PROXY or http_proxy is set
+});
+
+fetch('https://www.example.com', (res) => {
+  // This request will be proxied if HTTPS_PROXY or https_proxy is set
+});
+
+// To restore the original global agent and dispatcher settings, call the returned function.
+// restore();
+```
+
+To enable proxy support dynamically and globally with custom settings:
 
 ```cjs
 const http = require('node:http');
@@ -4506,25 +4547,6 @@ fetch('https://www.example.com', (res) => {
 });
 ```
 
-To clear the dynamically enabled global proxy configuration:
-
-```cjs
-const http = require('node:http');
-const restore = http.setGlobalProxyFromEnv({ /* ... */ });
-// Perform requests that will be proxied.
-restore();
-// From now on, requests will no longer be proxied.
-```
-
-```mjs
-import http from 'node:http';
-
-const restore = http.setGlobalProxyFromEnv({ /* ... */ });
-// Perform requests that will be proxied.
-restore();
-// From now on, requests will no longer be proxied.
-```
-
 To create a custom agent with built-in proxy support:
 
 ```cjs

lib/http.js

@@ -25,7 +25,7 @@ const {
   ObjectDefineProperty,
 } = primordials;
 
-const { validateInteger } = require('internal/validators');
+const { validateInteger, validateObject } = require('internal/validators');
 const httpAgent = require('_http_agent');
 const { ClientRequest } = require('_http_client');
 const { methods, parsers } = require('_http_common');
@@ -129,7 +129,8 @@ function lazyUndici() {
   return undici ??= require('internal/deps/undici/undici');
 }
 
-function setGlobalProxyFromEnv(env) {
+function setGlobalProxyFromEnv(env = process.env) {
+  validateObject(env, 'proxyEnv');
   const httpProxy = parseProxyUrl(env, 'http:');
   const httpsProxy = parseProxyUrl(env, 'https:');
   const noProxy = env.no_proxy || env.NO_PROXY;

test/client-proxy/test-http-set-global-proxy-from-env-fetch-default.mjs

@@ -0,0 +1,35 @@
+// Tests that http.setGlobalProxyFromEnv() without arguments uses process.env.
+
+import '../common/index.mjs';
+import assert from 'node:assert';
+import { startTestServers, checkProxiedFetch } from '../common/proxy-server.js';
+
+const { proxyLogs, proxyUrl, shutdown, httpEndpoint: { serverHost, requestUrl } } = await startTestServers({
+  httpEndpoint: true,
+});
+
+// Test that calling setGlobalProxyFromEnv() without arguments uses process.env
+await checkProxiedFetch({
+  FETCH_URL: requestUrl,
+  // Set the proxy in the environment instead of passing it to SET_GLOBAL_PROXY
+  http_proxy: proxyUrl,
+  SET_GLOBAL_PROXY_DEFAULT: '1',  // Signal to call without arguments
+}, {
+  stdout: 'Hello world',
+});
+
+shutdown();
+
+// FIXME(undici:4083): undici currently always tunnels the request over
+// CONNECT if proxyTunnel is not explicitly set to false.
+const expectedLogs = [{
+  method: 'CONNECT',
+  url: serverHost,
+  headers: {
+    'connection': 'close',
+    'host': serverHost,
+    'proxy-connection': 'keep-alive',
+  },
+}];
+
+assert.deepStrictEqual(proxyLogs, expectedLogs);

test/client-proxy/test-http-set-global-proxy-from-env-http-request-default.mjs

@@ -0,0 +1,33 @@
+// Tests that http.setGlobalProxyFromEnv() without arguments uses process.env.
+
+import '../common/index.mjs';
+import assert from 'node:assert';
+import { startTestServers, checkProxiedRequest } from '../common/proxy-server.js';
+
+const { proxyLogs, proxyUrl, shutdown, httpEndpoint: { serverHost, requestUrl } } = await startTestServers({
+  httpEndpoint: true,
+});
+
+// Test that calling setGlobalProxyFromEnv() without arguments uses process.env
+await checkProxiedRequest({
+  REQUEST_URL: requestUrl,
+  // Set the proxy in the environment instead of passing it to SET_GLOBAL_PROXY
+  http_proxy: proxyUrl,
+  SET_GLOBAL_PROXY_DEFAULT: '1',  // Signal to call without arguments
+}, {
+  stdout: 'Hello world',
+});
+
+shutdown();
+
+const expectedLogs = [{
+  method: 'GET',
+  url: requestUrl,
+  headers: {
+    'connection': 'keep-alive',
+    'proxy-connection': 'keep-alive',
+    'host': serverHost,
+  },
+}];
+
+assert.deepStrictEqual(proxyLogs, expectedLogs);

test/client-proxy/test-http-set-global-proxy-from-env-invalid-type.mjs

@@ -0,0 +1,15 @@
+// Tests that http.setGlobalProxyFromEnv() validates that proxyEnv is an object.
+
+import '../common/index.mjs';
+import assert from 'node:assert';
+import http from 'node:http';
+
+for (const invalidInput of [42, 'string', null, [], true]) {
+  assert.throws(
+    () => http.setGlobalProxyFromEnv(invalidInput),
+    {
+      code: 'ERR_INVALID_ARG_TYPE',
+      name: 'TypeError',
+    },
+  );
+}

test/fixtures/fetch-and-log.mjs

@@ -4,6 +4,8 @@ let restore;
 if (process.env.SET_GLOBAL_PROXY) {
   const config = JSON.parse(process.env.SET_GLOBAL_PROXY);
   restore = http.setGlobalProxyFromEnv(config);
+} else if (process.env.SET_GLOBAL_PROXY_DEFAULT) {
+  restore = http.setGlobalProxyFromEnv();
 }
 
 const response = await fetch(process.env.FETCH_URL);

test/fixtures/request-and-log.js

@@ -2,6 +2,8 @@ let restore;
 if (process.env.SET_GLOBAL_PROXY) {
   const config = JSON.parse(process.env.SET_GLOBAL_PROXY);
   restore = require('http').setGlobalProxyFromEnv(config);
+} else if (process.env.SET_GLOBAL_PROXY_DEFAULT) {
+  restore = require('http').setGlobalProxyFromEnv();
 }
 
 const url = process.env.REQUEST_URL;