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

FullPathResolver: Avoid exception for illegal characters #8575

Merged
Merged
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
6 changes: 3 additions & 3 deletions GitCommands/FileAssociatedIconProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public FileAssociatedIconProvider()
/// </remarks>
public Icon Get(string workingDirectory, string relativeFilePath)
{
var extension = Path.GetExtension(relativeFilePath);
var extension = PathUtil.GetExtension(relativeFilePath);
if (string.IsNullOrWhiteSpace(extension))
{
return null;
Expand All @@ -68,7 +68,7 @@ public Icon Get(string workingDirectory, string relativeFilePath)
// extensions from the registry and using p/invokes and WinAPI, which have
// significantly higher maintenance overhead.

var fullPath = Path.Combine(workingDirectory, relativeFilePath);
var fullPath = PathUtil.Combine(workingDirectory, relativeFilePath);
if (!_fileSystem.File.Exists(fullPath))
{
tempFile = CreateTempFile(Path.GetFileName(fullPath));
Expand Down Expand Up @@ -117,4 +117,4 @@ internal void ResetCache()
LoadedFileIcons.Clear();
}
}
}
}
20 changes: 17 additions & 3 deletions GitCommands/FullPathResolver.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ public FullPathResolver(Func<string> getWorkingDir)
/// <summary>
/// Resolves the provided path (folder or file) against the current working directory.
/// </summary>
/// <remarks>
/// Behaves similar to the .NET Core 2.1 version that do not throw on paths with illegal
/// Windows characters (that could be OK in Git paths or for cross platform) but returns
/// null instead of an possible path.
/// but </remarks>
/// <param name="path">Folder or file path to resolve.</param>
/// <returns>
/// <paramref name="path" /> if <paramref name="path" /> is rooted; otherwise resolved path from working directory of the current repository.
Expand All @@ -41,12 +46,21 @@ public string Resolve(string path)
{
if (string.IsNullOrWhiteSpace(path))
{
throw new ArgumentNullException(nameof(path));
return null;
}

if (Path.IsPathRooted(path))
try
{
return path;
if (Path.IsPathRooted(path))
{
return path;
}
}
catch (ArgumentException)
{
// Illegal characters in path.
// This is used for Git paths that may not be possible in the host file system
return null;
}

var workingDir = _getWorkingDir();
Expand Down
4 changes: 2 additions & 2 deletions GitCommands/Git/GitItemStatusFileExtensionComparer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public override int Compare(GitItemStatus x, GitItemStatus y)

var lhsPath = GetPrimarySortingPath(x);
var rhsPath = GetPrimarySortingPath(y);
var lhsExt = Path.GetExtension(lhsPath);
var rhsExt = Path.GetExtension(rhsPath);
var lhsExt = PathUtil.GetExtension(lhsPath);
var rhsExt = PathUtil.GetExtension(rhsPath);

var comparisonResult = StringComparer.InvariantCulture.Compare(lhsExt, rhsExt);
if (comparisonResult == 0)
Expand Down
4 changes: 1 addition & 3 deletions GitCommands/GitRevisionInfoProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,7 @@ IEnumerable<IGitItem> YieldSubItems()
{
if (subItem is GitItem gitItem)
{
gitItem.FileName = Path.Combine(
basePath,
gitItem.FileName ?? string.Empty);
gitItem.FileName = PathUtil.Combine(basePath, gitItem.FileName) ?? string.Empty;
}

yield return subItem;
Expand Down
56 changes: 52 additions & 4 deletions GitCommands/PathUtil.cs
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,54 @@ public static string NormalizePath([NotNull] this string path)
}
}

/// <summary>
/// Wrapper for Path.Combine
/// </summary>
/// <remark>
/// Similar to the .NET Core 2.1 variant, except that null is returned if Windows
/// invalid characters (that may be accepted in Git or other filesystems)
/// are in the paths instead of a possible path (the OS or file system will throw
/// if the paths are invalid).
/// </remark>
/// <param name="path1">initial part</param>
/// <param name="path2">second part</param>
/// <returns>path if it can be combined, null otherwise</returns>
[CanBeNull]
public static string Combine(string path1, string path2)
{
try
{
return Path.Combine(path1, path2);
}
catch (ArgumentException)
{
return null;
}
}

