5

I have a legacy .NET application that I have ported to .NET 6.0 and am executing cross platform (Windows & Linux).

Much of the legacy code was developed with hard coded path creation, using backslash in strings, like

new StreamWriter(FeuxBarreFolderName + @"\\JazzSchmazz.txt");

that I have converted to something like

new StreamWriter(Path.Combine(FeuxBarreFolderName, "JazzSchmazz.txt"));

I want to prevent any developer from accidentally or purposefully adding \ in a hard coded path again. Other than handling this at code review, is there a way that I can do this automatically, either by code analysis or spectacular failure at testing time? My searches have been unhelpful.

Ideas that I've had

  • Modify the ext4 filesystem on my Docker integration tests to prevent filenames with a \. I have no idea if this is possible / reasonable.
  • Get creative with permissions on my Docker integration tests to prevent creating files in a root directory (take away w from the parent dir), but allow it on the child dir (add w again to a specific child dir). This feels super brittle.
  • Find or write an analyzer to look at strings and complain about any strings that contain a \ character.

Is there a better way here?

Sami Kuhmonen
  • 30,146
  • 9
  • 61
  • 74
Kevin Kostrzewa
  • 193
  • 1
  • 9
  • Note, use `Path.Join` rather than `Path.Combine` – canton7 Mar 24 '22 at 14:37
  • @canton7 Okay - why? Path.Join would result in duplicate path seperates. Ex: `Path.Join(@"C:\somefolder\", @"\somenestedfolder\")` would result in "C:\somefolder\\somenestedfolder\" which doesn't seem desirable. – mason Mar 24 '22 at 14:39
  • @canton7 thanks, but we can't because we have to be .netstandard 2.0 in our packages for reasons. When we can ditch that, we'll certainly consider. – Kevin Kostrzewa Mar 24 '22 at 14:41
  • @mason I think what's being suggested is that I use `new StreamWriter(Path.Join(FeuxBarreFolderName, "JazzSchmazz.txt"));` – Kevin Kostrzewa Mar 24 '22 at 14:43
  • @mason `Path.Combine` returns ``\somenestedfolder\`` in that case, which is even less desiable. `Path.Join` was added because this behaviour of `Path.Combine` is unintuitive and is a source of bugs – canton7 Mar 24 '22 at 14:45
  • 1
    It could be a fun challenge for an analyzer, but it's not going to be able to catch all cases. If most of your cases just have a string literal being passed to a call to some method which is expecting a file path, an analyzer will give good immediate feedback though, and probably won't take much effort. I think testing for filenames which contain ``\`` in your integration tests is probably a sensible thing to do regardless. – canton7 Mar 24 '22 at 14:49
  • 1
    @canton7 yeah, this is possibly a reason for me to own writing and managing an analyzer, which feels interesting. Agreed on the "not all cases". doing a post IT sweep of the container with a "find" looking for filenames with \ feels useful. That won't catch checking for a file's existence, though. – Kevin Kostrzewa Mar 24 '22 at 14:53

1 Answers1

1

As there's not a built in way to protect against this at development or execution time, I'm going to add this code to run after my integration tests. It will sweep the filesystem and throw an exception if it finds any files or directories created with an inappropriate character (backslash).

I wanted to add a check for colon as well, in case folks were creating paths like "c:\feux\barre.txt", but it looks like at least NuGet creates directories with colon (ex. ~/.local/share/NuGet/v3-cache/670c1461c29885f9aa22c281d8b7da90845b38e4$ps:_api.nuget.org_v3_index.json)

void walkTree(System.IO.DirectoryInfo dirInfo, Action<System.IO.FileSystemInfo> action)
{
    try
    {
        foreach (var file in dirInfo.GetFiles())
            action(file);
    }
    catch (System.IO.DirectoryNotFoundException)
    { }

    try
    {
        foreach (var subdir in dirInfo.GetDirectories())
        {
            action(subdir);
            walkTree(subdir, action);
        }
    }
    catch (System.UnauthorizedAccessException)
    { }
    catch (System.IO.DirectoryNotFoundException)
    { }
    catch (System.Security.SecurityException)
    { }
}

foreach (var drive in System.Environment.GetLogicalDrives())
{
    var driveInfo = new System.IO.DriveInfo(drive);

    if (!driveInfo.IsReady)
        continue;
    walkTree(driveInfo.RootDirectory,
        (FileSystemInfo fi) =>
        {
            if (fi.Name.Contains('\\'))
                throw new InvalidOperationException($"'{fi.FullName}' was created with an inappropriate character");
        });
}
Kevin Kostrzewa
  • 193
  • 1
  • 9