Skip to content

Commit

Permalink
CA-375900: Prepend \\?\ to file paths when creating streams for arc…
Browse files Browse the repository at this point in the history
…hive generation (#3184)

* CA-375900: Prepend `//?/` to file paths when creating streams for archive generation
The string works to enable creation of files with paths larger than 260 characters.
* CA-375900: Add directory support and rename utility method
* Fix whitespace in `ArchiveWriterTest`
* CA-375900: Explicitly enumerate files and directories in `ArchiveWriter`
`Directory.GetFiles` and `Directory.GetDirectories` do not enumerate if paths are longer than 260, even when prepended with `//?/`.
* CA-375900: Add long path tests to `ArchiveWriter`
* CA-375900: Add long path tests to `ArchiveIterator`
* CA-375900: Ensure files are added to folders in archive
* Use a recursive method to add directories and files to archive in `ArchiveIterator`
Also improves progress reporting by basing it on directory count
* Fix typos
* Expand `ArchiveWriterTests` to cover all combinations of directory and path lengths
* Ensure that directories used in recursive `Directory.Delete` calls are using long path format
If files in the directory exceed the 260 character limit, the calls will fail
* Expand `ArchiveIteratorTests` to cover all combinations of directory and path lengths
* Ensure relative path name removes `rootPath`
* Fix typo
* Do not use long paths when importing OVFs
The import uses `DiscUtils` which cannot handle paths prepended with `//?/`
* Remove use of `ToLongWindowsPath` within appliance export
This partially reverts commit 819425855c56c14b937849714b359003465bd2f4.
* Refactoring and some corrections.

Signed-off-by: Danilo Del Busso <[email protected]>
Signed-off-by: Konstantina Chremmou <[email protected]>
Co-authored-by: Konstantina Chremmou <[email protected]>
  • Loading branch information
danilo-delbusso and kc284 authored Sep 22, 2023
1 parent 9470e20 commit 86fa2f6
Show file tree
Hide file tree
Showing 7 changed files with 305 additions and 105 deletions.
110 changes: 71 additions & 39 deletions XenAdminTests/ArchiveTests/ArchiveIteratorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
using System.IO;
using System.Text;
using NUnit.Framework;
using XenCenterLib;
using XenCenterLib.Archive;

namespace XenAdminTests.ArchiveTests
Expand All @@ -55,7 +56,7 @@ public Stream ExtractedFileReturn
disposed = false;
}
}
public string CurrentFileNameReturn { private get; set; }
public string CurrentFileNameReturn { get; set; }
public long CurrentFileSizeReturn { private get; set; }
public DateTime ModTimeReturn { private get; set; }
public bool IsDirectoryReturn { private get; set; }
Expand Down Expand Up @@ -97,10 +98,10 @@ public override void ExtractCurrentFile(Stream extractedFileContents, Action can

public override string CurrentFileName()
{
if (String.IsNullOrEmpty(CurrentFileNameReturn))
if (string.IsNullOrEmpty(CurrentFileNameReturn))
return CurrentFileNameReturn;

return NumberOfCallsLeftReturn + CurrentFileNameReturn;
return CurrentFileNameReturn + NumberOfCallsLeftReturn;
}

public override long CurrentFileSize()
Expand All @@ -121,19 +122,15 @@ public override bool IsDirectory()
protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
if( !disposed )
{
if( disposing )
{
if(extractedFile != null)
extractedFile.Dispose();
}
}

if (!disposed && disposing)
extractedFile?.Dispose();
}
}
#endregion

private ArchiveIteratorFake fakeIterator;
private string tempPath;

[OneTimeSetUp]
public void Setup()
Expand All @@ -150,66 +147,101 @@ public void TearDown()
[SetUp]
public void TestSetup()
{
tempPath = null;
fakeIterator.Reset();
}

[TearDown]
public void TestTearDown()
{
if (Directory.Exists(tempPath))
Directory.Delete(tempPath, true);
}


[Test]
public void AnExceptionIsThrownForNullArgumentWhenCallingExtractAllContents()
public void TestExtractToNullDestinationPath()
{
Assert.Throws(typeof(ArgumentNullException), () => fakeIterator.ExtractAllContents(null));
}

[Test]
public void AnExceptionIsThrownForANullFileNameWhenCallingExtractAllContents()
public void TestExtractNullFile()
{
fakeIterator.CurrentFileNameReturn = null;
Assert.Throws(typeof(ArgumentNullException), () => fakeIterator.ExtractAllContents(Path.GetTempPath()));
Assert.Throws(typeof(NullReferenceException), () => fakeIterator.ExtractAllContents(Path.GetTempPath()));
}

