Decide relative path validity from the string, not the working directory - #230
Merged
Merged
Conversation
…ory [patch] `IsDirectoryPathAttribute` called `File.Exists(value)` on every value, and `IsFilePathAttribute` called `Directory.Exists(value)`. For a path that is not fully qualified, those resolve against the process's current working directory, so whether a path was valid depended on what files happened to sit beside whatever was running. That reached consumers. On Linux .NET publishes an extensionless apphost named after the project, so a test project called `ktsu.BlastMerge.Test` leaves a file of exactly that name next to the test assembly. `AppData.AppDomain` converts the app domain name to a `RelativeDirectoryPath`, `File.Exists` found the apphost, and the conversion threw. On Windows the apphost carries a `.exe` suffix, so the same string validated. Nothing about the name differed between the platforms. Both attributes now run the existence check only when the path is fully qualified, which is the case where it names one location and the question is answerable. Anything else is validated by shape alone. Note this also excludes Windows drive-relative paths such as `C:foo`, which resolve against a per-drive working directory and are ambient in the same way. Four tests cover it: the two collision cases reproducing the reported failure, and two pinning that an absolute path pointing at the wrong kind of entry is still rejected. Fixes #196 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UArBGBWxw5dSrddFUcYccd
|
This was referenced Sep 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #196
The bug
IsDirectoryPathAttributecalledFile.Exists(value)on every value, andIsFilePathAttributecalledDirectory.Exists(value). For a path that is not fully qualified, those resolve against the process's current working directory — so whether a path was valid depended on what files happened to sit beside whatever was running.That reached consumers rather than staying in tests. On Linux .NET publishes an extensionless apphost named exactly after the project, so a test project called
ktsu.BlastMerge.Testleaves a file calledktsu.BlastMerge.Testnext to the test assembly.AppData.AppDomainconverts the app domain name to aRelativeDirectoryPath,File.Existsfound the apphost, and the conversion threw:On Windows the apphost carries a
.exesuffix, so nothing was found and the same string validated. Nothing about the name differed between the two platforms — only what was on disk beside the process.The fix
Both attributes now run the existence check only when the path is fully qualified, which is the one case where it names a single location and the question is answerable. Anything else is validated by shape alone.
This is the first of the three options the issue weighed ("restrict the existence check to absolute paths and validate relative ones by shape alone"). It needs no API change and no base directory threaded through validation, so it fixes the live breakage without committing the library to a larger design.
Path.IsPathFullyQualifiedis used rather thanIsPathRooted, which also excludes Windows drive-relative paths likeC:foo— those resolve against a per-drive working directory and are ambient in exactly the same way.IsFilePathAttributecarried the mirrored defect in the same code shape. Fixing only the reported half would have leftRelativeFilePathbroken by the identical mechanism, so both are changed together; the diff is the same four lines twice.Tests
Four tests in
PathValidationAttributeTests, all reproducing or pinning real behaviour:IsDirectoryPathAttribute_RelativeNameCollidingWithFileInWorkingDirectory_ShouldPassIsFilePathAttribute_RelativeNameCollidingWithDirectoryInWorkingDirectory_ShouldPassIsDirectoryPathAttribute_AbsolutePathOfExistingFile_ShouldFailIsFilePathAttribute_AbsolutePathOfExistingDirectory_ShouldFailThe two collision tests were verified to fail against the unmodified attributes and pass with the fix — reverting only the two source files and re-running gives exactly the reported
Cannot convert ...exception on both. The two absolute-path tests pass either way by design: they pin behaviour the fix must preserve.Each creates its fixture under a GUID-suffixed name and cleans up in a
finally, so they are safe under the assembly's method-level parallelization and mutate no process-global state (notably, they do not change the working directory).Verification
dotnet build— clean, 0 warnings (ktsu.Sdk builds warnings as errors)dotnet test Semantics.Test— 1202 tests, 0 failed (1194 passed, 8 skipped as Windows-only).sonarlint/sonar-local.props— the 5 findings reported are all pre-existing, inTextValidationAttributesTests.csandPaths/DirectoryNameTests.cs, neither of which this PR touches. No new findings.Not run:
dotnet format --verify-no-changes, which fails on this repo independently of this change — it does not run the source generator, so the generated quantity types are undefined during its compile.Docs
docs/validation-reference.mddescribed both attributes in terms that no longer match. Updated with the qualification and a note on how to ask the existence question about a relative path (AsAbsolute(baseDirectory)first, then validate).Follow-up, out of scope here
The issue reports the failure via
ktsu.AppDataStorage→ktsu.BlastMerge. Those live in other repos and are not verified by this PR; confirming them green on Linux is the remaining acceptance item on #196 once this ships.🤖 Generated with Claude Code
https://claude.ai/code/session_01UArBGBWxw5dSrddFUcYccd
Generated by Claude Code