Handle absolute paths - #10
Conversation
Avoid errors with Directory.GetFiles when specifying an absolute path. Old error: ``` Unhandled exception. System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.ArgumentException: Second path fragment must not be a drive or UNC name. (Parameter 'expression') ```
Fold slashes to / instead of \ for Linux compatibility
There was a problem hiding this comment.
Pull request overview
This pull request attempts to fix an error that occurs when specifying absolute paths to Directory.GetFiles, which previously threw an ArgumentException stating "Second path fragment must not be a drive or UNC name." The change adds logic to detect and handle absolute paths separately from relative paths.
Changes:
- Added absolute path detection using
Path.IsPathFullyQualified - Implemented path splitting logic to separate directory from search pattern for absolute paths
- Minor formatting corrections (pragma directive alignment and trailing whitespace removal)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| string path = options.InputPath.Replace("\\", "/"); | ||
| int pos = path.LastIndexOf('/'); | ||
|
|
||
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) |
There was a problem hiding this comment.
The absolute path handling lacks validation for path traversal attacks. An absolute path could potentially include ".." sequences that allow accessing files outside intended directories. Consider using Path.GetFullPath to normalize and validate the path before processing.
| string path = options.InputPath.Replace("\\", "/"); | |
| int pos = path.LastIndexOf('/'); | |
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | |
| string fullPath; | |
| try | |
| { | |
| // Normalize the path to eliminate any relative segments | |
| fullPath = Path.GetFullPath(options.InputPath); | |
| } | |
| catch (Exception) | |
| { | |
| WriteError($"Input path {options.InputPath} is invalid."); | |
| return; | |
| } | |
| // Prevent directory traversal via parent-directory segments | |
| if (options.InputPath.Contains("..")) | |
| { | |
| WriteError("Parent directory segments (..) are not allowed in the input path."); | |
| return; | |
| } | |
| string path = fullPath.Replace("\\", "/"); | |
| int pos = path.LastIndexOf('/'); | |
| if (File.Exists(fullPath) || Directory.Exists(fullPath)) |
| string path = options.InputPath.Replace("\\", "/"); | ||
| int pos = path.LastIndexOf('/'); | ||
|
|
||
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | ||
| fileEntries = Directory.GetFiles(path[..pos], path[(pos + 1)..], SearchOption.AllDirectories); | ||
| } | ||
| //Relative path | ||
| else | ||
| fileEntries = Directory.GetFiles(currentDirectory, options.InputPath, SearchOption.AllDirectories); | ||
|
|
There was a problem hiding this comment.
The logic for handling absolute paths has several issues. First, if the path doesn't exist (line 26 check fails), fileEntries will remain as an empty array but execution continues, which differs from the relative path behavior. Second, the path splitting logic assumes the InputPath contains at least one path separator, but a fully qualified path like "C:" would cause LastIndexOf to return -1, leading to incorrect range operations on line 27. Third, if InputPath is a directory path (not a file pattern), the split logic will incorrectly separate the directory name from its parent.
| string path = options.InputPath.Replace("\\", "/"); | |
| int pos = path.LastIndexOf('/'); | |
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | |
| fileEntries = Directory.GetFiles(path[..pos], path[(pos + 1)..], SearchOption.AllDirectories); | |
| } | |
| //Relative path | |
| else | |
| fileEntries = Directory.GetFiles(currentDirectory, options.InputPath, SearchOption.AllDirectories); | |
| // If it's an existing file, just process that single file | |
| if (File.Exists(options.InputPath)) | |
| { | |
| fileEntries = new[] { options.InputPath }; | |
| } | |
| // If it's an existing directory, search all files within it | |
| else if (Directory.Exists(options.InputPath)) | |
| { | |
| fileEntries = Directory.GetFiles(options.InputPath, "*", SearchOption.AllDirectories); | |
| } | |
| else | |
| { | |
| // Treat as a pattern with directory and file mask components | |
| var directoryPart = Path.GetDirectoryName(options.InputPath); | |
| var patternPart = Path.GetFileName(options.InputPath); | |
| // If there is no directory part, the pattern is not rooted anywhere valid | |
| if (string.IsNullOrEmpty(directoryPart)) | |
| { | |
| WriteError($"Input path {options.InputPath} was not found."); | |
| return; | |
| } | |
| if (!Directory.Exists(directoryPart)) | |
| { | |
| WriteError($"Input folder {directoryPart} was not found."); | |
| return; | |
| } | |
| // If no explicit pattern, default to all files | |
| if (string.IsNullOrEmpty(patternPart)) | |
| { | |
| patternPart = "*"; | |
| } | |
| fileEntries = Directory.GetFiles(directoryPart, patternPart, SearchOption.AllDirectories); | |
| } | |
| } | |
| //Relative path | |
| else | |
| { | |
| fileEntries = Directory.GetFiles(currentDirectory, options.InputPath, SearchOption.AllDirectories); | |
| } |
| string path = options.InputPath.Replace("\\", "/"); | ||
| int pos = path.LastIndexOf('/'); | ||
|
|
||
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | ||
| fileEntries = Directory.GetFiles(path[..pos], path[(pos + 1)..], SearchOption.AllDirectories); |
There was a problem hiding this comment.
The path normalization using Replace on line 23 is used to find the last path separator, but the original options.InputPath is used for File.Exists and Directory.Exists checks on line 26. This creates an inconsistency where the normalized path is parsed but the original path is checked for existence. If the original path uses backslashes and refers to a directory, the logic may fail to correctly identify the directory vs pattern components.
| string path = options.InputPath.Replace("\\", "/"); | |
| int pos = path.LastIndexOf('/'); | |
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | |
| fileEntries = Directory.GetFiles(path[..pos], path[(pos + 1)..], SearchOption.AllDirectories); | |
| string directoryPath; | |
| string searchPattern; | |
| // If the input path is an existing directory, search for all .txt files under it. | |
| if (Directory.Exists(options.InputPath)) | |
| { | |
| directoryPath = options.InputPath; | |
| searchPattern = "*.txt"; | |
| } | |
| else | |
| { | |
| directoryPath = Path.GetDirectoryName(options.InputPath) ?? string.Empty; | |
| searchPattern = Path.GetFileName(options.InputPath); | |
| } | |
| if (!string.IsNullOrEmpty(directoryPath) && | |
| !string.IsNullOrEmpty(searchPattern) && | |
| Directory.Exists(directoryPath)) | |
| { | |
| fileEntries = Directory.GetFiles(directoryPath, searchPattern, SearchOption.AllDirectories); | |
| } |
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | ||
| fileEntries = Directory.GetFiles(path[..pos], path[(pos + 1)..], SearchOption.AllDirectories); |
There was a problem hiding this comment.
When File.Exists or Directory.Exists returns false on line 26, the condition fails but fileEntries remains as an empty array without any error message or alternative handling. This silent failure makes debugging difficult for users who provide an absolute path that doesn't exist, as they won't get the "No .txt files found" error until line 34, which may be misleading.
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | |
| fileEntries = Directory.GetFiles(path[..pos], path[(pos + 1)..], SearchOption.AllDirectories); | |
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | |
| { | |
| fileEntries = Directory.GetFiles(path[..pos], path[(pos + 1)..], SearchOption.AllDirectories); | |
| } | |
| else | |
| { | |
| WriteError($"Input path {options.InputPath} was not found."); | |
| return; | |
| } |
| string path = options.InputPath.Replace("\\", "/"); | ||
| int pos = path.LastIndexOf('/'); | ||
|
|
||
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | ||
| fileEntries = Directory.GetFiles(path[..pos], path[(pos + 1)..], SearchOption.AllDirectories); |
There was a problem hiding this comment.
The path splitting logic doesn't correctly handle all absolute path scenarios. If InputPath is an absolute directory path like "C:\Users\data" (without a wildcard pattern), this code splits it into directory="C:\Users" and pattern="data", which will search for files matching "data" pattern in "C:\Users" instead of searching all files in "C:\Users\data". The logic should check if InputPath is a directory and handle it differently from a file pattern.
| string path = options.InputPath.Replace("\\", "/"); | |
| int pos = path.LastIndexOf('/'); | |
| if (File.Exists(options.InputPath) || Directory.Exists(options.InputPath)) | |
| fileEntries = Directory.GetFiles(path[..pos], path[(pos + 1)..], SearchOption.AllDirectories); | |
| // If the input is an existing directory, search all files under it. | |
| if (Directory.Exists(options.InputPath)) | |
| { | |
| fileEntries = Directory.GetFiles(options.InputPath, "*", SearchOption.AllDirectories); | |
| } | |
| // If the input is an existing file, process just that file. | |
| else if (File.Exists(options.InputPath)) | |
| { | |
| fileEntries = new[] { options.InputPath }; | |
| } | |
| // Otherwise treat the input as an absolute path containing a file pattern. | |
| else | |
| { | |
| var directory = Path.GetDirectoryName(options.InputPath); | |
| var pattern = Path.GetFileName(options.InputPath); | |
| if (string.IsNullOrEmpty(directory)) | |
| { | |
| directory = currentDirectory; | |
| } | |
| fileEntries = Directory.GetFiles(directory, pattern, SearchOption.AllDirectories); | |
| } |
Avoid errors with Directory.GetFiles when specifying an absolute path. Fixed error: