mirror of
https://github.com/rustfs/rustfs.git
synced 2026-10-27 06:18:43 +00:00
fix(ecstore): resolve Windows drive paths in URL endpoints (#8629)
On Windows, URL endpoints such as http://host:9000/C:/data{1...4} (the only form multi-node and pool-expansion layouts accept) produced the disk path `/C:/data1`: `try_from` stripped the synthetic slash, but `Url::set_path` put it back, and `get_file_path` only removed it for `file` endpoints. Windows resolves `/C:/data1` as an invalid `C:` component under the current drive, so disk validation and `resolve_local_disk_root` failed. - `get_file_path` now restores `/X:/...` to `X:/...` for every scheme on Windows; URL paths without a drive (`/export1`, `/C:foo`) are unchanged. - UNC shares (`\\nas\share\d1`, `//nas/share/d1`, `\\?\UNC\...`) are rejected with an explicit error instead of becoming a file URL whose host is the share server and whose path drops the share name. - A drive root (`http://host:9000/D:/`) is accepted in URL form, matching the path-style `D:\` endpoint that was already accepted. - The "distinct physical disks" error explains on Windows that directories on one volume share a device identity, alongside the existing RUSTFS_UNSAFE_BYPASS_DISK_CHECK hint. - The Windows filesystem lane now runs on layout changes and runs the layout::endpoint:: tests; new Windows tests carry the windows_ prefix. - Document Windows endpoint forms, the same-volume check, and the SNSD expansion limit in cluster-lifecycle-operations.md.
This commit is contained in:
唐小鸭
@@ -19,6 +19,7 @@ on:
|
||||
branches: [ main ]
|
||||
paths:
|
||||
- "crates/ecstore/src/disk/**"
|
||||
- "crates/ecstore/src/layout/**"
|
||||
- "crates/ecstore/src/store/init_format.rs"
|
||||
- "crates/ecstore/src/bucket/utils.rs"
|
||||
- "crates/ecstore/Cargo.toml"
|
||||
@@ -36,6 +37,7 @@ on:
|
||||
branches: [ main ]
|
||||
paths:
|
||||
- "crates/ecstore/src/disk/**"
|
||||
- "crates/ecstore/src/layout/**"
|
||||
- "crates/ecstore/src/store/init_format.rs"
|
||||
- "crates/ecstore/src/bucket/utils.rs"
|
||||
- "crates/ecstore/Cargo.toml"
|
||||
@@ -99,6 +101,10 @@ jobs:
|
||||
shell: pwsh
|
||||
run: cargo test -p rustfs-ecstore --lib windows_ -- --nocapture
|
||||
|
||||
- name: Test endpoint path parsing
|
||||
shell: pwsh
|
||||
run: "cargo test -p rustfs-ecstore --lib layout::endpoint:: -- --nocapture"
|
||||
|
||||
- name: Test startup temporary-directory cleanup
|
||||
shell: pwsh
|
||||
run: cargo test -p rustfs-ecstore --lib cleanup_tmp_on_startup_ -- --nocapture
|
||||
|
||||
@@ -25300,6 +25300,22 @@ mod test {
|
||||
assert!(debug.contains("endpoint"));
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[tokio::test]
|
||||
async fn windows_local_disk_opens_url_style_drive_endpoint() {
|
||||
use tempfile::tempdir;
|
||||
|
||||
let dir = tempdir().expect("temp dir should be created");
|
||||
let drive_path = dir.path().to_string_lossy().replace('\\', "/");
|
||||
let endpoint =
|
||||
Endpoint::try_from(format!("http://127.0.0.1:9000/{drive_path}").as_str()).expect("URL endpoint should parse");
|
||||
assert_eq!(Path::new(&endpoint.get_file_path()), dir.path());
|
||||
|
||||
LocalDisk::new(&endpoint, false)
|
||||
.await
|
||||
.expect("a URL-style drive endpoint must open as a local disk");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn std_backend_truncate_append_stream_and_full_read_restore_bytes() {
|
||||
use tempfile::tempdir;
|
||||
|
||||
@@ -101,23 +101,30 @@ impl TryFrom<&str> for Endpoint {
|
||||
let path = Path::new(&path).absolutize()?;
|
||||
|
||||
#[cfg(windows)]
|
||||
let path = if has_leading_slash_windows_drive(&path) {
|
||||
let path = match strip_windows_drive_url_path(&path) {
|
||||
// Url::path() exposes file-like Windows paths as `/C:/...`.
|
||||
// Strip only that synthetic leading slash; plain URL paths
|
||||
// such as `/export1` must stay URL paths, not become
|
||||
// relative paths under the current drive.
|
||||
Path::new(&path[1..]).absolutize()?.to_string_lossy().into_owned()
|
||||
} else {
|
||||
path
|
||||
Some(drive_path) => Path::new(drive_path).absolutize()?.to_string_lossy().into_owned(),
|
||||
None => path,
|
||||
};
|
||||
#[cfg(windows)]
|
||||
let path = Path::new(&path);
|
||||
|
||||
debug!("endpoint try_from: path={}", path.display());
|
||||
|
||||
if path.parent().is_none() || path.as_os_str().is_empty() {
|
||||
// A Windows drive root such as `C:\` has no parent, but it is
|
||||
// accepted for parity with the path-style `C:\` endpoint.
|
||||
#[cfg(windows)]
|
||||
let is_root = path.parent().is_none() && !is_windows_drive_root(path);
|
||||
#[cfg(not(windows))]
|
||||
let is_root = path.parent().is_none();
|
||||
if is_root || path.as_os_str().is_empty() {
|
||||
return Err(Error::other("empty or root path is not supported in URL endpoint"));
|
||||
}
|
||||
#[cfg(windows)]
|
||||
reject_windows_unc_path(path)?;
|
||||
|
||||
match path.to_str() {
|
||||
Some(v) => url.set_path(v),
|
||||
@@ -230,20 +237,63 @@ impl Endpoint {
|
||||
std::borrow::Cow::Borrowed(path)
|
||||
}
|
||||
};
|
||||
// Url::path() always starts with `/`, so every scheme stores a Windows
|
||||
// drive path as `/C:/...`. Restore the drive path for `file` and
|
||||
// `http(s)` endpoints alike; Windows would otherwise resolve `/C:/data`
|
||||
// as an invalid `C:` component under the current drive.
|
||||
#[cfg(windows)]
|
||||
if self.url.scheme() == "file" {
|
||||
let stripped: &str = decoded.strip_prefix('/').unwrap_or(&decoded);
|
||||
debug!("get_file_path windows: path={}", stripped);
|
||||
return stripped.to_string();
|
||||
if let Some(drive_path) = strip_windows_drive_url_path(&decoded) {
|
||||
debug!("get_file_path windows: path={}", drive_path);
|
||||
return drive_path.to_string();
|
||||
}
|
||||
decoded.into_owned()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn has_leading_slash_windows_drive(path: &str) -> bool {
|
||||
/// Returns `C:/...` for a URL path shaped like `/C:/...`, otherwise `None`.
|
||||
///
|
||||
/// The separator after the colon is required: `/C:foo` is a plain URL path,
|
||||
/// and `/export1` must never be reinterpreted as a drive path.
|
||||
#[cfg_attr(not(windows), allow(dead_code))]
|
||||
fn strip_windows_drive_url_path(path: &str) -> Option<&str> {
|
||||
let bytes = path.as_bytes();
|
||||
bytes.len() >= 4 && bytes[0] == b'/' && bytes[1].is_ascii_alphabetic() && bytes[2] == b':' && bytes[3] == b'/'
|
||||
if bytes.len() >= 4 && bytes[0] == b'/' && bytes[1].is_ascii_alphabetic() && bytes[2] == b':' && bytes[3] == b'/' {
|
||||
Some(&path[1..])
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
fn is_windows_drive_root(path: &Path) -> bool {
|
||||
use std::path::{Component, Prefix};
|
||||
|
||||
let mut components = path.components();
|
||||
matches!(
|
||||
components.next(),
|
||||
Some(Component::Prefix(prefix)) if matches!(prefix.kind(), Prefix::Disk(_) | Prefix::VerbatimDisk(_))
|
||||
) && matches!(components.next(), Some(Component::RootDir))
|
||||
&& components.next().is_none()
|
||||
}
|
||||
|
||||
/// UNC shares are rejected instead of supported: `Url::from_file_path` turns
|
||||
/// `\\nas\share\d1` into `file://nas/share/d1`, so the share server would be
|
||||
/// read as the endpoint host and the share name dropped from the drive path,
|
||||
/// and a network share does not provide the local-volume semantics (atomic
|
||||
/// rename, fsync, volume identity) that erasure-coded drives rely on.
|
||||
#[cfg(windows)]
|
||||
fn reject_windows_unc_path(path: &Path) -> Result<()> {
|
||||
use std::path::{Component, Prefix};
|
||||
|
||||
if let Some(Component::Prefix(prefix)) = path.components().next()
|
||||
&& matches!(prefix.kind(), Prefix::UNC(..) | Prefix::VerbatimUNC(..))
|
||||
{
|
||||
return Err(Error::other(
|
||||
"UNC network share paths are not supported as drive endpoints; use a local volume path such as \
|
||||
D:\\rustfs\\data1, or http://host:9000/D:/rustfs/data1 for URL endpoints",
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// parse a file path into a URL.
|
||||
@@ -261,6 +311,9 @@ fn url_parse_from_file_path(value: &str) -> Result<Url> {
|
||||
Err(err) => return Err(Error::other(format!("absolute path failed: {err}"))),
|
||||
};
|
||||
|
||||
#[cfg(windows)]
|
||||
reject_windows_unc_path(&file_path)?;
|
||||
|
||||
match Url::from_file_path(file_path) {
|
||||
Ok(url) => Ok(url),
|
||||
Err(_) => Err(Error::other("Convert a file path into an URL failed")),
|
||||
@@ -538,17 +591,110 @@ mod test {
|
||||
assert_eq!(url_endpoint.get_file_path(), "/path/to/data");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn strip_windows_drive_url_path_requires_drive_separator() {
|
||||
assert_eq!(strip_windows_drive_url_path("/C:/data1"), Some("C:/data1"));
|
||||
assert_eq!(strip_windows_drive_url_path("/d:/rustfs/data1"), Some("d:/rustfs/data1"));
|
||||
assert_eq!(strip_windows_drive_url_path("/C:/"), Some("C:/"));
|
||||
assert_eq!(strip_windows_drive_url_path("/C:foo"), None);
|
||||
assert_eq!(strip_windows_drive_url_path("/C:"), None);
|
||||
assert_eq!(strip_windows_drive_url_path("/export1"), None);
|
||||
assert_eq!(strip_windows_drive_url_path("/1:/data"), None);
|
||||
assert_eq!(strip_windows_drive_url_path("C:/data1"), None);
|
||||
}
|
||||
|
||||
#[cfg(not(windows))]
|
||||
#[test]
|
||||
fn url_endpoint_drive_shaped_path_stays_a_url_path_off_windows() {
|
||||
let endpoint = Endpoint::try_from("http://host:9000/C:/data1").unwrap();
|
||||
assert_eq!(endpoint.get_file_path(), "/C:/data1");
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn test_windows_url_drive_path_requires_separator_after_colon() {
|
||||
let drive_path_endpoint = Endpoint::try_from("http://host/C:/data").unwrap();
|
||||
assert_eq!(drive_path_endpoint.get_type(), EndpointType::Url);
|
||||
assert!(has_leading_slash_windows_drive(Url::parse("http://host/C:/data").unwrap().path()));
|
||||
fn windows_url_endpoint_file_path_restores_drive_path() {
|
||||
for (arg, expected) in [
|
||||
("http://host:9000/C:/data1", "C:/data1"),
|
||||
("https://host:9000/d:/rustfs/data4", "d:/rustfs/data4"),
|
||||
("http://host:9000/C:\\data1", "C:/data1"),
|
||||
("http://host:9000/C:/rustfs/../data1", "C:/data1"),
|
||||
("http://host:9000/C:/Program%20Files/rustfs", "C:/Program Files/rustfs"),
|
||||
] {
|
||||
let endpoint = Endpoint::try_from(arg).unwrap_or_else(|err| panic!("{arg}: {err}"));
|
||||
assert_eq!(endpoint.get_type(), EndpointType::Url, "{arg}");
|
||||
assert_eq!(endpoint.get_file_path(), expected, "{arg}");
|
||||
assert!(Path::new(&endpoint.get_file_path()).is_absolute(), "{arg}");
|
||||
}
|
||||
}
|
||||
|
||||
let url_path_endpoint = Endpoint::try_from("http://host/C:foo").unwrap();
|
||||
assert_eq!(url_path_endpoint.get_type(), EndpointType::Url);
|
||||
assert!(!has_leading_slash_windows_drive(Url::parse("http://host/C:foo").unwrap().path()));
|
||||
assert_eq!(url_path_endpoint.get_file_path(), "/C:foo");
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_url_endpoint_keeps_url_identity_after_drive_path_parse() {
|
||||
let endpoint = Endpoint::try_from("http://host:9000/C:/data1").unwrap();
|
||||
assert_eq!(endpoint.url.as_str(), "http://host:9000/C:/data1");
|
||||
assert_eq!(endpoint.to_string(), "http://host:9000/C:/data1");
|
||||
assert_eq!(endpoint.host_port(), "host:9000");
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_url_endpoint_keeps_plain_url_paths() {
|
||||
for (arg, expected) in [("http://host:9000/export1", "/export1"), ("http://host:9000/C:foo", "/C:foo")] {
|
||||
let endpoint = Endpoint::try_from(arg).unwrap();
|
||||
assert_eq!(endpoint.get_type(), EndpointType::Url, "{arg}");
|
||||
assert_eq!(endpoint.get_file_path(), expected, "{arg}");
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_url_and_path_endpoints_share_drive_file_path() {
|
||||
let path_endpoint = Endpoint::try_from("C:\\rustfs\\data1").unwrap();
|
||||
let url_endpoint = Endpoint::try_from("http://host:9000/C:/rustfs/data1").unwrap();
|
||||
let verbatim_endpoint = Endpoint::try_from("\\\\?\\C:\\rustfs\\data1").unwrap();
|
||||
|
||||
assert_eq!(path_endpoint.get_type(), EndpointType::Path);
|
||||
assert_eq!(path_endpoint.get_file_path(), "C:/rustfs/data1");
|
||||
assert_eq!(url_endpoint.get_file_path(), path_endpoint.get_file_path());
|
||||
assert_eq!(verbatim_endpoint.get_file_path(), path_endpoint.get_file_path());
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_url_endpoint_resolves_existing_drive_directory() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let drive_path = dir.path().to_string_lossy().replace('\\', "/");
|
||||
let endpoint = Endpoint::try_from(format!("http://127.0.0.1:9000/{drive_path}").as_str()).unwrap();
|
||||
|
||||
let file_path = endpoint.get_file_path();
|
||||
assert_eq!(Path::new(&file_path), dir.path());
|
||||
rustfs_utils::canonicalize(&file_path)
|
||||
.unwrap_or_else(|err| panic!("URL endpoint path '{file_path}' must resolve to the temp directory: {err}"));
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_drive_root_endpoints_are_accepted_in_both_forms() {
|
||||
let path_endpoint = Endpoint::try_from("D:\\").unwrap();
|
||||
assert_eq!(path_endpoint.get_file_path(), "D:/");
|
||||
|
||||
let url_endpoint = Endpoint::try_from("http://host:9000/D:/").unwrap();
|
||||
assert_eq!(url_endpoint.get_type(), EndpointType::Url);
|
||||
assert_eq!(url_endpoint.get_file_path(), "D:/");
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[test]
|
||||
fn windows_unc_endpoints_are_rejected() {
|
||||
for arg in [
|
||||
"\\\\nas\\share\\d1",
|
||||
"//nas/share/d1",
|
||||
"\\\\?\\UNC\\nas\\share\\d1",
|
||||
"http://host:9000//nas/share/d1",
|
||||
] {
|
||||
let err = Endpoint::try_from(arg).expect_err(arg);
|
||||
assert!(err.to_string().contains("UNC network share paths are not supported"), "{arg}: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -1729,11 +1729,21 @@ fn validate_local_physical_disk_independence(pools: &[Endpoints]) -> Result<()>
|
||||
|
||||
Err(Error::other(format!(
|
||||
"local erasure endpoints must use distinct physical disks; detected shared devices [{details}]. \
|
||||
validation diagnostics: [{diagnostics_summary}]. \
|
||||
validation diagnostics: [{diagnostics_summary}]. {SHARED_DEVICE_PLATFORM_HINT}\
|
||||
Set {ENV_UNSAFE_BYPASS_DISK_CHECK}=true only for local testing or CI to bypass this safety check"
|
||||
)))
|
||||
}
|
||||
|
||||
/// Windows reports the volume mount point (for example `C:\`) as the device
|
||||
/// identity, so every directory on one drive letter or mounted volume counts
|
||||
/// as the same physical disk.
|
||||
#[cfg(windows)]
|
||||
const SHARED_DEVICE_PLATFORM_HINT: &str = "On Windows the device identity is the volume mount point, so directories on the same \
|
||||
drive letter or mounted volume (for example C:\\data1 and C:\\data2) count as one disk; place each endpoint on its own \
|
||||
volume. ";
|
||||
#[cfg(not(windows))]
|
||||
const SHARED_DEVICE_PLATFORM_HINT: &str = "";
|
||||
|
||||
fn validate_local_cross_device_mounts(local_paths: &[String]) -> Result<()> {
|
||||
rustfs_utils::os::check_cross_device_mounts(local_paths)
|
||||
.map_err(|err| Error::other(format!("local endpoint cross-device mount validation failed: {err}")))
|
||||
@@ -3581,6 +3591,38 @@ mod test {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(windows)]
|
||||
#[serial]
|
||||
#[tokio::test]
|
||||
async fn windows_url_endpoints_on_one_volume_resolve_and_report_shared_volume() {
|
||||
async_with_vars([(ENV_UNSAFE_BYPASS_DISK_CHECK, None::<&str>), (ENV_MINIO_CI, None::<&str>)], async {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let mut endpoints = Vec::new();
|
||||
for name in ["disk1", "disk2"] {
|
||||
let disk = dir.path().join(name);
|
||||
std::fs::create_dir_all(&disk).unwrap();
|
||||
let drive_path = disk.to_string_lossy().replace('\\', "/");
|
||||
let mut endpoint = Endpoint::try_from(format!("http://127.0.0.1:9000/{drive_path}").as_str()).unwrap();
|
||||
endpoint.is_local = true;
|
||||
endpoints.push(endpoint);
|
||||
}
|
||||
|
||||
let err = validate_local_physical_disk_independence(&[Endpoints::from(endpoints)]).unwrap_err();
|
||||
|
||||
// Before the drive-path fix, `/C:/...` failed canonicalization here
|
||||
// with "failed to resolve local endpoint path".
|
||||
let err_text = err.to_string();
|
||||
assert!(
|
||||
!err_text.contains("failed to resolve local endpoint path"),
|
||||
"unexpected error: {err_text}"
|
||||
);
|
||||
assert!(err_text.contains("distinct physical disks"), "unexpected error: {err_text}");
|
||||
assert!(err_text.contains("same drive letter or mounted volume"), "unexpected error: {err_text}");
|
||||
assert!(err_text.contains(ENV_UNSAFE_BYPASS_DISK_CHECK), "unexpected error: {err_text}");
|
||||
})
|
||||
.await;
|
||||
}
|
||||
|
||||
#[cfg(target_os = "linux")]
|
||||
#[serial]
|
||||
#[tokio::test]
|
||||
|
||||
@@ -44,6 +44,14 @@ Rules enforced by `DisksLayout::from_volumes`: with more than one argument every
|
||||
|
||||
Startup order (`init_startup_storage_foundation` in `rustfs/src/startup_storage.rs`): parse endpoints, validate storage classes against every pool (`ECStore::validate_startup_storage_class`), enforce the unsupported-filesystem policy, initialize local disks, initialize lock clients, then load or create the format quorum and build the store. A storage-class error stops startup before any disk is touched.
|
||||
|
||||
### 2.1 Windows endpoints
|
||||
|
||||
Multi-node and multi-pool layouts need URL endpoints, so on Windows the drive path goes after the port with forward slashes: `http://node{1...4}:9000/D:/rustfs/data{1...4}`. `Endpoint::get_file_path` (`crates/ecstore/src/layout/endpoint.rs`) turns the URL path `/D:/rustfs/data1` back into `D:/rustfs/data1` for every scheme; a URL path without a drive, such as `/export1`, stays a URL path. A drive root (`D:\` or `http://node1:9000/D:/`) is accepted in both forms. UNC shares (`\\nas\share\d1`, `//nas/share/d1`, `\\?\UNC\...`) are rejected at parse time: they are not local volumes, and parsing would read the share server as the endpoint host.
|
||||
|
||||
Endpoint parsing then requires distinct physical disks (`validate_local_physical_disk_independence` in `crates/ecstore/src/layout/endpoints.rs`). On Windows the device identity is the volume mount point from `GetVolumePathNameW` (`crates/utils/src/os/windows.rs`), so `D:/rustfs/data1` through `D:/rustfs/data4` are one disk and startup fails with `must use distinct physical disks`. Give each endpoint its own volume (a drive letter or a volume mounted into an NTFS folder). For a single-machine test only, `RUSTFS_UNSAFE_BYPASS_DISK_CHECK=true` skips the check; it gives no protection against losing that volume.
|
||||
|
||||
A single local path (`rustfs D:\rustfs\data`) is a single-node single-drive deployment and can never gain a pool (§7). If the machine may expand later, start with URL endpoints and at least two drives per pool.
|
||||
|
||||
## 3. Startup configuration
|
||||
|
||||
| Variable | Effect | Boundary |
|
||||
|
||||
Reference in New Issue
Block a user