/// <summary>
/// Wrapper for Path.GetExtension
/// </summary>
/// <remark>
/// <see cref="Combine"/> for motivation.
/// </remark>
/// <param name="path">path to check</param>
/// <returns>path if it can be combined, empty otherwise</returns>
[NotNull]
public static string GetExtension(string path)
{
try
{
return Path.GetExtension(path);
}
catch (ArgumentException)
{
// This could return part after a '.' using string commands,
// but wait for .NET Core with this edge case
return string.Empty;
}
}

[NotNull]
public static string Resolve([NotNull] string path, string relativePath = "")
{
Expand Down Expand Up @@ -223,7 +271,7 @@ public static bool TryFindFullPath([NotNull] string fileName, out string fullPat

foreach (var path in EnvironmentPathsProvider.GetEnvironmentValidPaths())
{
fullPath = Path.Combine(path, fileName);
fullPath = Combine(path, fileName);
if (File.Exists(fullPath))
{
return true;
Expand Down Expand Up @@ -251,7 +299,7 @@ public static bool TryFindShellPath([NotNull] string shell, out string shellPath
return true;
}

shellPath = Path.Combine(AppSettings.GitBinDir, shell);
shellPath = Combine(AppSettings.GitBinDir, shell);
if (File.Exists(shellPath))
{
return true;
Expand Down Expand Up @@ -369,7 +417,7 @@ string FindFileInEnvVarFolder(string environmentVariable, string location, strin
return null;
}

var path = Path.Combine(envVarFolder, location);
var path = Combine(envVarFolder, location);
if (!Directory.Exists(path))
{
return null;
Expand All @@ -380,7 +428,7 @@ string FindFileInEnvVarFolder(string environmentVariable, string location, strin

string FindFile(string location, string fileName1)
{
string fullName = Path.Combine(location, fileName1);
string fullName = Combine(location, fileName1);
if (File.Exists(fullName))
{
return fullName;
Expand Down
6 changes: 3 additions & 3 deletions GitCommands/StringExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -179,18 +179,18 @@ public static string Combine([CanBeNull] this string left, [NotNull] string sep,
}

/// <summary>
/// Quotes this string with the specified <paramref name="quotationMark"/>
/// Quotes this string with the specified <paramref name="q"/>
/// </summary>
[Pure]
[NotNull]
public static string Quote([CanBeNull] this string s, [NotNull] string quotationMark = "\"")
public static string Quote([CanBeNull] this string s, [NotNull] string q = "\"")
{
if (s == null)
{
return "";
}

return quotationMark + s + quotationMark;
return $"{q}{s.Replace(q, "\\" + q)}{q}";
}

/// <summary>
Expand Down
4 changes: 2 additions & 2 deletions GitUI/AutoCompletion/CommitAutoCompleteProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public async Task<IEnumerable<AutoCompleteWord>> GetAutoCompleteWordsAsync(Cance
{
cancellationToken.ThrowIfCancellationRequested();

var regex = GetRegexForExtension(Path.GetExtension(file.Name));
var regex = GetRegexForExtension(PathUtil.GetExtension(file.Name));

if (regex != null)
{
Expand Down Expand Up @@ -94,7 +94,7 @@ private static Regex GetRegexForExtension(string extension)

private static IEnumerable<string> ReadOrInitializeAutoCompleteRegexes()
{
var path = Path.Combine(AppSettings.ApplicationDataPath.Value, "AutoCompleteRegexes.txt");
var path = PathUtil.Combine(AppSettings.ApplicationDataPath.Value, "AutoCompleteRegexes.txt");

if (File.Exists(path))
{
Expand Down
15 changes: 13 additions & 2 deletions GitUI/CommandsDialogs/FormBrowse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2015,14 +2015,20 @@ public static void CopyFullPathToClipboard(FileStatusList diffFiles, GitModule m
var fileNames = new StringBuilder();
foreach (var item in diffFiles.SelectedItems)
{
var path = PathUtil.Combine(module.WorkingDir, item.Item.Name);
if (string.IsNullOrWhiteSpace(path))
{
continue;
}

// Only use append line when multiple items are selected.
// This to make it easier to use the text from clipboard when 1 file is selected.
if (fileNames.Length > 0)
{
fileNames.AppendLine();
}

fileNames.Append(Path.Combine(module.WorkingDir, item.Item.Name).ToNativePath());
fileNames.Append(path.ToNativePath());
}

ClipboardUtil.TrySetText(fileNames.ToString());
Expand Down Expand Up @@ -2423,7 +2429,12 @@ public static void OpenContainingFolder(FileStatusList diffFiles, GitModule modu

foreach (var item in diffFiles.SelectedItems)
{
string filePath = Path.Combine(module.WorkingDir, item.Item.Name.ToNativePath());
string filePath = PathUtil.Combine(module.WorkingDir, item.Item.Name.ToNativePath());
if (string.IsNullOrWhiteSpace(filePath))
{
continue;
}

FormBrowseUtil.ShowFileOrParentFolderInFileExplorer(filePath);
}
}
Expand Down
22 changes: 15 additions & 7 deletions GitUI/CommandsDialogs/FormCommit.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2598,14 +2598,20 @@ private void FilenameToClipboardToolStripMenuItemClick(object sender, EventArgs
var fileNames = new StringBuilder();
foreach (var item in list.SelectedItems)
{
var fileName = _fullPathResolver.Resolve(item.Item.Name);
if (string.IsNullOrWhiteSpace(fileName))
{
continue;
}

// Only use append line when multiple items are selected.
// This to make it easier to use the text from clipboard when 1 file is selected.
if (fileNames.Length > 0)
{
fileNames.AppendLine();
}

fileNames.Append(_fullPathResolver.Resolve(item.Item.Name).ToNativePath());
fileNames.Append(fileName.ToNativePath());
}

ClipboardUtil.TrySetText(fileNames.ToString());
Expand Down Expand Up @@ -2700,12 +2706,17 @@ private void editFileToolStripMenuItem_Click(object sender, EventArgs e)
}

var item = list.SelectedGitItem;
if (item == null)
if (item is null)
{
return;
}

var fileName = _fullPathResolver.Resolve(item.Name);
if (string.IsNullOrWhiteSpace(fileName))
{
return;
}

UICommands.StartFileEditorDialog(fileName);

UnstagedSelectionChanged(null, null);
Expand Down Expand Up @@ -3184,13 +3195,10 @@ private void OpenContainingFolder(FileStatusList list)
{
foreach (var item in list.SelectedItems)
{
var fileNames = new StringBuilder();
fileNames.Append(_fullPathResolver.Resolve(item.Item.Name).ToNativePath());

string filePath = fileNames.ToString();
string filePath = _fullPathResolver.Resolve(item.Item.Name);
if (File.Exists(filePath))
{
OsShellUtil.SelectPathInFileExplorer(filePath);
OsShellUtil.SelectPathInFileExplorer(filePath.ToNativePath());
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion GitUI/CommandsDialogs/FormFileHistory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -446,8 +446,13 @@ private void saveAsToolStripMenuItem_Click(object sender, EventArgs e)
orgFileName = FileName;
}

string fullName = _fullPathResolver.Resolve(orgFileName.ToNativePath());
string fullName = _fullPathResolver.Resolve(orgFileName);
if (string.IsNullOrWhiteSpace(fullName))
{
return;
}

fullName = fullName.ToNativePath();
using (var fileDialog = new SaveFileDialog
{
InitialDirectory = Path.GetDirectoryName(fullName),
Expand Down
Loading