[Test]
public void VerifyAFileIsWrittenWhenCallingExtractAllContents()

[TestCase(true, true)]
[TestCase(true, false)]
[TestCase(false, true)]
[TestCase(false, false)]
public void TestExtractFile(bool longDirectoryPath, bool longFilePath)
{
string tempPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
tempPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());

var dirCharNumber = (longDirectoryPath ? 248 : 247) - tempPath.Length - 2;
//2 was removed for the combining slash between tempPath and dir, and the combining slash between dir and filename
var dir = new string('A', dirCharNumber);
var fileCharNumber = (longFilePath ? 260 : 259) - Path.Combine(tempPath, dir).Length - 2;
//2 was removed for the combining slash between the full dir path and filename, and the NumberOfCallsLeftReturn
var fileName = new string('B', fileCharNumber);

const int numberOfFiles = 3;
fakeIterator.NumberOfCallsLeftReturn = numberOfFiles;
fakeIterator.CurrentFileNameReturn = Path.Combine(dir, fileName);
fakeIterator.ExtractAllContents(tempPath);

//Test file has been created
string targetFile = Path.Combine(tempPath, fakeIterator.CurrentFileName());
Assert.IsTrue(File.Exists(targetFile), "File Exists");
for (var i = 0; i < 3; i++)
{
string targetFile = Path.Combine(tempPath, fakeIterator.CurrentFileNameReturn + i);

Assert.IsTrue(File.ReadAllBytes(targetFile).Length > 1, "File length > 1");
if (longDirectoryPath || longFilePath)
targetFile = StringUtility.ToLongWindowsPathUnchecked(targetFile);

//Check recursively that there are only the correct number of files
Assert.IsTrue(Directory.GetFiles(tempPath, "*.*", SearchOption.AllDirectories).Length == numberOfFiles, "File number is correct");
Assert.IsTrue(File.Exists(targetFile), "File should exist");
Assert.IsNotEmpty(File.ReadAllBytes(targetFile), "File should not be empty");

Assert.IsFalse((File.GetAttributes(targetFile) & FileAttributes.Directory) == FileAttributes.Directory, "Is not a dir");
Assert.IsFalse((File.GetAttributes(targetFile) & FileAttributes.Directory) == FileAttributes.Directory,
"It should not have directory attributes");
}

//Check recursively that there are only the correct number of files
var actualFileNumber = Directory.GetFiles(tempPath, "*.*", SearchOption.AllDirectories).Length;
Assert.AreEqual(numberOfFiles, actualFileNumber, $"There should be {numberOfFiles}");

Directory.Delete(tempPath,true);
if (longDirectoryPath || longFilePath)
tempPath = StringUtility.ToLongWindowsPathUnchecked(tempPath);
}

[Test]
public void VerifyADirectoryIsWrittenWhenCallingExtractAllContents()

[TestCase(true)]
[TestCase(false)]
public void TestExtractDirectory(bool longDirectoryPath)
{
string tempPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
tempPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
var dirCharNumber = (longDirectoryPath ? 248 : 247) - tempPath.Length - 2;
//2 was removed for the combining slash between tempPath and dir, and the NumberOfCallsLeftReturn
var dir = new string('A', dirCharNumber);

fakeIterator.IsDirectoryReturn = true;
fakeIterator.CurrentFileNameReturn = "FakePath" + Path.DirectorySeparatorChar;
fakeIterator.CurrentFileNameReturn = dir;
fakeIterator.ExtractAllContents(tempPath);

//Test file has been created
string targetPath = Path.Combine(tempPath, fakeIterator.CurrentFileName());
Assert.IsFalse(File.Exists(targetPath), "No files exist");
Assert.IsTrue(Directory.Exists(targetPath), "Directories exist");

//No files - just a directory
Assert.IsTrue(Directory.GetFiles(tempPath).Length < 1, "No file in the directory" );
if (longDirectoryPath)
targetPath = StringUtility.ToLongWindowsPathUnchecked(targetPath);

//Check it's a directory
Assert.IsTrue((File.GetAttributes(targetPath) & FileAttributes.Directory) == FileAttributes.Directory, "Has directory attributes");
Assert.IsFalse(File.Exists(targetPath), "Files should not exist");
Assert.IsEmpty(Directory.GetFiles(tempPath), "Directory should not have files");

Directory.Delete(tempPath, true);
}
Assert.IsTrue(Directory.Exists(targetPath), "Directory should exist");
Assert.IsTrue((File.GetAttributes(targetPath) & FileAttributes.Directory) == FileAttributes.Directory,
"It should have directory attributes");

