From ad30703e8e6bcf0fb87b5bc4ad1b49d040e54c77 Mon Sep 17 00:00:00 2001 From: Nayeem Rahman Date: Wed, 11 Sep 2024 11:11:39 +0100 Subject: [PATCH] fix(lsp): encode url parts before parsing as uri (#25509) --- cli/lsp/diagnostics.rs | 5 ++-- cli/lsp/repl.rs | 5 ++-- cli/lsp/testing/execution.rs | 5 ++-- cli/lsp/urls.rs | 48 ++++++++++++++++++++++++++++++++-- tests/integration/lsp_tests.rs | 45 +++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 10 deletions(-) diff --git a/cli/lsp/diagnostics.rs b/cli/lsp/diagnostics.rs index 9d9a0ae458..1aebaf56fc 100644 --- a/cli/lsp/diagnostics.rs +++ b/cli/lsp/diagnostics.rs @@ -12,6 +12,7 @@ use super::language_server::StateSnapshot; use super::performance::Performance; use super::tsc; use super::tsc::TsServer; +use super::urls::uri_parse_unencoded; use super::urls::url_to_uri; use super::urls::LspUrlMap; @@ -53,11 +54,9 @@ use deno_semver::package::PackageReq; use import_map::ImportMap; use import_map::ImportMapError; use log::error; -use lsp_types::Uri; use std::collections::HashMap; use std::collections::HashSet; use std::path::PathBuf; -use std::str::FromStr; use std::sync::atomic::AtomicUsize; use std::sync::Arc; use std::thread; @@ -738,7 +737,7 @@ fn to_lsp_related_information( if let (Some(file_name), Some(start), Some(end)) = (&ri.file_name, &ri.start, &ri.end) { - let uri = Uri::from_str(file_name).unwrap(); + let uri = uri_parse_unencoded(file_name).unwrap(); Some(lsp::DiagnosticRelatedInformation { location: lsp::Location { uri, diff --git a/cli/lsp/repl.rs b/cli/lsp/repl.rs index a238a22ccd..fa5809045e 100644 --- a/cli/lsp/repl.rs +++ b/cli/lsp/repl.rs @@ -1,7 +1,6 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use std::collections::HashMap; -use std::str::FromStr; use deno_ast::LineAndColumnIndex; use deno_ast::ModuleSpecifier; @@ -42,6 +41,7 @@ use super::config::LanguageWorkspaceSettings; use super::config::ObjectLiteralMethodSnippets; use super::config::TestingSettings; use super::config::WorkspaceSettings; +use super::urls::uri_parse_unencoded; use super::urls::url_to_uri; #[derive(Debug)] @@ -263,7 +263,8 @@ impl ReplLanguageServer { } fn get_document_uri(&self) -> Uri { - Uri::from_str(self.cwd_uri.join("$deno$repl.ts").unwrap().as_str()).unwrap() + uri_parse_unencoded(self.cwd_uri.join("$deno$repl.ts").unwrap().as_str()) + .unwrap() } } diff --git a/cli/lsp/testing/execution.rs b/cli/lsp/testing/execution.rs index a952f2c490..c2398d4ff2 100644 --- a/cli/lsp/testing/execution.rs +++ b/cli/lsp/testing/execution.rs @@ -12,6 +12,7 @@ use crate::lsp::client::Client; use crate::lsp::client::TestingNotification; use crate::lsp::config; use crate::lsp::logging::lsp_log; +use crate::lsp::urls::uri_parse_unencoded; use crate::lsp::urls::uri_to_url; use crate::lsp::urls::url_to_uri; use crate::tools::test; @@ -32,12 +33,10 @@ use deno_core::ModuleSpecifier; use deno_runtime::deno_permissions::Permissions; use deno_runtime::tokio_util::create_and_run_current_thread; use indexmap::IndexMap; -use lsp_types::Uri; use std::borrow::Cow; use std::collections::HashMap; use std::collections::HashSet; use std::num::NonZeroUsize; -use std::str::FromStr; use std::sync::Arc; use std::time::Duration; use std::time::Instant; @@ -536,7 +535,7 @@ impl LspTestDescription { &self, tests: &IndexMap, ) -> lsp_custom::TestIdentifier { - let uri = Uri::from_str(&self.location().file_name).unwrap(); + let uri = uri_parse_unencoded(&self.location().file_name).unwrap(); let static_id = self.static_id(); let mut root_desc = self; while let Some(parent_id) = root_desc.parent_id() { diff --git a/cli/lsp/urls.rs b/cli/lsp/urls.rs index 13e66dfe99..6c7da4f134 100644 --- a/cli/lsp/urls.rs +++ b/cli/lsp/urls.rs @@ -55,6 +55,25 @@ const COMPONENT: &percent_encoding::AsciiSet = &percent_encoding::CONTROLS .add(b'+') .add(b','); +/// Characters that may be left unencoded in a `Url` path but not valid in a +/// `Uri` path. +const URL_TO_URI_PATH: &percent_encoding::AsciiSet = + &percent_encoding::CONTROLS + .add(b'[') + .add(b']') + .add(b'^') + .add(b'|'); + +/// Characters that may be left unencoded in a `Url` query but not valid in a +/// `Uri` query. +const URL_TO_URI_QUERY: &percent_encoding::AsciiSet = + &URL_TO_URI_PATH.add(b'\\').add(b'`').add(b'{').add(b'}'); + +/// Characters that may be left unencoded in a `Url` fragment but not valid in +/// a `Uri` fragment. +const URL_TO_URI_FRAGMENT: &percent_encoding::AsciiSet = + &URL_TO_URI_PATH.add(b'#').add(b'\\').add(b'{').add(b'}'); + fn hash_data_specifier(specifier: &ModuleSpecifier) -> String { let mut file_name_str = specifier.path().to_string(); if let Some(query) = specifier.query() { @@ -122,8 +141,33 @@ impl LspUrlMapInner { } } +pub fn uri_parse_unencoded(s: &str) -> Result { + url_to_uri(&Url::parse(s)?) +} + pub fn url_to_uri(url: &Url) -> Result { - Ok(Uri::from_str(url.as_str()).inspect_err(|err| { + let components = deno_core::url::quirks::internal_components(url); + let mut input = String::with_capacity(url.as_str().len()); + input.push_str(&url.as_str()[..components.path_start as usize]); + input.push_str( + &percent_encoding::utf8_percent_encode(url.path(), URL_TO_URI_PATH) + .to_string(), + ); + if let Some(query) = url.query() { + input.push('?'); + input.push_str( + &percent_encoding::utf8_percent_encode(query, URL_TO_URI_QUERY) + .to_string(), + ); + } + if let Some(fragment) = url.fragment() { + input.push('#'); + input.push_str( + &percent_encoding::utf8_percent_encode(fragment, URL_TO_URI_FRAGMENT) + .to_string(), + ); + } + Ok(Uri::from_str(&input).inspect_err(|err| { lsp_warn!("Could not convert URL \"{url}\" to URI: {err}") })?) } @@ -189,7 +233,7 @@ impl LspUrlMap { } else { to_deno_uri(specifier) }; - let uri = Uri::from_str(&uri_str)?; + let uri = uri_parse_unencoded(&uri_str)?; inner.put(specifier.clone(), uri.clone()); uri }; diff --git a/tests/integration/lsp_tests.rs b/tests/integration/lsp_tests.rs index 4c8372df6e..cfd0da840b 100644 --- a/tests/integration/lsp_tests.rs +++ b/tests/integration/lsp_tests.rs @@ -9722,6 +9722,51 @@ fn lsp_lockfile_redirect_resolution() { client.shutdown(); } +// Regression test for https://github.com/denoland/vscode_deno/issues/1157. +#[test] +fn lsp_diagnostics_brackets_in_file_name() { + let context = TestContextBuilder::new().use_temp_cwd().build(); + let mut client = context.new_lsp_command().build(); + client.initialize_default(); + let diagnostics = client.did_open(json!({ + "textDocument": { + "uri": "file:///a/%5Bfile%5D.ts", + "languageId": "typescript", + "version": 1, + "text": "/** @deprecated */\nexport const a = \"a\";\n\na;\n", + }, + })); + assert_eq!( + json!(diagnostics.all()), + json!([ + { + "range": { + "start": { "line": 3, "character": 0 }, + "end": { "line": 3, "character": 1 }, + }, + "severity": 4, + "code": 6385, + "source": "deno-ts", + "message": "'a' is deprecated.", + "relatedInformation": [ + { + "location": { + "uri": "file:///a/%5Bfile%5D.ts", + "range": { + "start": { "line": 0, "character": 4 }, + "end": { "line": 0, "character": 16 }, + }, + }, + "message": "The declaration was marked as deprecated here.", + }, + ], + "tags": [2], + }, + ]), + ); + client.shutdown(); +} + #[test] fn lsp_diagnostics_deprecated() { let context = TestContextBuilder::new().use_temp_cwd().build();