From ba7ea71c00f228a2d7580c63ebc57305ed76342b Mon Sep 17 00:00:00 2001 From: tidely <43219534+tidely@users.noreply.github.com> Date: Wed, 5 Nov 2025 00:11:54 +0200 Subject: [PATCH] node_runtime: Improve proxy mapping (#41807) Closes #ISSUE More accurately map localhost to `127.0.0.1`. Previously we would lowercase and simply replace all instances of localhost inside of the URL string, meaning query parameters, username, password etc. could not contain the string `localhost` or contain uppercase letters without getting modified. Added a test ensuring the mapping logic works. The previous implementation would fail this new test. Release Notes: - Improved the behavior of mapping `localhost` to `127.0.0.1` when passing configured proxy urls to `node` --- crates/http_client/src/http_client.rs | 2 +- crates/node_runtime/src/node_runtime.rs | 59 ++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/crates/http_client/src/http_client.rs b/crates/http_client/src/http_client.rs index 056cee4e346e34b5689a0dfe3278c880b7297986..6050d75c3edc8aefb1122df6e6af3bf078673217 100644 --- a/crates/http_client/src/http_client.rs +++ b/crates/http_client/src/http_client.rs @@ -16,7 +16,7 @@ use parking_lot::Mutex; #[cfg(feature = "test-support")] use std::fmt; use std::{any::type_name, sync::Arc}; -pub use url::Url; +pub use url::{Host, Url}; #[derive(Default, Debug, Clone, PartialEq, Eq, Hash)] pub enum RedirectPolicy { diff --git a/crates/node_runtime/src/node_runtime.rs b/crates/node_runtime/src/node_runtime.rs index 3b2ba83ec339c13c3f00cc58d0fcbdffe2efd915..babdf56117a9fa42c4dee7ca8653728d1d0203e1 100644 --- a/crates/node_runtime/src/node_runtime.rs +++ b/crates/node_runtime/src/node_runtime.rs @@ -2,7 +2,7 @@ use anyhow::{Context as _, Result, anyhow, bail}; use async_compression::futures::bufread::GzipDecoder; use async_tar::Archive; use futures::{AsyncReadExt, FutureExt as _, channel::oneshot, future::Shared}; -use http_client::{HttpClient, Url}; +use http_client::{Host, HttpClient, Url}; use log::Level; use semver::Version; use serde::Deserialize; @@ -13,6 +13,7 @@ use std::{ env::{self, consts}, ffi::OsString, io, + net::{IpAddr, Ipv4Addr}, path::{Path, PathBuf}, process::Output, sync::Arc, @@ -799,17 +800,18 @@ fn configure_npm_command( command.args(["--prefix".into(), directory.to_path_buf()]); } - if let Some(proxy) = proxy { + if let Some(mut proxy) = proxy.cloned() { // Map proxy settings from `http://localhost:10809` to `http://127.0.0.1:10809` // NodeRuntime without environment information can not parse `localhost` // correctly. // TODO: map to `[::1]` if we are using ipv6 - let proxy = proxy - .to_string() - .to_ascii_lowercase() - .replace("localhost", "127.0.0.1"); + if matches!(proxy.host(), Some(Host::Domain(domain)) if domain.eq_ignore_ascii_case("localhost")) + { + // When localhost is a valid Host, so is `127.0.0.1` + let _ = proxy.set_ip_host(IpAddr::V4(Ipv4Addr::LOCALHOST)); + } - command.args(["--proxy", &proxy]); + command.args(["--proxy", proxy.as_str()]); } #[cfg(windows)] @@ -830,3 +832,46 @@ fn configure_npm_command( } } } + +#[cfg(test)] +mod tests { + use http_client::Url; + + use super::configure_npm_command; + + // Map localhost to 127.0.0.1 + // NodeRuntime without environment information can not parse `localhost` correctly. + #[test] + fn test_configure_npm_command_map_localhost_proxy() { + const CASES: [(&str, &str); 4] = [ + // Map localhost to 127.0.0.1 + ("http://localhost:9090/", "http://127.0.0.1:9090/"), + ("https://google.com/", "https://google.com/"), + ( + "http://username:password@proxy.thing.com:8080/", + "http://username:password@proxy.thing.com:8080/", + ), + // Test when localhost is contained within a different part of the URL + ( + "http://username:localhost@localhost:8080/", + "http://username:localhost@127.0.0.1:8080/", + ), + ]; + + for (proxy, mapped_proxy) in CASES { + let mut dummy = smol::process::Command::new(""); + let proxy = Url::parse(proxy).unwrap(); + configure_npm_command(&mut dummy, None, Some(&proxy)); + let proxy = dummy + .get_args() + .skip_while(|&arg| arg != "--proxy") + .skip(1) + .next(); + let proxy = proxy.expect("Proxy was not passed to Command correctly"); + assert_eq!( + proxy, mapped_proxy, + "Incorrectly mapped localhost to 127.0.0.1" + ); + } + } +}