if (longDirectoryPath)
tempPath = StringUtility.ToLongWindowsPathUnchecked(tempPath);
}
}
}
131 changes: 106 additions & 25 deletions XenAdminTests/ArchiveTests/ArchiveWriterTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
using System.Linq;
using System.Text;
using NUnit.Framework;
using XenCenterLib;
using XenCenterLib.Archive;

namespace XenAdminTests.ArchiveTests
Expand Down Expand Up @@ -68,18 +69,18 @@ private void DisposeStreamList()
{
if (AddedStreamData != null)
{
foreach (Stream stream in AddedStreamData)
foreach (Stream stream in AddedStreamData)
{
if( stream != null )
if (stream != null)
stream.Dispose();
}
}
}
}

public override void Add(Stream filetoAdd, string fileName, DateTime modificationTime, Action cancellingDelegate)
public override void Add(Stream fileToAdd, string fileName, DateTime modificationTime, Action cancellingDelegate)
{
disposed = false;
AddedStreamData.Add(filetoAdd);
AddedStreamData.Add(fileToAdd);
AddedFileNameData.Add(fileName);
AddedDates.Add(modificationTime);
}
Expand All @@ -93,9 +94,9 @@ public override void AddDirectory(string directoryName, DateTime modificationTim
protected override void Dispose(bool disposing)
{
base.Dispose(disposing);
if(disposing)
if (disposing)
{
if( !disposed )
if (!disposed)
{
DisposeStreamList();
}
Expand Down Expand Up @@ -136,20 +137,10 @@ public void DatelessAddCallsImplementation()
Assert.AreEqual(1, fakeWriter.AddedDates.Count);
Assert.AreEqual(fileName, fakeWriter.AddedFileNameData[0], "File name");
Assert.IsTrue(fakeWriter.AddedStreamData[0].Length == 14, "Stream has data");
AssertCurrentDateIsPlausible(fakeWriter.AddedDates[0]);
Assert.That(fakeWriter.AddedDates[0], Is.EqualTo(DateTime.Now).Within(TimeSpan.FromSeconds(5)));
}
}

private void AssertCurrentDateIsPlausible(DateTime currentDate)
{
//If this is failing check that the number of seconds is enough
const double seconds = 5.0;
DateTime maxDate = DateTime.Now.AddSeconds(seconds);
DateTime minDate = DateTime.Now.AddSeconds(-1.0 * seconds);
Assert.IsTrue(currentDate > minDate, "Date is > minimum");
Assert.IsTrue(currentDate < maxDate, "Date is < maximum");
}

[Test]
public void DatelessAddDirectoryCallsImplementation()
{
Expand All @@ -164,7 +155,7 @@ public void DatelessAddDirectoryCallsImplementation()
Assert.AreEqual(0, fakeWriter.AddedStreamData.Count);
Assert.AreEqual(totalAdded, fakeWriter.AddedDates.Count);
Assert.AreEqual(dirName, fakeWriter.AddedFileNameData[0], "File name");
AssertCurrentDateIsPlausible(fakeWriter.AddedDates[0]);
Assert.That(fakeWriter.AddedDates[0], Is.EqualTo(DateTime.Now).Within(TimeSpan.FromSeconds(5)));
}

[Test]
Expand All @@ -173,6 +164,96 @@ public void CreateArchiveThrowsWithBadPath()
Assert.Throws(typeof(FileNotFoundException), () => fakeWriter.CreateArchive("Yellow brick road - not a path!"));
}

[TestCase(true, true)]
[TestCase(false, true)]
[TestCase(true, false)]
[TestCase(false, false)]
public void CreateArchiveWithLongPath(bool longDirectoryPath, bool longFilePath)
{
//set up the path to zip
var zipPath = PopulateLongPathArchive(true, longDirectoryPath, longFilePath, out var addedData);

fakeWriter.CreateArchive(zipPath);

foreach (var datum in addedData)
Assert.Contains(datum, fakeWriter.AddedFileNameData);

// 2 folders and one file
Assert.AreEqual(addedData.Count, fakeWriter.AddedFileNameData.Count);

//clean up: we need to ensure we're deleting the folder
if (longDirectoryPath || longFilePath)
zipPath = StringUtility.ToLongWindowsPathUnchecked(zipPath);

Directory.Delete(zipPath, true);
}

