From 2215a3ea2e79dd579388872d6e12e6ee987e745b Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Tue, 10 Oct 2023 19:32:22 +0100 Subject: [PATCH] fix(lsp): normalize "deno:" urls statelessly (#20867) --- cli/lsp/urls.rs | 100 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 72 insertions(+), 28 deletions(-) diff --git a/cli/lsp/urls.rs b/cli/lsp/urls.rs index f5c1702772..c3bd381d44 100644 --- a/cli/lsp/urls.rs +++ b/cli/lsp/urls.rs @@ -24,8 +24,6 @@ pub static INVALID_SPECIFIER: Lazy = /// the component percent encoding set. /// /// See: -/// -// TODO(@kitsonk) - refactor when #9934 is landed. const COMPONENT: &percent_encoding::AsciiSet = &percent_encoding::CONTROLS .add(b' ') .add(b'"') @@ -47,6 +45,7 @@ const COMPONENT: &percent_encoding::AsciiSet = &percent_encoding::CONTROLS .add(b'^') .add(b'|') .add(b'$') + .add(b'%') .add(b'&') .add(b'+') .add(b','); @@ -60,6 +59,43 @@ fn hash_data_specifier(specifier: &ModuleSpecifier) -> String { crate::util::checksum::gen(&[file_name_str.as_bytes()]) } +fn to_deno_url(specifier: &Url) -> String { + let mut string = String::with_capacity(specifier.as_str().len() + 6); + string.push_str("deno:/"); + string.push_str(specifier.scheme()); + for p in specifier[Position::BeforeHost..].split('/') { + string.push('/'); + string.push_str( + &percent_encoding::utf8_percent_encode(p, COMPONENT).to_string(), + ); + } + string +} + +fn from_deno_url(url: &Url) -> Option { + if url.scheme() != "deno" { + return None; + } + let mut segments = url.path_segments()?; + let mut string = String::with_capacity(url.as_str().len()); + string.push_str(segments.next()?); + string.push_str("://"); + string.push_str( + &percent_encoding::percent_decode(segments.next()?.as_bytes()) + .decode_utf8() + .ok()?, + ); + for segment in segments { + string.push('/'); + string.push_str( + &percent_encoding::percent_decode(segment.as_bytes()) + .decode_utf8() + .ok()?, + ); + } + Url::parse(&string).ok() +} + /// This exists to make it a little bit harder to accidentally use a `Url` /// in the wrong place where a client url should be used. #[derive(Debug, Clone, Hash, PartialEq, Eq, Ord, PartialOrd)] @@ -171,16 +207,7 @@ impl LspUrlMap { extension ) } else { - let mut str = String::with_capacity(specifier.as_str().len() + 6); - str.push_str("deno:/"); - str.push_str(specifier.scheme()); - for p in specifier[Position::BeforeHost..].split('/') { - str.push('/'); - str.push_str( - &percent_encoding::utf8_percent_encode(p, COMPONENT).to_string(), - ); - } - str + to_deno_url(specifier) }; let url = LspClientUrl(Url::parse(&specifier_str)?); inner.put(specifier.clone(), url.clone()); @@ -210,23 +237,22 @@ impl LspUrlMap { } let mut inner = self.inner.lock(); if let Some(specifier) = inner.get_specifier(url).cloned() { - specifier - } else { - let specifier = if url.scheme() == "file" { - if let Ok(path) = url.to_file_path() { - match kind { - LspUrlKind::Folder => Url::from_directory_path(path).unwrap(), - LspUrlKind::File => Url::from_file_path(path).unwrap(), - } - } else { - url.clone() - } - } else { - url.clone() - }; - inner.put(specifier.clone(), LspClientUrl(url.clone())); - specifier + return specifier; } + let mut specifier = None; + if url.scheme() == "file" { + if let Ok(path) = url.to_file_path() { + specifier = Some(match kind { + LspUrlKind::Folder => Url::from_directory_path(path).unwrap(), + LspUrlKind::File => Url::from_file_path(path).unwrap(), + }); + } + } else if let Some(s) = from_deno_url(url) { + specifier = Some(s); + } + let specifier = specifier.unwrap_or_else(|| url.clone()); + inner.put(specifier.clone(), LspClientUrl(url.clone())); + specifier } } @@ -261,6 +287,24 @@ mod tests { assert_eq!(actual_specifier, fixture); } + #[test] + fn test_lsp_url_reverse() { + let map = LspUrlMap::default(); + let fixture = + resolve_url("deno:/https/deno.land/x/pkg%401.0.0/mod.ts").unwrap(); + let actual_specifier = map.normalize_url(&fixture, LspUrlKind::File); + let expected_specifier = + Url::parse("https://deno.land/x/pkg@1.0.0/mod.ts").unwrap(); + assert_eq!(&actual_specifier, &expected_specifier); + + let actual_url = map + .normalize_specifier(&actual_specifier) + .unwrap() + .as_url() + .clone(); + assert_eq!(actual_url, fixture); + } + #[test] fn test_lsp_url_map_complex_encoding() { // Test fix for #9741 - not properly encoding certain URLs