feat(install): follow redirects for urls with no path (#17449)

This change makes absolute urls, that contain no path like `deno install
https://my-cli.io` to follow redirects and extract the name from it.

It allows modifies `test_util` server listener on port `4550`
(`REDIRECT_ABSOLUTE_PORT`) to allow for specifying `redirect_to` query
param, that fill use that value for it's next redirect.

Fixes https://github.com/denoland/deno/issues/17409
This commit is contained in:
Kamil Ogórek 2023-02-10 16:11:11 +01:00 committed by GitHub
parent 4baaa246a2
commit 5778e1196e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 199 additions and 86 deletions

View file

@ -300,7 +300,7 @@ impl HttpClient {
.map(Some)
}
async fn get_redirected_response<U: reqwest::IntoUrl>(
pub async fn get_redirected_response<U: reqwest::IntoUrl>(
&self,
url: U,
) -> Result<Response, AnyError> {

View file

@ -6,6 +6,7 @@ use crate::args::ConfigFlag;
use crate::args::Flags;
use crate::args::InstallFlags;
use crate::args::TypeCheckMode;
use crate::http_util::HttpClient;
use crate::npm::NpmPackageReference;
use crate::proc_state::ProcState;
use crate::util::fs::canonicalize_path_maybe_not_exists;
@ -125,8 +126,19 @@ fn get_installer_root() -> Result<PathBuf, io::Error> {
Ok(home_path)
}
pub fn infer_name_from_url(url: &Url) -> Option<String> {
if let Ok(npm_ref) = NpmPackageReference::from_specifier(url) {
pub async fn infer_name_from_url(url: &Url) -> Option<String> {
// If there's an absolute url with no path, eg. https://my-cli.com
// perform a request, and see if it redirects another file instead.
let mut url = url.clone();
if url.path() == "/" {
let client = HttpClient::new(None, None).unwrap();
if let Ok(res) = client.get_redirected_response(url.clone()).await {
url = res.url().clone();
}
}
if let Ok(npm_ref) = NpmPackageReference::from_specifier(&url) {
if let Some(sub_path) = npm_ref.sub_path {
if !sub_path.contains('/') {
return Some(sub_path);
@ -226,14 +238,14 @@ pub async fn install_command(
.await?;
// create the install shim
create_install_shim(flags, install_flags)
create_install_shim(flags, install_flags).await
}
fn create_install_shim(
async fn create_install_shim(
flags: Flags,
install_flags: InstallFlags,
) -> Result<(), AnyError> {
let shim_data = resolve_shim_data(&flags, &install_flags)?;
let shim_data = resolve_shim_data(&flags, &install_flags).await?;
// ensure directory exists
if let Ok(metadata) = fs::metadata(&shim_data.installation_dir) {
@ -283,7 +295,7 @@ struct ShimData {
extra_files: Vec<(PathBuf, String)>,
}
fn resolve_shim_data(
async fn resolve_shim_data(
flags: &Flags,
install_flags: &InstallFlags,
) -> Result<ShimData, AnyError> {
@ -297,10 +309,11 @@ fn resolve_shim_data(
// Check if module_url is remote
let module_url = resolve_url_or_path(&install_flags.module_url)?;
let name = install_flags
.name
.clone()
.or_else(|| infer_name_from_url(&module_url));
let name = if install_flags.name.is_some() {
install_flags.name.clone()
} else {
infer_name_from_url(&module_url).await
};
let name = match name {
Some(name) => name,
@ -479,115 +492,131 @@ mod tests {
use test_util::testdata_path;
use test_util::TempDir;
#[test]
fn install_infer_name_from_url() {
#[tokio::test]
async fn install_infer_name_from_url() {
assert_eq!(
infer_name_from_url(
&Url::parse("https://example.com/abc/server.ts").unwrap()
),
)
.await,
Some("server".to_string())
);
assert_eq!(
infer_name_from_url(
&Url::parse("https://example.com/abc/main.ts").unwrap()
),
)
.await,
Some("abc".to_string())
);
assert_eq!(
infer_name_from_url(
&Url::parse("https://example.com/abc/mod.ts").unwrap()
),
)
.await,
Some("abc".to_string())
);
assert_eq!(
infer_name_from_url(
&Url::parse("https://example.com/abc/index.ts").unwrap()
),
)
.await,
Some("abc".to_string())
);
assert_eq!(
infer_name_from_url(
&Url::parse("https://example.com/abc/cli.ts").unwrap()
),
)
.await,
Some("abc".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("https://example.com/main.ts").unwrap()),
infer_name_from_url(&Url::parse("https://example.com/main.ts").unwrap())
.await,
Some("main".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("https://example.com").unwrap()),
infer_name_from_url(&Url::parse("https://example.com").unwrap()).await,
None
);
assert_eq!(
infer_name_from_url(&Url::parse("file:///abc/server.ts").unwrap()),
infer_name_from_url(&Url::parse("file:///abc/server.ts").unwrap()).await,
Some("server".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("file:///abc/main.ts").unwrap()),
infer_name_from_url(&Url::parse("file:///abc/main.ts").unwrap()).await,
Some("abc".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("file:///main.ts").unwrap()),
infer_name_from_url(&Url::parse("file:///main.ts").unwrap()).await,
Some("main".to_string())
);
assert_eq!(infer_name_from_url(&Url::parse("file:///").unwrap()), None);
assert_eq!(
infer_name_from_url(&Url::parse("file:///").unwrap()).await,
None
);
assert_eq!(
infer_name_from_url(
&Url::parse("https://example.com/abc@0.1.0").unwrap()
),
)
.await,
Some("abc".to_string())
);
assert_eq!(
infer_name_from_url(
&Url::parse("https://example.com/abc@0.1.0/main.ts").unwrap()
),
)
.await,
Some("abc".to_string())
);
assert_eq!(
infer_name_from_url(
&Url::parse("https://example.com/abc@def@ghi").unwrap()
),
)
.await,
Some("abc".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("https://example.com/@abc.ts").unwrap()),
infer_name_from_url(&Url::parse("https://example.com/@abc.ts").unwrap())
.await,
Some("@abc".to_string())
);
assert_eq!(
infer_name_from_url(
&Url::parse("https://example.com/@abc/mod.ts").unwrap()
),
)
.await,
Some("@abc".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("file:///@abc.ts").unwrap()),
infer_name_from_url(&Url::parse("file:///@abc.ts").unwrap()).await,
Some("@abc".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("file:///@abc/cli.ts").unwrap()),
infer_name_from_url(&Url::parse("file:///@abc/cli.ts").unwrap()).await,
Some("@abc".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("npm:cowsay@1.2/cowthink").unwrap()),
infer_name_from_url(&Url::parse("npm:cowsay@1.2/cowthink").unwrap())
.await,
Some("cowthink".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("npm:cowsay@1.2/cowthink/test").unwrap()),
infer_name_from_url(&Url::parse("npm:cowsay@1.2/cowthink/test").unwrap())
.await,
Some("cowsay".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("npm:cowsay@1.2").unwrap()),
infer_name_from_url(&Url::parse("npm:cowsay@1.2").unwrap()).await,
Some("cowsay".to_string())
);
assert_eq!(
infer_name_from_url(&Url::parse("npm:@types/node@1.2").unwrap()),
infer_name_from_url(&Url::parse("npm:@types/node@1.2").unwrap()).await,
None
);
}
#[test]
fn install_unstable() {
#[tokio::test]
async fn install_unstable() {
let temp_dir = TempDir::new();
let bin_dir = temp_dir.path().join("bin");
std::fs::create_dir(&bin_dir).unwrap();
@ -605,6 +634,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
let mut file_path = bin_dir.join("echo_test");
@ -626,8 +656,8 @@ mod tests {
}
}
#[test]
fn install_inferred_name() {
#[tokio::test]
async fn install_inferred_name() {
let shim_data = resolve_shim_data(
&Flags::default(),
&InstallFlags {
@ -638,6 +668,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
assert_eq!(shim_data.name, "echo_server");
@ -647,8 +678,8 @@ mod tests {
);
}
#[test]
fn install_inferred_name_from_parent() {
#[tokio::test]
async fn install_inferred_name_from_parent() {
let shim_data = resolve_shim_data(
&Flags::default(),
&InstallFlags {
@ -659,6 +690,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
assert_eq!(shim_data.name, "subdir");
@ -668,8 +700,36 @@ mod tests {
);
}
#[test]
fn install_custom_dir_option() {
#[tokio::test]
async fn install_inferred_name_after_redirect_for_no_path_url() {
let _http_server_guard = test_util::http_server();
let shim_data = resolve_shim_data(
&Flags::default(),
&InstallFlags {
module_url: "http://localhost:4550/?redirect_to=/subdir/redirects/a.ts"
.to_string(),
args: vec![],
name: None,
root: Some(env::temp_dir()),
force: false,
},
)
.await
.unwrap();
assert_eq!(shim_data.name, "a");
assert_eq!(
shim_data.args,
vec![
"run",
"--no-config",
"http://localhost:4550/?redirect_to=/subdir/redirects/a.ts",
]
);
}
#[tokio::test]
async fn install_custom_dir_option() {
let shim_data = resolve_shim_data(
&Flags::default(),
&InstallFlags {
@ -680,6 +740,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
assert_eq!(shim_data.name, "echo_test");
@ -689,8 +750,8 @@ mod tests {
);
}
#[test]
fn install_with_flags() {
#[tokio::test]
async fn install_with_flags() {
let shim_data = resolve_shim_data(
&Flags {
allow_net: Some(vec![]),
@ -707,6 +768,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
assert_eq!(shim_data.name, "echo_test");
@ -724,8 +786,8 @@ mod tests {
);
}
#[test]
fn install_prompt() {
#[tokio::test]
async fn install_prompt() {
let shim_data = resolve_shim_data(
&Flags {
no_prompt: true,
@ -739,6 +801,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
assert_eq!(
@ -752,8 +815,8 @@ mod tests {
);
}
#[test]
fn install_allow_all() {
#[tokio::test]
async fn install_allow_all() {
let shim_data = resolve_shim_data(
&Flags {
allow_all: true,
@ -767,6 +830,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
assert_eq!(
@ -780,8 +844,8 @@ mod tests {
);
}
#[test]
fn install_npm_lockfile_default() {
#[tokio::test]
async fn install_npm_lockfile_default() {
let temp_dir = canonicalize_path(&env::temp_dir()).unwrap();
let shim_data = resolve_shim_data(
&Flags {
@ -796,6 +860,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
let lock_path = temp_dir.join("bin").join(".cowsay.lock.json");
@ -813,8 +878,8 @@ mod tests {
assert_eq!(shim_data.extra_files, vec![(lock_path, "{}".to_string())]);
}
#[test]
fn install_npm_no_lock() {
#[tokio::test]
async fn install_npm_no_lock() {
let shim_data = resolve_shim_data(
&Flags {
allow_all: true,
@ -829,6 +894,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
assert_eq!(
@ -844,8 +910,8 @@ mod tests {
assert_eq!(shim_data.extra_files, vec![]);
}
#[test]
fn install_local_module() {
#[tokio::test]
async fn install_local_module() {
let temp_dir = TempDir::new();
let bin_dir = temp_dir.path().join("bin");
std::fs::create_dir(&bin_dir).unwrap();
@ -863,6 +929,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
let mut file_path = bin_dir.join("echo_test");
@ -875,8 +942,8 @@ mod tests {
assert!(content.contains(&local_module_url.to_string()));
}
#[test]
fn install_force() {
#[tokio::test]
async fn install_force() {
let temp_dir = TempDir::new();
let bin_dir = temp_dir.path().join("bin");
std::fs::create_dir(&bin_dir).unwrap();
@ -891,6 +958,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
let mut file_path = bin_dir.join("echo_test");
@ -909,7 +977,8 @@ mod tests {
root: Some(temp_dir.path().to_path_buf()),
force: false,
},
);
)
.await;
assert!(no_force_result.is_err());
assert!(no_force_result
.unwrap_err()
@ -929,15 +998,16 @@ mod tests {
root: Some(temp_dir.path().to_path_buf()),
force: true,
},
);
)
.await;
assert!(force_result.is_ok());
// Assert modified
let file_content_2 = fs::read_to_string(&file_path).unwrap();
assert!(file_content_2.contains("cat.ts"));
}
#[test]
fn install_with_config() {
#[tokio::test]
async fn install_with_config() {
let temp_dir = TempDir::new();
let bin_dir = temp_dir.path().join("bin");
let config_file_path = temp_dir.path().join("test_tsconfig.json");
@ -960,7 +1030,8 @@ mod tests {
root: Some(temp_dir.path().to_path_buf()),
force: true,
},
);
)
.await;
assert!(result.is_ok());
let config_file_name = ".echo_test.deno.json";
@ -973,8 +1044,8 @@ mod tests {
// TODO: enable on Windows after fixing batch escaping
#[cfg(not(windows))]
#[test]
fn install_shell_escaping() {
#[tokio::test]
async fn install_shell_escaping() {
let temp_dir = TempDir::new();
let bin_dir = temp_dir.path().join("bin");
std::fs::create_dir(&bin_dir).unwrap();
@ -989,6 +1060,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
let mut file_path = bin_dir.join("echo_test");
@ -1007,8 +1079,8 @@ mod tests {
}
}
#[test]
fn install_unicode() {
#[tokio::test]
async fn install_unicode() {
let temp_dir = TempDir::new();
let bin_dir = temp_dir.path().join("bin");
std::fs::create_dir(&bin_dir).unwrap();
@ -1028,6 +1100,7 @@ mod tests {
force: false,
},
)
.await
.unwrap();
let mut file_path = bin_dir.join("echo_test");
@ -1047,8 +1120,8 @@ mod tests {
assert!(status.success());
}
#[test]
fn install_with_import_map() {
#[tokio::test]
async fn install_with_import_map() {
let temp_dir = TempDir::new();
let bin_dir = temp_dir.path().join("bin");
let import_map_path = temp_dir.path().join("import_map.json");
@ -1070,7 +1143,8 @@ mod tests {
root: Some(temp_dir.path().to_path_buf()),
force: true,
},
);
)
.await;
assert!(result.is_ok());
let mut file_path = bin_dir.join("echo_test");
@ -1093,8 +1167,8 @@ mod tests {
}
// Regression test for https://github.com/denoland/deno/issues/10556.
#[test]
fn install_file_url() {
#[tokio::test]
async fn install_file_url() {
let temp_dir = TempDir::new();
let bin_dir = temp_dir.path().join("bin");
let module_path = fs::canonicalize(testdata_path().join("cat.ts")).unwrap();
@ -1111,7 +1185,8 @@ mod tests {
root: Some(temp_dir.path().to_path_buf()),
force: true,
},
);
)
.await;
assert!(result.is_ok());
let mut file_path = bin_dir.join("echo_test");

View file

@ -42,7 +42,8 @@ pub async fn compile(
let module_specifier = resolve_url_or_path(&compile_flags.source_file)?;
let deno_dir = &ps.dir;
let output_path = resolve_compile_executable_output_path(&compile_flags)?;
let output_path =
resolve_compile_executable_output_path(&compile_flags).await?;
let graph = Arc::try_unwrap(
create_graph_and_maybe_check(module_specifier.clone(), &ps).await?,
@ -279,20 +280,33 @@ async fn write_standalone_binary(
Ok(())
}
fn resolve_compile_executable_output_path(
async fn resolve_compile_executable_output_path(
compile_flags: &CompileFlags,
) -> Result<PathBuf, AnyError> {
let module_specifier = resolve_url_or_path(&compile_flags.source_file)?;
compile_flags.output.as_ref().and_then(|output| {
if path_has_trailing_slash(output) {
let infer_file_name = infer_name_from_url(&module_specifier).map(PathBuf::from)?;
Some(output.join(infer_file_name))
let mut output = compile_flags.output.clone();
if let Some(out) = output.as_ref() {
if path_has_trailing_slash(out) {
if let Some(infer_file_name) = infer_name_from_url(&module_specifier)
.await
.map(PathBuf::from)
{
output = Some(out.join(infer_file_name));
}
} else {
Some(output.to_path_buf())
output = Some(out.to_path_buf());
}
}).or_else(|| {
infer_name_from_url(&module_specifier).map(PathBuf::from)
}).ok_or_else(|| generic_error(
}
if output.is_none() {
output = infer_name_from_url(&module_specifier)
.await
.map(PathBuf::from)
}
output.ok_or_else(|| generic_error(
"An executable name was not provided. One could not be inferred from the URL. Aborting.",
)).map(|output| {
get_os_specific_filepath(output, &compile_flags.target)
@ -323,14 +337,15 @@ fn get_os_specific_filepath(
mod test {
pub use super::*;
#[test]
fn resolve_compile_executable_output_path_target_linux() {
#[tokio::test]
async fn resolve_compile_executable_output_path_target_linux() {
let path = resolve_compile_executable_output_path(&CompileFlags {
source_file: "mod.ts".to_string(),
output: Some(PathBuf::from("./file")),
args: Vec::new(),
target: Some("x86_64-unknown-linux-gnu".to_string()),
})
.await
.unwrap();
// no extension, no matter what the operating system is
@ -339,14 +354,15 @@ mod test {
assert_eq!(path.file_name().unwrap(), "file");
}
#[test]
fn resolve_compile_executable_output_path_target_windows() {
#[tokio::test]
async fn resolve_compile_executable_output_path_target_windows() {
let path = resolve_compile_executable_output_path(&CompileFlags {
source_file: "mod.ts".to_string(),
output: Some(PathBuf::from("./file")),
args: Vec::new(),
target: Some("x86_64-pc-windows-msvc".to_string()),
})
.await
.unwrap();
assert_eq!(path.file_name().unwrap(), "file.exe");
}

View file

@ -594,6 +594,28 @@ async fn absolute_redirect(
) -> hyper::Result<Response<Body>> {
let path = req.uri().path();
if path == "/" {
// We have to manually extract query params here,
// as `req.uri()` returns `PathAndQuery` only,
// and we cannot use `Url::parse(req.uri()).query_pairs()`,
// as it requires url to have a proper base.
let query_params: HashMap<_, _> = req
.uri()
.query()
.unwrap_or_default()
.split('&')
.filter_map(|s| {
s.split_once('=').map(|t| (t.0.to_owned(), t.1.to_owned()))
})
.collect();
if let Some(url) = query_params.get("redirect_to") {
println!("URL: {url:?}");
let redirect = redirect_resp(url.to_owned());
return Ok(redirect);
}
}
if path.starts_with("/REDIRECT") {
let url = &req.uri().path()[9..];
println!("URL: {url:?}");