[Test]
public void CreateArchiveWithLongPath_PathTooLong()
{
//! N.B.: If this test fails it might be because the project has moved to a version of .NET Core
//! that does not require calls to `StringUtils.ToLongWindowsPath`. Please review its uses
//! and remove it from the codebase if possible.

// this test ensures PopulateLongPathArchive's correctness
// since CreateArchiveWithLongPath depends on it

Assert.DoesNotThrow(() => PopulateLongPathArchive(false, false, false, out _));
Assert.Throws<DirectoryNotFoundException>(() => PopulateLongPathArchive(false, false, true, out _));
Assert.Throws<PathTooLongException>(() => PopulateLongPathArchive(false, true, true, out _));
Assert.Throws<PathTooLongException>(() => PopulateLongPathArchive(false, true, false, out _));
}

/// <summary>
/// Set up method creating a directory containing 2 subdirectories one of which has a file
/// </summary>
/// <param name="createValidPaths">set to true to ensure folders and files are prepended with \\?\</param>
/// <returns>the path to the folder</returns>
private string PopulateLongPathArchive(bool createValidPaths, bool longDirectoryPath, bool longFilePath, out List<string> addedData)
{
var zipPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());
Directory.CreateDirectory(zipPath);

var dirCharNumber1 = (longDirectoryPath ? 248 : 247) - zipPath.Length - 2;
//2 was removed for the combining slash between tempPath and dir, and the first character
var relativeDirectoryPath1 = 0 + new string('A', dirCharNumber1);

var dirCharNumber2 = (longDirectoryPath ? 248 : 247) - zipPath.Length - 3;
//3 was removed for the combining slash between zipPath and dir, the first character,
//and the combining slash between dir and filename
var relativeDirectoryPath2 = 1 + new string('A', dirCharNumber2);

var fileCharNumber = (longFilePath ? 260 : 259) - Path.Combine(zipPath, relativeDirectoryPath2).Length - 1;
//1 was removed for the combining slash between the full dir path and filename
var fileName = new string('B', fileCharNumber);
var relativeFilePath = Path.Combine(relativeDirectoryPath2, fileName);

addedData = new List<string>
{
relativeDirectoryPath1.Replace(@"\", "/"),
relativeDirectoryPath2.Replace(@"\", "/"),
relativeFilePath.Replace(@"\", "/")
};

var directoryPath1 = Path.Combine(zipPath, relativeDirectoryPath1);
var directoryPath2 = Path.Combine(zipPath, relativeDirectoryPath2);
var filePath = Path.Combine(directoryPath2, fileName);

if (createValidPaths)
{
directoryPath1 = StringUtility.ToLongWindowsPathUnchecked(directoryPath1);
directoryPath2 = StringUtility.ToLongWindowsPathUnchecked(directoryPath2);
filePath = StringUtility.ToLongWindowsPathUnchecked(filePath);
}

Directory.CreateDirectory(directoryPath1);
Directory.CreateDirectory(directoryPath2);
File.WriteAllText(filePath, "Hello, World!");


return zipPath;
}

[Test]
public void CreateArchiveWorksWithValidDirectoryStructure()
{
Expand All @@ -184,17 +265,17 @@ public void CreateArchiveWorksWithValidDirectoryStructure()
{
string subfolder = Path.Combine(tempPath, Path.GetRandomFileName());
Directory.CreateDirectory(subfolder);
CreateFiles( subfolder, i);
CreateFiles(subfolder, i);
}

fakeWriter.CreateArchive(tempPath);

Assert.AreEqual(12, fakeWriter.AddedDates.Count );
Assert.AreEqual(12, fakeWriter.AddedDates.Count);
Assert.AreEqual(12, fakeWriter.AddedFileNameData.Count);
Assert.AreEqual(8, fakeWriter.AddedStreamData.Count);
foreach( DateTime date in fakeWriter.AddedDates )
AssertCurrentDateIsPlausible(date);

foreach (DateTime date in fakeWriter.AddedDates)
Assert.That(date, Is.EqualTo(DateTime.Now).Within(TimeSpan.FromSeconds(5)));

foreach (string name in fakeWriter.AddedFileNameData)
Assert.AreEqual(-1, name.IndexOfAny(@":\".ToArray()), "Unwanted chars found in path");
Expand All @@ -206,7 +287,7 @@ private void CreateFiles(string tempPath, int numberOfFiles)
{
for (int i = 0; i < numberOfFiles; i++)
{
using( FileStream fs = File.OpenWrite(Path.Combine(tempPath, Path.GetRandomFileName())))
using (FileStream fs = File.OpenWrite(Path.Combine(tempPath, Path.GetRandomFileName())))
{
fs.Write(Encoding.ASCII.GetBytes("This is a test"), 0, 14);
fs.Flush();
Expand Down
Loading

0 comments on commit 86fa2f6

Please sign in to comment.