From 9917a541873dc17deb2007224bdf9013ed5e0481 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 1 Oct 2025 01:17:32 -0400 Subject: [PATCH 1/7] fix-astral-sh/ty/issues/1290 --- crates/ruff_db/src/vendored.rs | 58 +++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/crates/ruff_db/src/vendored.rs b/crates/ruff_db/src/vendored.rs index 4c1cb951a328e3..fb6cfb811b21eb 100644 --- a/crates/ruff_db/src/vendored.rs +++ b/crates/ruff_db/src/vendored.rs @@ -401,7 +401,7 @@ impl<'a> From<&'a VendoredPath> for NormalizedVendoredPath<'a> { /// /// ## Panics: /// If a path with an unsupported component for vendored paths is passed. - /// Unsupported components are path prefixes and path root directories. + /// Unsupported components are path prefixes (but root directories are now supported). fn from(path: &'a VendoredPath) -> Self { /// Remove `.` and `..` components, and validate that unsupported components are not present. /// @@ -421,6 +421,12 @@ impl<'a> From<&'a VendoredPath> for NormalizedVendoredPath<'a> { // (see https://github.com/astral-sh/ruff/pull/11991#issuecomment-2185278014) normalized_parts.pop(); } + camino::Utf8Component::RootDir => { + // Ignore root directory components - vendored paths should be relative + // to the zip archive root anyway. This handles cases like "/module1.py" + // which can occur when paths are parsed from URIs. + continue; + } unsupported => { panic!("Unsupported component in a vendored path: {unsupported}") } @@ -734,4 +740,54 @@ pub(crate) mod tests { VendoredPath::new("./stdlib/asyncio/../asyncio/tasks.pyi"), ) } + + #[test] + fn functools_file_with_leading_slash() { + test_file(&mock_typeshed(), VendoredPath::new("/stdlib/functools.pyi")) + } + + #[test] + fn asyncio_file_with_leading_slash() { + test_file( + &mock_typeshed(), + VendoredPath::new("/stdlib/asyncio/tasks.pyi"), + ) + } + + #[test] + fn stdlib_dir_with_leading_slash() { + test_directory("/stdlib") + } + + #[test] + fn asyncio_dir_with_leading_slash() { + test_directory("/stdlib/asyncio") + } + + #[test] + fn issue_1290_reproduction() { + // Test case that reproduces the issue from https://github.com/astral-sh/ty/issues/1290 + // The playground was crashing when trying to access files like "vendored:/module1.py" + // which would be parsed as "/module1.py" and cause a panic due to unsupported RootDir component + let mock_typeshed = mock_typeshed(); + + // These paths should not panic anymore + let paths = [ + "/module1.py", + "/main.py", + "/stdlib/functools.pyi", + "/stdlib/asyncio/tasks.pyi", + ]; + + for path in paths { + let vendored_path = VendoredPath::new(path); + // This should not panic - it should either find the file or return an error gracefully + let result = mock_typeshed.read_to_string(vendored_path); + // We don't care if the file exists or not, just that it doesn't panic + match result { + Ok(_) => println!("Found file: {}", path), + Err(_) => println!("File not found (expected): {}", path), + } + } + } } From 1f1c59709da6af09bbd9c6b9030314945ca0603a Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 1 Oct 2025 01:22:13 -0400 Subject: [PATCH 2/7] cargo fmt --- crates/ruff_db/src/vendored.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/ruff_db/src/vendored.rs b/crates/ruff_db/src/vendored.rs index fb6cfb811b21eb..3bdc5a46e4eb7d 100644 --- a/crates/ruff_db/src/vendored.rs +++ b/crates/ruff_db/src/vendored.rs @@ -770,15 +770,15 @@ pub(crate) mod tests { // The playground was crashing when trying to access files like "vendored:/module1.py" // which would be parsed as "/module1.py" and cause a panic due to unsupported RootDir component let mock_typeshed = mock_typeshed(); - + // These paths should not panic anymore let paths = [ "/module1.py", - "/main.py", + "/main.py", "/stdlib/functools.pyi", "/stdlib/asyncio/tasks.pyi", ]; - + for path in paths { let vendored_path = VendoredPath::new(path); // This should not panic - it should either find the file or return an error gracefully From e3aa964fa3eae26030e41bbbf9658ab47332f5df Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 1 Oct 2025 15:02:19 -0400 Subject: [PATCH 3/7] Ignore Windows path prefixes in vendored path normalization --- crates/ruff_db/src/vendored.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/ruff_db/src/vendored.rs b/crates/ruff_db/src/vendored.rs index 3bdc5a46e4eb7d..c1292c29d719ca 100644 --- a/crates/ruff_db/src/vendored.rs +++ b/crates/ruff_db/src/vendored.rs @@ -399,9 +399,8 @@ impl<'a> From<&'a VendoredPath> for NormalizedVendoredPath<'a> { /// - Normalize `\\` separators to `/` /// - Validate that the path does not have any unsupported components /// - /// ## Panics: - /// If a path with an unsupported component for vendored paths is passed. - /// Unsupported components are path prefixes (but root directories are now supported). + /// Note: Windows path prefixes and root directories are ignored so that + /// vendored paths are always treated as relative to the archive root. fn from(path: &'a VendoredPath) -> Self { /// Remove `.` and `..` components, and validate that unsupported components are not present. /// @@ -427,9 +426,14 @@ impl<'a> From<&'a VendoredPath> for NormalizedVendoredPath<'a> { // which can occur when paths are parsed from URIs. continue; } - unsupported => { - panic!("Unsupported component in a vendored path: {unsupported}") - } + // On Windows, `Prefix` can represent drive letters or verbatim prefixes + // like `\\?\`. Treat these as ignored so that the remaining components + // are resolved relative to the archive root instead of panicking. + #[cfg(windows)] + camino::Utf8Component::Prefix(_) => continue, + // `Prefix` does not occur on Unix per camino docs; no action required. + #[cfg(unix)] + _ => unreachable!(), } } normalized_parts.join("/") From ad289750f619f7c2ab34efe86e58a3ad23924451 Mon Sep 17 00:00:00 2001 From: Dan Date: Wed, 1 Oct 2025 15:10:38 -0400 Subject: [PATCH 4/7] handle `Utf8Component::Prefix(_)` unconditionally --- crates/ruff_db/src/vendored.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/crates/ruff_db/src/vendored.rs b/crates/ruff_db/src/vendored.rs index c1292c29d719ca..904ed6f2f8bb51 100644 --- a/crates/ruff_db/src/vendored.rs +++ b/crates/ruff_db/src/vendored.rs @@ -429,11 +429,9 @@ impl<'a> From<&'a VendoredPath> for NormalizedVendoredPath<'a> { // On Windows, `Prefix` can represent drive letters or verbatim prefixes // like `\\?\`. Treat these as ignored so that the remaining components // are resolved relative to the archive root instead of panicking. - #[cfg(windows)] + // On Unix, `Prefix` does not occur per camino docs, but we still handle it + // to keep the match exhaustive across all targets. camino::Utf8Component::Prefix(_) => continue, - // `Prefix` does not occur on Unix per camino docs; no action required. - #[cfg(unix)] - _ => unreachable!(), } } normalized_parts.join("/") From 17e551a6b4a906975293ee5978078e9021326fdf Mon Sep 17 00:00:00 2001 From: Dan Date: Thu, 2 Oct 2025 15:48:47 -0400 Subject: [PATCH 5/7] Normalize vendored path to remove leading slash --- playground/ty/src/Editor/Editor.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/playground/ty/src/Editor/Editor.tsx b/playground/ty/src/Editor/Editor.tsx index c82250b9e1cde1..de9349c8a5a01f 100644 --- a/playground/ty/src/Editor/Editor.tsx +++ b/playground/ty/src/Editor/Editor.tsx @@ -206,7 +206,8 @@ class PlaygroundServer private getVendoredPath(uri: Uri): string { // Monaco parses "vendored://stdlib/typing.pyi" as authority="stdlib", path="/typing.pyi" // We need to reconstruct the full path - return uri.authority ? `${uri.authority}${uri.path}` : uri.path; + const fullPath = uri.authority ? `${uri.authority}${uri.path}` : uri.path; + return fullPath.startsWith("/") ? fullPath.slice(1) : fullPath; } constructor( From a8570331c6504448b257fe78bb6091d5fedf2dd7 Mon Sep 17 00:00:00 2001 From: Dan Date: Fri, 3 Oct 2025 16:36:24 -0400 Subject: [PATCH 6/7] revert changes, add vendored check alongside `/` case --- crates/ruff_db/src/vendored.rs | 68 +++-------------------------- playground/ty/src/Editor/Editor.tsx | 3 +- playground/ty/src/Playground.tsx | 18 ++++++++ 3 files changed, 24 insertions(+), 65 deletions(-) diff --git a/crates/ruff_db/src/vendored.rs b/crates/ruff_db/src/vendored.rs index 904ed6f2f8bb51..4c1cb951a328e3 100644 --- a/crates/ruff_db/src/vendored.rs +++ b/crates/ruff_db/src/vendored.rs @@ -399,8 +399,9 @@ impl<'a> From<&'a VendoredPath> for NormalizedVendoredPath<'a> { /// - Normalize `\\` separators to `/` /// - Validate that the path does not have any unsupported components /// - /// Note: Windows path prefixes and root directories are ignored so that - /// vendored paths are always treated as relative to the archive root. + /// ## Panics: + /// If a path with an unsupported component for vendored paths is passed. + /// Unsupported components are path prefixes and path root directories. fn from(path: &'a VendoredPath) -> Self { /// Remove `.` and `..` components, and validate that unsupported components are not present. /// @@ -420,18 +421,9 @@ impl<'a> From<&'a VendoredPath> for NormalizedVendoredPath<'a> { // (see https://github.com/astral-sh/ruff/pull/11991#issuecomment-2185278014) normalized_parts.pop(); } - camino::Utf8Component::RootDir => { - // Ignore root directory components - vendored paths should be relative - // to the zip archive root anyway. This handles cases like "/module1.py" - // which can occur when paths are parsed from URIs. - continue; + unsupported => { + panic!("Unsupported component in a vendored path: {unsupported}") } - // On Windows, `Prefix` can represent drive letters or verbatim prefixes - // like `\\?\`. Treat these as ignored so that the remaining components - // are resolved relative to the archive root instead of panicking. - // On Unix, `Prefix` does not occur per camino docs, but we still handle it - // to keep the match exhaustive across all targets. - camino::Utf8Component::Prefix(_) => continue, } } normalized_parts.join("/") @@ -742,54 +734,4 @@ pub(crate) mod tests { VendoredPath::new("./stdlib/asyncio/../asyncio/tasks.pyi"), ) } - - #[test] - fn functools_file_with_leading_slash() { - test_file(&mock_typeshed(), VendoredPath::new("/stdlib/functools.pyi")) - } - - #[test] - fn asyncio_file_with_leading_slash() { - test_file( - &mock_typeshed(), - VendoredPath::new("/stdlib/asyncio/tasks.pyi"), - ) - } - - #[test] - fn stdlib_dir_with_leading_slash() { - test_directory("/stdlib") - } - - #[test] - fn asyncio_dir_with_leading_slash() { - test_directory("/stdlib/asyncio") - } - - #[test] - fn issue_1290_reproduction() { - // Test case that reproduces the issue from https://github.com/astral-sh/ty/issues/1290 - // The playground was crashing when trying to access files like "vendored:/module1.py" - // which would be parsed as "/module1.py" and cause a panic due to unsupported RootDir component - let mock_typeshed = mock_typeshed(); - - // These paths should not panic anymore - let paths = [ - "/module1.py", - "/main.py", - "/stdlib/functools.pyi", - "/stdlib/asyncio/tasks.pyi", - ]; - - for path in paths { - let vendored_path = VendoredPath::new(path); - // This should not panic - it should either find the file or return an error gracefully - let result = mock_typeshed.read_to_string(vendored_path); - // We don't care if the file exists or not, just that it doesn't panic - match result { - Ok(_) => println!("Found file: {}", path), - Err(_) => println!("File not found (expected): {}", path), - } - } - } } diff --git a/playground/ty/src/Editor/Editor.tsx b/playground/ty/src/Editor/Editor.tsx index de9349c8a5a01f..c82250b9e1cde1 100644 --- a/playground/ty/src/Editor/Editor.tsx +++ b/playground/ty/src/Editor/Editor.tsx @@ -206,8 +206,7 @@ class PlaygroundServer private getVendoredPath(uri: Uri): string { // Monaco parses "vendored://stdlib/typing.pyi" as authority="stdlib", path="/typing.pyi" // We need to reconstruct the full path - const fullPath = uri.authority ? `${uri.authority}${uri.path}` : uri.path; - return fullPath.startsWith("/") ? fullPath.slice(1) : fullPath; + return uri.authority ? `${uri.authority}${uri.path}` : uri.path; } constructor( diff --git a/playground/ty/src/Playground.tsx b/playground/ty/src/Playground.tsx index e0e600c5ed9b6f..6b7134ddd8e693 100644 --- a/playground/ty/src/Playground.tsx +++ b/playground/ty/src/Playground.tsx @@ -60,6 +60,15 @@ export default function Playground() { }, [files]); const handleFileAdded = (workspace: Workspace, name: string) => { + if (name.startsWith("/")) { + setError("File names cannot start with '/'."); + return; + } + if (name.startsWith("vendored:")) { + setError("File names cannot start with 'vendored:'."); + return; + } + let handle = null; if (name === SETTINGS_FILE_NAME) { @@ -96,6 +105,15 @@ export default function Playground() { file: FileId, newName: string, ) => { + if (newName.startsWith("/")) { + setError("File names cannot start with '/'."); + return; + } + if (newName.startsWith("vendored:")) { + setError("File names cannot start with 'vendored:'."); + return; + } + const handle = files.handles[file]; let newHandle: FileHandle | null = null; if (handle == null) { From 4f987b6f63be11828e013e30a30f2ea4ed713924 Mon Sep 17 00:00:00 2001 From: Dan Date: Fri, 3 Oct 2025 16:40:14 -0400 Subject: [PATCH 7/7] remove erroneous logic from `handleFileAdded`. Only apply to `handFileRenamed` --- playground/ty/src/Playground.tsx | 9 --------- 1 file changed, 9 deletions(-) diff --git a/playground/ty/src/Playground.tsx b/playground/ty/src/Playground.tsx index 6b7134ddd8e693..1391ee548f9c98 100644 --- a/playground/ty/src/Playground.tsx +++ b/playground/ty/src/Playground.tsx @@ -60,15 +60,6 @@ export default function Playground() { }, [files]); const handleFileAdded = (workspace: Workspace, name: string) => { - if (name.startsWith("/")) { - setError("File names cannot start with '/'."); - return; - } - if (name.startsWith("vendored:")) { - setError("File names cannot start with 'vendored:'."); - return; - } - let handle = null; if (name === SETTINGS_FILE_NAME) {