Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use std::path::absolute for hyperlinks on Windows #2865

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ jobs:
include:
- build: pinned
os: ubuntu-latest
rust: 1.74.0
rust: 1.79.0
- build: stable
os: ubuntu-latest
rust: stable
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ exclude = [
build = "build.rs"
autotests = false
edition = "2021"
rust-version = "1.72"
rust-version = "1.79"

[[bin]]
bench = false
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -412,7 +412,7 @@ $ sudo xbps-install -Syv ripgrep

If you're a **Rust programmer**, ripgrep can be installed with `cargo`.

* Note that the minimum supported version of Rust for ripgrep is **1.72.0**,
* Note that the minimum supported version of Rust for ripgrep is **1.79.0**,
although ripgrep may work with older versions.
* Note that the binary may be bigger than expected because it contains debug
symbols. This is intentional. To remove debug symbols and therefore reduce
Expand All @@ -435,7 +435,7 @@ $ cargo binstall ripgrep

ripgrep is written in Rust, so you'll need to grab a
[Rust installation](https://www.rust-lang.org/) in order to compile it.
ripgrep compiles with Rust 1.72.0 (stable) or newer. In general, ripgrep tracks
ripgrep compiles with Rust 1.79.0 (stable) or newer. In general, ripgrep tracks
the latest stable release of the Rust compiler.

To build ripgrep:
Expand Down
82 changes: 56 additions & 26 deletions crates/printer/src/hyperlink.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,16 +702,20 @@ impl HyperlinkPath {
/// Returns a hyperlink path from an OS path.
#[cfg(windows)]
pub(crate) fn from_path(original_path: &Path) -> Option<HyperlinkPath> {
// On Windows, Path::canonicalize returns the result of
// GetFinalPathNameByHandleW with VOLUME_NAME_DOS,
// which produces paths such as the following:
// On Windows, we use `std::path::absolute` instead of `Path::canonicalize`
// as it can be much faster since it does not touch the file system.
// It wraps the [`GetFullPathNameW`][1] API, except for verbatim paths
// (those which start with `\\?\`, see [the documentation][2] for details).
//
// Here, we strip any verbatim path prefixes since we cannot use them
// in hyperlinks anyway. This can only happen if the user explicitly
// supplies a verbatim path as input, which already needs to be absolute:
//
// \\?\C:\dir\file.txt (local path)
// \\?\UNC\server\dir\file.txt (network share)
//
// The \\?\ prefix comes from VOLUME_NAME_DOS and is constant.
// It is followed either by the drive letter, or by UNC\
// (universal naming convention), which denotes a network share.
// The `\\?\` prefix is constant for verbatim paths, and can be followed
// by `UNC\` (universal naming convention), which denotes a network share.
//
// Given that the default URL format on Windows is file://{path}
// we need to return the following from this function:
Expand Down Expand Up @@ -750,18 +754,19 @@ impl HyperlinkPath {
//
// It doesn't parse any other number of slashes in "file//server" as a
// network path.
//
// [1]: https://learn.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-getfullpathnamew
// [2]: https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file

const WIN32_NAMESPACE_PREFIX: &str = r"\\?\";
const UNC_PREFIX: &str = r"UNC\";

// As for Unix, we canonicalize the path to make sure we have an
// absolute path.
let path = match original_path.canonicalize() {
let path = match std::path::absolute(original_path) {
Ok(path) => path,
Err(err) => {
log::debug!(
"hyperlink creation for {:?} failed, error occurred \
during path canonicalization: {}",
during conversion to absolute path: {}",
original_path,
err,
);
Expand All @@ -784,24 +789,20 @@ impl HyperlinkPath {
return None;
}
};
// As the comment above says, we expect all canonicalized paths to
// begin with a \\?\. If it doesn't, then something weird is happening
// and we should just give up.
if !string.starts_with(WIN32_NAMESPACE_PREFIX) {
log::debug!(
"hyperlink creation for {:?} failed, canonicalization \
returned {:?}, which does not start with \\\\?\\",
original_path,
path,
);
return None;
}
string = &string[WIN32_NAMESPACE_PREFIX.len()..];

// And as above, drop the UNC prefix too, but keep the leading slash.
if string.starts_with(UNC_PREFIX) {
string = &string[(UNC_PREFIX.len() - 1)..];
// Strip verbatim path prefixes (see the comment above for details).
if string.starts_with(WIN32_NAMESPACE_PREFIX) {
string = &string[WIN32_NAMESPACE_PREFIX.len()..];

// Drop the UNC prefix if there is one, but keep the leading slash.
if string.starts_with(UNC_PREFIX) {
string = &string[(UNC_PREFIX.len() - 1)..];
}
} else if string.starts_with(r"\\") || string.starts_with(r"//") {
// Drop one of the two leading slashes of network paths, it will be added back.
string = &string[1..];
}

// Finally, add a leading slash. In the local file case, this turns
// C:\foo\bar into /C:\foo\bar (and then percent encoding turns it into
// /C:/foo/bar). In the network share case, this turns \share\foo\bar
Expand Down Expand Up @@ -1006,4 +1007,33 @@ mod tests {
err(InvalidVariable("bar{{".to_string())),
);
}

#[test]
#[cfg(windows)]
fn convert_to_hyperlink_path() {
let convert = |path| {
String::from_utf8(
HyperlinkPath::from_path(Path::new(path)).unwrap().0,
)
.unwrap()
};

assert_eq!(convert(r"C:\dir\file.txt"), "/C:/dir/file.txt");
assert_eq!(
convert(r"C:\foo\bar\..\other\baz.txt"),
"/C:/foo/other/baz.txt"
);

assert_eq!(convert(r"\\server\dir\file.txt"), "//server/dir/file.txt");
assert_eq!(
convert(r"\\server\dir\foo\..\other\file.txt"),
"//server/dir/other/file.txt"
);

assert_eq!(convert(r"\\?\C:\dir\file.txt"), "/C:/dir/file.txt");
assert_eq!(
convert(r"\\?\UNC\server\dir\file.txt"),
"//server/dir/file.txt"
);
}
}