From 07e1e85c5d09eafe464fc79812d9caea971c5194 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Mon, 20 Jan 2025 16:36:18 -0500 Subject: [PATCH] Avoid deserialization error for paths above the root (#10789) ## Summary Closes https://github.com/astral-sh/uv/issues/10777 ## Test Plan Copied over this lockfile: ```toml version = 1 requires-python = ">=3.12" resolution-markers = [ "sys_platform == 'win32'", "sys_platform != 'win32'", ] [[package]] name = "pyasn1" version = "0.6.1" source = { registry = "https://pypi.org/simple" } sdist = { url = "https://files.pythonhosted.org/packages/ba/e9/01f1a64245b89f039897cb0130016d79f77d52669aae6ee7b159a6c4c018/pyasn1-0.6.1.tar.gz", hash = "sha256:6f580d2bdd84365380830acf45550f2511469f673cb4a5ae3857a3170128b034", size = 145322 } wheels = [ { url = "https://files.pythonhosted.org/packages/c8/f1/d6a797abb14f6283c0ddff96bbdd46937f64122b8c925cab503dd37f8214/pyasn1-0.6.1-py3-none-any.whl", hash = "sha256:0d632f46f2ba09143da3a8afe9e33fb6f92fa2320ab7e886e2d0f7672af84629", size = 83135 }, ] [[package]] name = "pyasn1-modules" version = "0.4.1" source = { registry = "https://pypi.org/simple" } dependencies = [ { name = "pyasn1" }, ] sdist = { url = "https://files.pythonhosted.org/packages/1d/67/6afbf0d507f73c32d21084a79946bfcfca5fbc62a72057e9c23797a737c9/pyasn1_modules-0.4.1.tar.gz", hash = "sha256:c28e2dbf9c06ad61c71a075c7e0f9fd0f1b0bb2d2ad4377f240d33ac2ab60a7c", size = 310028 } wheels = [ { url = "https://files.pythonhosted.org/packages/77/89/bc88a6711935ba795a679ea6ebee07e128050d6382eaa35a0a47c8032bdc/pyasn1_modules-0.4.1-py3-none-any.whl", hash = "sha256:49bfa96b45a292b711e986f222502c1c9a5e1f4e568fc30e2574a6c7d07838fd", size = 181537 }, ] [[package]] name = "python-ldap" version = "3.4.4" source = { registry = "https://pypi.org/simple" } resolution-markers = [ "sys_platform != 'win32'", ] dependencies = [ { name = "pyasn1", marker = "sys_platform != 'win32'" }, { name = "pyasn1-modules", marker = "sys_platform != 'win32'" }, ] sdist = { url = "https://files.pythonhosted.org/packages/fd/8b/1eeb4025dc1d3ac2f16678f38dec9ebdde6271c74955b72db5ce7a4dbfbd/python-ldap-3.4.4.tar.gz", hash = "sha256:7edb0accec4e037797705f3a05cbf36a9fde50d08c8f67f2aef99a2628fab828", size = 377889 } [[package]] name = "python-ldap" version = "3.4.4" source = { path = "../../../uti/Python/python_ldap-3.4.4-cp312-cp312-win_amd64.whl" } resolution-markers = [ "sys_platform == 'win32'", ] dependencies = [ { name = "pyasn1", marker = "sys_platform == 'win32'" }, { name = "pyasn1-modules", marker = "sys_platform == 'win32'" }, ] wheels = [ { filename = "python_ldap-3.4.4-cp312-cp312-win_amd64.whl", hash = "sha256:94d2ca2b3ced81c9d89aa5c79d4965d03053e1ffdcfae73e9fac85d25b692e85" }, ] [package.metadata] requires-dist = [ { name = "pyasn1", specifier = ">=0.3.7" }, { name = "pyasn1-modules", specifier = ">=0.1.5" }, ] [[package]] name = "uv-test" version = "1.0" source = { virtual = "." } dependencies = [ { name = "python-ldap", version = "3.4.4", source = { registry = "https://pypi.org/simple" }, marker = "sys_platform != 'win32'" }, { name = "python-ldap", version = "3.4.4", source = { path = "../../../uti/Python/python_ldap-3.4.4-cp312-cp312-win_amd64.whl" }, marker = "sys_platform == 'win32'" }, ] [package.metadata] requires-dist = [ { name = "python-ldap", marker = "sys_platform != 'win32'" }, { name = "python-ldap", marker = "sys_platform == 'win32'", path = "../../../../../../../../../../../../uti/Python/python_ldap-3.4.4-cp312-cp312-win_amd64.whl" }, ] ``` Verified that `cargo run sync --frozen` installs `python-ldap` from PyPI, without erroring. --- crates/uv-pep508/src/verbatim_url.rs | 28 +++++++++++++++++++++++++ crates/uv-pypi-types/src/requirement.rs | 18 +++++++++++----- crates/uv-resolver/src/lock/mod.rs | 15 ++++++------- 3 files changed, 49 insertions(+), 12 deletions(-) diff --git a/crates/uv-pep508/src/verbatim_url.rs b/crates/uv-pep508/src/verbatim_url.rs index 817675dc8..31e099bca 100644 --- a/crates/uv-pep508/src/verbatim_url.rs +++ b/crates/uv-pep508/src/verbatim_url.rs @@ -112,6 +112,34 @@ impl VerbatimUrl { Ok(Self { url, given: None }) } + /// Parse a URL from a normalized path. + /// + /// Like [`VerbatimUrl::from_absolute_path`], but skips the normalization step. + pub fn from_normalized_path(path: impl AsRef) -> Result { + let path = path.as_ref(); + + // Error if the path is relative. + let path = if path.is_absolute() { + path + } else { + return Err(VerbatimUrlError::WorkingDirectory(path.to_path_buf())); + }; + + // Extract the fragment, if it exists. + let (path, fragment) = split_fragment(path); + + // Convert to a URL. + let mut url = Url::from_file_path(path.clone()) + .unwrap_or_else(|()| panic!("path is absolute: {}", path.display())); + + // Set the fragment, if it exists. + if let Some(fragment) = fragment { + url.set_fragment(Some(fragment)); + } + + Ok(Self { url, given: None }) + } + /// Set the verbatim representation of the URL. #[must_use] pub fn with_given(self, given: impl AsRef) -> Self { diff --git a/crates/uv-pypi-types/src/requirement.rs b/crates/uv-pypi-types/src/requirement.rs index ee8ef0e4a..44ad317b1 100644 --- a/crates/uv-pypi-types/src/requirement.rs +++ b/crates/uv-pypi-types/src/requirement.rs @@ -870,10 +870,12 @@ impl TryFrom for RequirementSource { } // TODO(charlie): The use of `CWD` here is incorrect. These should be resolved relative // to the workspace root, but we don't have access to it here. When comparing these - // sources in the lockfile, we replace the URL anyway. + // sources in the lockfile, we replace the URL anyway. Ideally, we'd either remove the + // URL field or make it optional. RequirementSourceWire::Path { path } => { let path = PathBuf::from(path); - let url = VerbatimUrl::from_path(&path, &*CWD)?; + let url = + VerbatimUrl::from_normalized_path(uv_fs::normalize_path_buf(CWD.join(&path)))?; Ok(Self::Path { ext: DistExtension::from_path(path.as_path()) .map_err(|err| ParsedUrlError::MissingExtensionPath(path.clone(), err))?, @@ -883,7 +885,9 @@ impl TryFrom for RequirementSource { } RequirementSourceWire::Directory { directory } => { let directory = PathBuf::from(directory); - let url = VerbatimUrl::from_path(&directory, &*CWD)?; + let url = VerbatimUrl::from_normalized_path(uv_fs::normalize_path_buf( + CWD.join(&directory), + ))?; Ok(Self::Directory { install_path: directory, editable: false, @@ -893,7 +897,9 @@ impl TryFrom for RequirementSource { } RequirementSourceWire::Editable { editable } => { let editable = PathBuf::from(editable); - let url = VerbatimUrl::from_path(&editable, &*CWD)?; + let url = VerbatimUrl::from_normalized_path(uv_fs::normalize_path_buf( + CWD.join(&editable), + ))?; Ok(Self::Directory { install_path: editable, editable: true, @@ -903,7 +909,9 @@ impl TryFrom for RequirementSource { } RequirementSourceWire::Virtual { r#virtual } => { let r#virtual = PathBuf::from(r#virtual); - let url = VerbatimUrl::from_path(&r#virtual, &*CWD)?; + let url = VerbatimUrl::from_normalized_path(uv_fs::normalize_path_buf( + CWD.join(&r#virtual), + ))?; Ok(Self::Directory { install_path: r#virtual, editable: false, diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs index dd38ec2c0..eab76925f 100644 --- a/crates/uv-resolver/src/lock/mod.rs +++ b/crates/uv-resolver/src/lock/mod.rs @@ -2489,12 +2489,13 @@ impl Package { } } -/// Attempts to construct a `VerbatimUrl` from the given `Path`. +/// Attempts to construct a `VerbatimUrl` from the given normalized `Path`. fn verbatim_url(path: &Path, id: &PackageId) -> Result { - let url = VerbatimUrl::from_absolute_path(path).map_err(|err| LockErrorKind::VerbatimUrl { - id: id.clone(), - err, - })?; + let url = + VerbatimUrl::from_normalized_path(path).map_err(|err| LockErrorKind::VerbatimUrl { + id: id.clone(), + err, + })?; Ok(url) } @@ -4146,7 +4147,7 @@ fn normalize_requirement(requirement: Requirement, root: &Path) -> Result { let install_path = uv_fs::normalize_path_buf(root.join(&install_path)); - let url = VerbatimUrl::from_absolute_path(&install_path) + let url = VerbatimUrl::from_normalized_path(&install_path) .map_err(LockErrorKind::RequirementVerbatimUrl)?; Ok(Requirement { @@ -4169,7 +4170,7 @@ fn normalize_requirement(requirement: Requirement, root: &Path) -> Result { let install_path = uv_fs::normalize_path_buf(root.join(&install_path)); - let url = VerbatimUrl::from_absolute_path(&install_path) + let url = VerbatimUrl::from_normalized_path(&install_path) .map_err(LockErrorKind::RequirementVerbatimUrl)?; Ok(Requirement {