diff --git a/README.md b/README.md index d9580120..c67acec9 100644 --- a/README.md +++ b/README.md @@ -27,6 +27,32 @@ code lives behind `cfg(windows)`. The workspace's `wtf-string` crate is the exception -- its portable core has no `cfg(windows)` gating, so CI additionally builds, tests, and lints it on Linux and macOS. +## Crate naming + +The `-sys` suffix here does **not** carry its usual ecosystem meaning. A `-sys` +crate is normally raw FFI declarations with no abstraction over them; every +`windows-*-sys` crate in this workspace instead wraps a great deal of `unsafe`, +and takes its declarations from +[`windows-sys`](https://crates.io/crates/windows-sys) rather than making its own. + +What the suffix marks is a **layer**: + +- **`windows-*-sys`** -- makes an existing Win32 API memory-safe *without adding + policy*. It schedules nothing, picks no delivery model, and reports the + platform's outcomes unnormalised. +- **no suffix** -- decides something Win32 has no equivalent of. + `windows-waitable-queues` is the worked example: it chooses a slot protocol, an + overflow policy, and a signalling discipline, so calling it `-sys` would + misdescribe how much it decides on the caller's behalf. + +So the presence of `unsafe` wrappers is not evidence either way -- it is the job +description of the first kind. The distinction is *policy*, and the suffix is the +only signal a reader has for how much a crate decides for them. + +This section is the statement of record. The convention governs published crate +names, so it belongs where a reader meets the crates rather than only in the +design notes. + ## Build Requires Rust `1.98` or newer. diff --git a/crates/windows-file-enumeration-sys/src/path/tests.rs b/crates/windows-file-enumeration-sys/src/path/tests.rs index 0e5013da..fa68f16c 100644 --- a/crates/windows-file-enumeration-sys/src/path/tests.rs +++ b/crates/windows-file-enumeration-sys/src/path/tests.rs @@ -181,9 +181,22 @@ fn a_device_namespace_path_is_resolved_rather_than_kept_verbatim() { /// An absolute path of exactly `units` UTF-16 units, already in normal form so /// `GetFullPathNameW` returns it unchanged and the resolved length is the input /// length. +/// +/// UTF-16 units and not bytes or `char`s, because that is the unit Win32 +/// measures a path in: `MAX_PATH` is a count of `WCHAR`. The three coincide for +/// the ASCII this builds, so counting the prefix any other way would pass today +/// and quietly measure the wrong thing the moment a case uses a character that +/// is not one byte, one scalar, and one unit at once. fn absolute_path_of_length(units: usize) -> String { let prefix = r"C:\"; - format!("{prefix}{}", "a".repeat(units - prefix.len())) + let prefix_units = prefix.encode_utf16().count(); + assert!( + units >= prefix_units, + "asked for a {units}-unit path, but the {prefix} prefix is already \ + {prefix_units} units; the subtraction below would underflow and panic \ + without saying why" + ); + format!("{prefix}{}", "a".repeat(units - prefix_units)) } #[test] @@ -193,7 +206,7 @@ fn an_ordinary_path_of_exactly_max_path_content_is_accepted() { // cannot see, and it is the expensive direction: it refuses a path Windows // would have opened. let path = absolute_path_of_length(259); - assert_eq!(path.chars().count(), 259); + assert_eq!(path.encode_utf16().count(), 259); let prepared = prepare_str(&path).expect("259 units is within the ordinary limit"); assert_eq!(text(&prepared), path); @@ -203,7 +216,7 @@ fn an_ordinary_path_of_exactly_max_path_content_is_accepted() { fn an_ordinary_path_one_unit_past_max_path_content_is_rejected() { // 260 counts the terminator, so 260 content units do not fit. let path = absolute_path_of_length(260); - assert_eq!(path.chars().count(), 260); + assert_eq!(path.encode_utf16().count(), 260); let error = prepare_str(&path).expect_err("260 units leaves no room for the terminator"); assert_eq!(error.failure(), RequestFailure::PathTooLong); diff --git a/crates/windows-namespace-request-sys/README.md b/crates/windows-namespace-request-sys/README.md index 6851d262..a1a5e075 100644 --- a/crates/windows-namespace-request-sys/README.md +++ b/crates/windows-namespace-request-sys/README.md @@ -32,6 +32,25 @@ and unassociated, because associating it with a completion port irreversibly forecloses `IoRing` use of it, and that choice belongs to a layer that knows the handle's destination. +## Why the `-sys` suffix + +Not in the usual sense. A `-sys` crate elsewhere in the ecosystem is normally +raw FFI declarations with no abstraction over them, and by that reading this +crate is misnamed: it declares almost no FFI of its own and takes its +declarations from [`windows-sys`](https://crates.io/crates/windows-sys). + +In this workspace the suffix marks a **layer**, not a linking strategy: a +`windows-*-sys` crate makes an existing Win32 API memory-safe **without adding +policy**, and a crate that decides something Win32 has no equivalent of drops +the suffix. That is why `windows-waitable-queues` carries no `-sys` -- it picks +a slot protocol and an overflow policy -- while this crate does, despite +wrapping a great deal of `unsafe`. Everything above is the suffix being earned: +it schedules nothing, chooses no delivery model, and reports raw Win32 outcomes +without normalising them. + +The convention is stated for the whole workspace in the repository's +[README](../../README.md#crate-naming). + ## A path is copied; a handle is duplicated Several entries take a handle rather than a path, and a request owns a @@ -127,5 +146,6 @@ operation coverage (every audited call site re-expressed) and scenario coverage context, many requests across concurrent workers from one shared capture, and a handle opened by one request carried into a later one). -Design decisions are recorded in [DESIGN-NOTES.md](DESIGN-NOTES.md). Not yet -released to crates.io. +Design decisions are recorded in [DESIGN-NOTES.md](DESIGN-NOTES.md). Published +on crates.io as +[`windows-namespace-request-sys`](https://crates.io/crates/windows-namespace-request-sys). diff --git a/crates/windows-namespace-request-sys/src/final_path/tests.rs b/crates/windows-namespace-request-sys/src/final_path/tests.rs index 4b700004..6a1d1636 100644 --- a/crates/windows-namespace-request-sys/src/final_path/tests.rs +++ b/crates/windows-namespace-request-sys/src/final_path/tests.rs @@ -117,9 +117,9 @@ fn the_buffer_grows_for_a_path_longer_than_the_first_attempt() { .to_string_lossy(); assert!( - resolved.chars().count() > 260, - "the fixture must actually exceed the first attempt: {} chars", - resolved.chars().count() + resolved.encode_utf16().count() > 260, + "the fixture must actually exceed the first attempt: {} UTF-16 units", + resolved.encode_utf16().count() ); assert!(resolved.ends_with("f.t"), "unexpected: {resolved}"); } diff --git a/crates/windows-namespace-request-sys/src/full_path/tests.rs b/crates/windows-namespace-request-sys/src/full_path/tests.rs index 7997dd5f..a8d734ea 100644 --- a/crates/windows-namespace-request-sys/src/full_path/tests.rs +++ b/crates/windows-namespace-request-sys/src/full_path/tests.rs @@ -102,9 +102,9 @@ fn a_deeply_nested_path_beyond_the_first_attempt_still_resolves() { let resolved = resolve(&path); assert!( - resolved.chars().count() > 260, - "the fixture must exceed the first attempt: {} chars", - resolved.chars().count() + resolved.encode_utf16().count() > 260, + "the fixture must exceed the first attempt: {} UTF-16 units", + resolved.encode_utf16().count() ); assert!(resolved.ends_with("file.txt"), "unexpected: {resolved}"); } diff --git a/crates/windows-namespace-request-sys/src/open/tests.rs b/crates/windows-namespace-request-sys/src/open/tests.rs index 252a5a3d..5d49d3bb 100644 --- a/crates/windows-namespace-request-sys/src/open/tests.rs +++ b/crates/windows-namespace-request-sys/src/open/tests.rs @@ -385,3 +385,69 @@ fn a_copy_duplicates_the_template_rather_than_sharing_the_owner() { copy.perform() .expect("the copy's template outlived the original"); } + +#[test] +fn every_configured_parameter_reads_back_through_its_own_accessor() { + // A mutation run replaced `desired_access`, `share_mode`, and + // `creation_disposition` with constants and nothing failed. The tests above + // build requests with `with_*` and then *open* them, so they exercise the + // fields through Win32 -- which is exactly what cannot distinguish an + // accessor reporting the truth from one reporting a constant, because the + // open path reads the struct's fields directly rather than through them. + // + // Every value here is deliberately non-zero and pairwise distinct. + // `OpenFile::new` starts every parameter at zero, so a test that configured + // a zero -- or reused one value twice -- would be satisfied by + // `-> Default::default()` and by an accessor reading a neighbour's field. + let fixture = Fixture::new("open-accessors"); + let request = request_for(fixture.directory()) + .with_desired_access(FILE_GENERIC_READ) + .with_share_mode(FILE_SHARE_READ) + .with_creation_disposition(OPEN_EXISTING) + .with_flags_and_attributes(FILE_FLAG_BACKUP_SEMANTICS); + + assert_eq!(request.desired_access(), FILE_GENERIC_READ); + assert_eq!(request.share_mode(), FILE_SHARE_READ); + assert_eq!(request.creation_disposition(), OPEN_EXISTING); + assert_eq!(request.flags_and_attributes(), FILE_FLAG_BACKUP_SEMANTICS); + + // The property the four assertions above rest on, stated rather than left + // to the reader's eye: these are Win32 constants and their values are not + // obvious, so a collision between two of them would silently weaken the + // test into one that cannot tell those accessors apart. + let configured = [ + FILE_GENERIC_READ, + FILE_SHARE_READ, + OPEN_EXISTING, + FILE_FLAG_BACKUP_SEMANTICS, + ]; + for (index, value) in configured.iter().enumerate() { + assert_ne!( + *value, 0, + "a zero is indistinguishable from the unset default" + ); + for other in &configured[index + 1..] { + assert_ne!( + value, other, + "two parameters share a value, so this test cannot tell their \ + accessors apart" + ); + } + } +} + +#[test] +fn an_unset_parameter_reads_back_as_nothing_rather_than_as_a_plausible_open() { + // The contract `OpenFile::new` states: every parameter starts at "the + // caller said nothing" rather than at a plausible-looking open, because a + // plausible default is exactly what a caller cannot see they got. An + // accessor that invented one would hide that from them. + let fixture = Fixture::new("open-unset"); + let request = request_for(fixture.directory()); + + assert_eq!(request.desired_access(), 0); + assert_eq!(request.share_mode(), 0); + assert_eq!(request.creation_disposition(), 0); + assert_eq!(request.flags_and_attributes(), 0); + assert!(request.security().is_none()); +} diff --git a/crates/windows-namespace-request-sys/src/open_by_id/tests.rs b/crates/windows-namespace-request-sys/src/open_by_id/tests.rs index b0ddb68a..7e81a81f 100644 --- a/crates/windows-namespace-request-sys/src/open_by_id/tests.rs +++ b/crates/windows-namespace-request-sys/src/open_by_id/tests.rs @@ -18,8 +18,8 @@ use windows_sys::Win32::Storage::FileSystem::{ }; use super::{FileIdentifier, OpenFileByIdentifier}; -use crate::CapturedHandle; use crate::handle::tests::{FILE_CONTENTS, Fixture, handle_allocation}; +use crate::{CapturedHandle, SecurityAttributes}; const AUDITED_SHARE: u32 = FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE; @@ -275,3 +275,79 @@ fn a_request_performs_the_same_way_on_another_thread() { assert_eq!(length, FILE_CONTENTS.len() as u64); } + +#[test] +fn every_configured_parameter_reads_back_through_its_own_accessor() { + // Four accessors -- `desired_access`, `share_mode`, `security`, and + // `flags_and_attributes` -- all survived replacement by constants in a + // mutation run. Every test above builds a request and then *performs* it, + // and the perform path reads the struct's fields directly, so nothing + // distinguished an accessor that reports the truth from one that does not. + // + // `OpenFileByIdentifier::new` starts every parameter at zero or `None`, so + // the values here are deliberately non-zero and pairwise distinct: a zero + // would be indistinguishable from the default, and a repeated value would + // let one accessor read a neighbour's field undetected. + let _allocating = handle_allocation() + .read() + .expect("the lock is not poisoned"); + let fixture = Fixture::new("byid-accessors"); + let file = fixture.open_file(); + let id = file_id_of(&file); + let hint = open_directory_for_hint(&fixture); + + let request = OpenFileByIdentifier::new( + CapturedHandle::capture(hint.as_handle()).expect("capture the volume hint"), + FileIdentifier::FileId(id), + ) + .with_desired_access(FILE_GENERIC_READ) + .with_share_mode(FILE_SHARE_READ) + .with_flags_and_attributes(FILE_FLAG_BACKUP_SEMANTICS); + + assert_eq!(request.desired_access(), FILE_GENERIC_READ); + assert_eq!(request.share_mode(), FILE_SHARE_READ); + assert_eq!(request.flags_and_attributes(), FILE_FLAG_BACKUP_SEMANTICS); + assert!( + request.security().is_none(), + "nothing was supplied, so nothing must be reported" + ); + + let configured = [ + FILE_GENERIC_READ, + FILE_SHARE_READ, + FILE_FLAG_BACKUP_SEMANTICS, + ]; + for (index, value) in configured.iter().enumerate() { + assert_ne!(*value, 0, "a zero is indistinguishable from the default"); + for other in &configured[index + 1..] { + assert_ne!( + value, other, + "two parameters share a value, so this test cannot tell their \ + accessors apart" + ); + } + } +} + +#[test] +fn supplied_security_attributes_read_back_rather_than_reporting_none() { + // `security -> None` is the one accessor whose default is already `None`, + // so it needs the opposite case: attributes that were supplied must be + // visible to a caller inspecting the request, not only to the open that + // consumes it. + let _allocating = handle_allocation() + .read() + .expect("the lock is not poisoned"); + let fixture = Fixture::new("byid-security"); + let file = fixture.open_file(); + let id = file_id_of(&file); + let hint = open_directory_for_hint(&fixture); + + let request = OpenFileByIdentifier::new( + CapturedHandle::capture(hint.as_handle()).expect("capture the volume hint"), + FileIdentifier::FileId(id), + ) + .with_security(Some(SecurityAttributes::new(None, false))); + + assert!(request.security().is_some()); +} diff --git a/crates/windows-namespace-request-sys/src/path/tests.rs b/crates/windows-namespace-request-sys/src/path/tests.rs index eec60ded..dbb4aac2 100644 --- a/crates/windows-namespace-request-sys/src/path/tests.rs +++ b/crates/windows-namespace-request-sys/src/path/tests.rs @@ -236,3 +236,190 @@ fn an_error_without_an_os_code_renders_only_its_description() { assert_eq!(error.to_string(), PathFailure::EmptyPath.description()); assert!(std::error::Error::source(&error).is_none()); } + +// --------------------------------------------------------------------------- +// Boundaries. +// +// A mutation sweep moved the ordinary path limit by one in both directions and +// changed `>` to `>=` and `==`, and every one of those survived: the tests +// above use comfortably-wrong lengths, which prove a check exists but not that +// it sits at the right unit. +// +// This is the same block, and for the same reason, as the one in +// `windows-file-enumeration-sys`'s path module -- the two crates carry +// near-identical path contracts, and the sweep found the same gap in both. +// --------------------------------------------------------------------------- + +/// An absolute path of exactly `units` UTF-16 units, already in normal form so +/// `GetFullPathNameW` returns it unchanged and the resolved length equals the +/// input length. +/// +/// UTF-16 units and not bytes or `char`s, because that is the unit Win32 +/// measures a path in: `MAX_PATH` is a count of `WCHAR`. The three coincide for +/// the ASCII this builds, so counting the prefix any other way would pass today +/// and quietly measure the wrong thing the moment a case uses a character that +/// is not one byte, one scalar, and one unit at once. +fn absolute_path_of_length(units: usize) -> String { + let prefix = r"C:\"; + let prefix_units = prefix.encode_utf16().count(); + assert!( + units >= prefix_units, + "asked for a {units}-unit path, but the {prefix} prefix is already \ + {prefix_units} units; the subtraction below would underflow and panic \ + without saying why" + ); + format!("{prefix}{}", "a".repeat(units - prefix_units)) +} + +#[test] +fn an_ordinary_path_of_exactly_max_path_content_is_accepted() { + // 259 = MAX_PATH - 1, the longest path that leaves room for the terminator. + // Rejecting it is the off-by-one a "much too long" test cannot see, and it + // is the expensive direction: it refuses a path Windows would have opened. + let path = absolute_path_of_length(259); + assert_eq!(path.encode_utf16().count(), 259); + + let prepared = prepare_str(&path).expect("259 units is within the ordinary limit"); + assert_eq!(text(&prepared), path); +} + +#[test] +fn an_ordinary_path_one_unit_past_max_path_content_is_rejected() { + let path = absolute_path_of_length(260); + assert_eq!(path.encode_utf16().count(), 260); + + let error = prepare_str(&path).expect_err("260 units leaves no room for the terminator"); + assert_eq!(error.failure(), PathFailure::PathTooLong); +} + +#[test] +fn the_ordinary_limit_is_one_less_than_max_path() { + // The relationship the two tests above rest on, stated directly so a change + // to the constant fails here with its reason rather than only as a puzzling + // length assertion elsewhere. + assert_eq!(MAX_PATH_CONTENT, MAX_PATH - 1); + assert_eq!(MAX_PATH_CONTENT, 259); +} + +#[test] +fn a_path_whose_character_count_hides_its_utf16_length_is_still_refused() { + // The two tests above build ASCII, where bytes, `char`s and UTF-16 units are + // the same number, so a path they accept or refuse says nothing about which + // unit was counted. This one separates them: a character above `U+FFFF` (a + // supplementary character) is one `char` but *two* UTF-16 units, because + // UTF-16 encodes it as a surrogate pair. A path built from those measured in + // scalars looks about half as long as Windows considers it. + // + // What this pins is the **contract** -- such a path is still refused -- and + // not any single site's arithmetic. Measured, because the distinction is not + // obvious: `prepare` checks the length twice, and sabotaging only the + // pre-check leaves this test passing, because `GetFullPathNameW` reports the + // resolved length in UTF-16 units and the post-check refuses on that. It + // takes disabling *both* to make this test fail. That second guard is the + // stronger one -- its count comes from Windows and so cannot be in the wrong + // unit -- which is worth knowing before anyone "simplifies" the pre-check + // away as redundant. + let supplementary = '\u{1F600}'; + assert_eq!(supplementary.len_utf16(), 2, "the premise of this test"); + + // 3 units of `C:\` plus 128 two-unit characters is exactly the limit. + let accepted = format!(r"C:\{}", supplementary.to_string().repeat(128)); + assert_eq!(accepted.encode_utf16().count(), 259); + assert_eq!(accepted.chars().count(), 131); + + let prepared = prepare_str(&accepted).expect("259 UTF-16 units is within the limit"); + assert_eq!(text(&prepared), accepted); + + // One more unit is over it. A limit enforced on `chars().count()` would see + // 132 against a bound of 259 and admit a path Windows refuses -- which is + // the expensive direction, since the caller is told the open may proceed. + let rejected = format!("{accepted}a"); + assert_eq!(rejected.encode_utf16().count(), 260); + assert_eq!(rejected.chars().count(), 132); + + let error = prepare_str(&rejected).expect_err("260 UTF-16 units is past the limit"); + assert_eq!(error.failure(), PathFailure::PathTooLong); +} + +#[test] +fn a_verbatim_drive_relative_path_with_a_separator_is_rejected() { + // `\\?\C:foo` has no separator at all, so it is refused before the root is + // ever inspected and never reaches the drive-designator check. This form + // does reach it: the root is `C:foo`, which contains a colon but is not a + // drive. Without it, the check could report every root as a drive and + // nothing would notice. + let error = prepare_str(r"\\?\C:foo\bar").expect_err("drive-relative, not fully qualified"); + assert_eq!(error.failure(), PathFailure::NotFullyQualified); +} + +#[test] +fn a_verbatim_root_needs_a_letter_before_its_colon_not_merely_a_colon() { + // Both halves of the drive-designator rule are load-bearing, and only a + // root that satisfies one but not the other separates them. `1:` has the + // colon in the right place and is still not a drive, so a check accepting + // *either* condition would wave it through. + for path in [r"\\?\1:\", r"\\?\1:\dir"] { + let error = prepare_str(path).expect_err("a digit is not a drive letter"); + assert_eq!( + error.failure(), + PathFailure::NotFullyQualified, + "for {path}" + ); + } + + // Deliberately no companion case for "second unit is not a colon": the + // check is guarded by `root.contains(&COLON)`, so a colonless root -- a + // volume GUID, say -- never reaches it and is accepted on its own terms. + let prepared = prepare_str(r"\\?\Ca\dir").expect("a colonless root is not a drive at all"); + assert_eq!(text(&prepared), r"\\?\Ca\dir"); +} + +#[test] +fn every_path_failure_describes_itself_distinctly() { + // `PathFailure::description -> "xyzzy"` survived: the tests above assert + // which *failure* was reported, never what it says, so a description that + // collapsed every variant onto one string would go unnoticed. + // + // Distinctness is the assertion that matters. A description exists to tell + // one failure from another, so it catches every constant substitution at + // once rather than one string at a time -- and non-emptiness alone would + // not, because a constant is non-empty too. + let cases = [ + ("EmptyPath", PathFailure::EmptyPath), + ("InteriorNul", PathFailure::InteriorNul), + ("PathTooLong", PathFailure::PathTooLong), + ("NotFullyQualified", PathFailure::NotFullyQualified), + ("PathResolution", PathFailure::PathResolution), + ]; + + for (name, failure) in cases { + assert!( + !failure.description().is_empty(), + "{name} has no description, so a reader learns nothing from it" + ); + } + for (index, (name, failure)) in cases.iter().enumerate() { + for (other_name, other) in &cases[index + 1..] { + assert_ne!( + failure.description(), + other.description(), + "{name} and {other_name} describe themselves identically, so the \ + description cannot tell them apart" + ); + } + } +} + +#[test] +fn a_failure_decided_here_carries_no_os_error_and_renders_as_its_description() { + // The half of `PathError` that has no Win32 call behind it. Asserting the + // exact rendering -- rather than merely that it is non-empty -- is what + // binds `Display` to `description`: without it the formatter could drop the + // description entirely and nothing would fail. + let error = prepare_str("").expect_err("an empty path names nothing"); + + assert_eq!(error.failure(), PathFailure::EmptyPath); + assert_eq!(error.raw_os_error(), None); + assert!(std::error::Error::source(&error).is_none()); + assert_eq!(error.to_string(), PathFailure::EmptyPath.description()); +} diff --git a/crates/windows-namespace-request-sys/src/security/tests.rs b/crates/windows-namespace-request-sys/src/security/tests.rs index a23f8f59..7e6f9f02 100644 --- a/crates/windows-namespace-request-sys/src/security/tests.rs +++ b/crates/windows-namespace-request-sys/src/security/tests.rs @@ -509,3 +509,43 @@ fn a_capture_moves_and_shares_across_threads() { assert_eq!(observed, AclState::Populated(1)); } + +#[test] +fn a_capture_failure_exposes_its_os_error_both_ways() { + // `raw_os_error` survived replacement by `None`, `Some(0)`, `Some(1)`, and + // `Some(-1)`, and `source` survived replacement by `None`. The test above + // asserts the failure stage and the rendered message, neither of which + // touches either accessor. + // + // The two routes are asserted against each other rather than against a + // literal code: which error Windows reports for a zeroed descriptor is its + // business, but whatever it is must reach a caller identically through the + // typed accessor and through the standard `source` chain. That also rules + // out every constant the sweep tried, including the plausible-looking ones. + use std::error::Error as _; + + let zeroed = AlignedBuffer::zeroed(size_of::(), SELF_RELATIVE_ALIGNMENT); + + // SAFETY: the buffer outlives the call; its contents are not a valid + // descriptor, which is what makes this fail. + let error = unsafe { SecurityDescriptor::capture(zeroed.as_ptr().cast::()) } + .expect_err("a zeroed descriptor has revision 0 and cannot be valid"); + + let code = error + .raw_os_error() + .expect("this failure came from a Win32 call, so it carries a code"); + assert_ne!( + code, 0, + "a success code would mean the capture had not failed at all" + ); + + let source = error.source().expect("the OS error is the source"); + assert_eq!( + source + .downcast_ref::() + .expect("the source is the io::Error behind the failure") + .raw_os_error(), + Some(code), + "the typed accessor and the source chain must report the same error" + ); +} diff --git a/crates/windows-namespace-request-sys/src/volume/tests.rs b/crates/windows-namespace-request-sys/src/volume/tests.rs index c5af630b..2efcfda9 100644 --- a/crates/windows-namespace-request-sys/src/volume/tests.rs +++ b/crates/windows-namespace-request-sys/src/volume/tests.rs @@ -5,9 +5,10 @@ use std::fs::File; use std::os::windows::io::{AsHandle, AsRawHandle}; -use super::QueryVolumeInformation; +use super::{QueryVolumeInformation, VolumeInformation}; use crate::CapturedHandle; use crate::handle::tests::{Fixture, handle_allocation}; +use wtf_string::Wtf16String; fn request_for(file: &File) -> QueryVolumeInformation { QueryVolumeInformation::new( @@ -189,3 +190,50 @@ fn a_query_performs_the_same_way_on_another_thread() { assert_eq!(observed, expected); } + +#[test] +fn each_accessor_reports_its_own_field() { + // Four accessors -- `label`, `serial_number`, `maximum_component_length`, + // and `flags` -- all survived replacement by constants in a mutation run. + // The tests above query a *real* volume, so they cannot assert exact + // values: whatever this machine reports is what they get, and a constant is + // as plausible as the truth. + // + // Built directly rather than queried, because what is under test is the + // wiring between four same-typed fields and their accessors, not the query. + // Three of them are `u32` and nothing but distinct values can tell a + // transposition apart -- the failure this catches is an accessor returning + // its neighbour, which every real query would hide behind plausible numbers. + let information = VolumeInformation { + label: Wtf16String::from("LABEL"), + serial_number: 0x1111_1111, + maximum_component_length: 0x2222_2222, + flags: 0x3333_3333, + filesystem_name: Wtf16String::from("NTFS"), + }; + + assert_eq!(information.label().to_string_lossy(), "LABEL"); + assert_eq!(information.serial_number(), 0x1111_1111); + assert_eq!(information.maximum_component_length(), 0x2222_2222); + assert_eq!(information.flags(), 0x3333_3333); + assert_eq!(information.filesystem_name().to_string_lossy(), "NTFS"); + + // The property the three numeric assertions rest on. Stated rather than + // eyeballed, so a later edit that reused a value would fail here instead of + // silently weakening the test into one that cannot detect a transposition. + let numeric = [ + information.serial_number(), + information.maximum_component_length(), + information.flags(), + ]; + for (index, value) in numeric.iter().enumerate() { + for other in &numeric[index + 1..] { + assert_ne!(value, other, "two numeric fields share a value"); + } + } + assert_ne!( + information.label(), + information.filesystem_name(), + "the two string fields must differ, or one accessor could serve both" + ); +} diff --git a/crates/windows-namespace-request-sys/src/watch/tests.rs b/crates/windows-namespace-request-sys/src/watch/tests.rs index 1544f131..5f550b9f 100644 --- a/crates/windows-namespace-request-sys/src/watch/tests.rs +++ b/crates/windows-namespace-request-sys/src/watch/tests.rs @@ -289,3 +289,51 @@ fn a_watch_moves_to_another_thread_and_still_signals() { "a notification handle is usable from a thread that did not create it" ); } + +#[test] +fn the_configured_subtree_and_filter_read_back_through_their_accessors() { + // `subtree -> false` and `filter -> Default::default()` both survived a + // mutation run. Every test above builds a watch and then *performs* it, and + // the perform path reads the fields directly -- so nothing distinguished an + // accessor reporting the truth from one reporting a constant. + // + // `WatchDirectory::new` defaults `subtree` to false and the filter to an + // empty set, so both values here are deliberately the opposite: a `true` + // subtree, and a filter with bits set. A test that watched the default + // shape would pass against either constant. + let fixture = Fixture::new("watch-accessors"); + let filter = NotifyFilter::FILE_NAME | NotifyFilter::LAST_WRITE; + let request = watch_for(&fixture, filter).with_subtree(true); + + assert!( + request.subtree(), + "a caller that asked to watch the subtree must be able to see that it did" + ); + assert_eq!(request.filter(), filter); + assert_ne!( + filter, + NotifyFilter::default(), + "the filter under test must differ from the default, or a constant \ + accessor passes" + ); +} + +#[test] +fn an_unconfigured_watch_reads_back_as_the_narrowest_one() { + // The other half: the defaults are what `new` documents rather than what an + // accessor invents, and asserting them is what stops the test above from + // being satisfied by an accessor that always reports the configured shape. + let fixture = Fixture::new("watch-defaults"); + let text = fixture + .directory() + .to_str() + .expect("the fixture path is valid UTF-8"); + let request = + WatchDirectory::new(prepare(&Wtf16String::from(text)).expect("prepare the fixture path")); + + assert!( + !request.subtree(), + "watching a whole tree is the expensive choice and must be asked for" + ); + assert_eq!(request.filter(), NotifyFilter::default()); +}