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

Tar: Only treat reparse points marked as junctions or symlinks as actual tar symlinks #89102

Closed
wants to merge 10 commits into from
Closed
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,21 @@ internal static SafeFindHandle FindFirstFile(string fileName, ref WIN32_FIND_DAT
return FindFirstFileExPrivate(fileName, FINDEX_INFO_LEVELS.FindExInfoBasic, ref data, FINDEX_SEARCH_OPS.FindExSearchNameMatch, IntPtr.Zero, 0);
}

internal static void GetFindData(string fullPath, bool isDirectory, bool ignoreAccessDenied, ref WIN32_FIND_DATA findData)
{
using SafeFindHandle handle = FindFirstFile(Path.TrimEndingDirectorySeparator(fullPath), ref findData);
if (handle.IsInvalid)
{
int errorCode = Marshal.GetLastPInvokeError();
// File not found doesn't make much sense coming from a directory.
if (isDirectory && errorCode == Errors.ERROR_FILE_NOT_FOUND)
errorCode = Errors.ERROR_PATH_NOT_FOUND;
if (ignoreAccessDenied && errorCode == Errors.ERROR_ACCESS_DENIED)
return;
throw Win32Marshal.GetExceptionForWin32Error(errorCode, fullPath);
}
}

internal enum FINDEX_INFO_LEVELS : uint
{
FindExInfoStandard = 0x0u,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,14 @@
<Compile Include="$(CommonPath)Interop\Windows\Interop.Errors.cs" Link="Common\Interop\Windows\Interop.Errors.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Interop.Libraries.cs" Link="Common\Interop\Windows\Interop.Libraries.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.CreateHardLink.cs" Link="Common\Interop\Windows\Kernel32\Interop.CreateHardLink.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FILE_TIME.cs" Link="Common\Interop\Windows\Kernel32\Interop.FILE_TIME.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FindFirstFileEx.cs" Link="Common\Interop\Windows\Kernel32\Interop.FindFirstFileEx.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FileOperations.cs" Link="Common\Interop\Windows\Kernel32\Interop.FileOperations.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FindClose.cs" Link="Common\Interop\Windows\Kernel32\Interop.FindClose.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.FormatMessage.cs" Link="Common\Interop\Windows\Kernel32\Interop.FormatMessage.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.MAX_PATH.cs" Link="Common\Interop\Windows\Kernel32\Interop.MAX_PATH.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Kernel32\Interop.WIN32_FIND_DATA.cs" Link="Common\Interop\Windows\Kernel32\Interop.WIN32_FIND_DATA.cs" />
<Compile Include="$(CommonPath)Microsoft\Win32\SafeHandles\SafeFindHandle.Windows.cs" Link="Common\Microsoft\Win32\SafeHandles\SafeFindHandle.Windows.cs" />
<Compile Include="$(CommonPath)System\IO\Archiving.Utils.Windows.cs" Link="Common\System\IO\Archiving.Utils.Windows.cs" />
<Compile Include="$(CommonPath)System\IO\PathInternal.Windows.cs" Link="Common\System\IO\PathInternal.Windows.cs" />
<Compile Include="$(CommonPath)System\IO\Win32Marshal.cs" Link="Common\System\IO\Win32Marshal.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,9 @@ internal static TarEntryType GetCorrectTypeFlagForFormat(TarEntryFormat format,
return entryType;
}

// Chooses the compatible regular file entry type for the specified format.
internal static TarEntryType GetRegularFileEntryTypeForFormat(TarEntryFormat format) => format is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;

/// <summary>Parses a byte span that represents an ASCII string containing a number in octal base.</summary>
internal static T ParseOctal<T>(ReadOnlySpan<byte> buffer) where T : struct, INumber<T>
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Threading;
using System.Threading.Tasks;

namespace System.Formats.Tar
{
Expand Down Expand Up @@ -33,7 +31,7 @@ private TarEntry ConstructEntryForWriting(string fullPath, string entryName, Fil
Interop.Sys.FileTypes.S_IFCHR => TarEntryType.CharacterDevice,
Interop.Sys.FileTypes.S_IFIFO => TarEntryType.Fifo,
Interop.Sys.FileTypes.S_IFLNK => TarEntryType.SymbolicLink,
Interop.Sys.FileTypes.S_IFREG => Format is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile,
Interop.Sys.FileTypes.S_IFREG => TarHelpers.GetRegularFileEntryTypeForFormat(Format),
Interop.Sys.FileTypes.S_IFDIR => TarEntryType.Directory,
_ => throw new IOException(SR.Format(SR.TarUnsupportedFile, fullPath)),
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@

using System.Diagnostics;
using System.IO;
using System.Threading;
using System.Threading.Tasks;

namespace System.Formats.Tar
{
Expand All @@ -21,18 +19,32 @@ private TarEntry ConstructEntryForWriting(string fullPath, string entryName, Fil

FileAttributes attributes = File.GetAttributes(fullPath);

bool isDirectory = (attributes & FileAttributes.Directory) != 0;

Interop.Kernel32.WIN32_FIND_DATA data = default;
Interop.Kernel32.GetFindData(fullPath, isDirectory, ignoreAccessDenied: false, ref data);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be called only when the file is a reparse point?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right.


TarEntryType entryType;
if ((attributes & FileAttributes.ReparsePoint) != 0)
{
entryType = TarEntryType.SymbolicLink;
if (data.dwReserved0 is Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_SYMLINK or
Interop.Kernel32.IOReparseOptions.IO_REPARSE_TAG_MOUNT_POINT)
Comment on lines +30 to +31
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like another data point in favor of having an API for ReparseTags #1908.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed.

{
entryType = TarEntryType.SymbolicLink;
}
else
{
// Treat any other reparse point as regular file
entryType = TarHelpers.GetRegularFileEntryTypeForFormat(Format);
}
}
else if ((attributes & FileAttributes.Directory) != 0)
else if (isDirectory)
{
entryType = TarEntryType.Directory;
}
else if ((attributes & (FileAttributes.Normal | FileAttributes.Archive)) != 0)
{
entryType = Format is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;
entryType = TarHelpers.GetRegularFileEntryTypeForFormat(Format);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public void WriteEntry_LongFileSize(TarEntryFormat entryFormat, long size, bool

using (TarWriter writer = new(s, leaveOpen: true))
{
TarEntry writeEntry = InvokeTarEntryCreationConstructor(entryFormat, entryFormat is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile, "foo");
TarEntry writeEntry = InvokeTarEntryCreationConstructor(entryFormat, GetRegularFileEntryTypeForFormat(entryFormat), "foo");
writeEntry.DataStream = new SimulatedDataStream(size);
writer.WriteEntry(writeEntry);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public async Task WriteEntry_LongFileSizeAsync(TarEntryFormat entryFormat, long

await using (TarWriter writer = new(s, leaveOpen: true))
{
TarEntry writeEntry = InvokeTarEntryCreationConstructor(entryFormat, entryFormat is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile, "foo");
TarEntry writeEntry = InvokeTarEntryCreationConstructor(entryFormat, GetRegularFileEntryTypeForFormat(entryFormat), "foo");
writeEntry.DataStream = new SimulatedDataStream(size);
await writer.WriteEntryAsync(writeEntry);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@
<Compile Include="TarFile\TarFile.ExtractToDirectory.File.Tests.Windows.cs" />
<Compile Include="TarFile\TarFile.ExtractToDirectoryAsync.File.Tests.Windows.cs" />
<Compile Include="TarWriter\TarWriter.File.Base.Windows.cs" />
<Compile Include="TarWriter\TarWriter.WriteEntry.File.Tests.Windows.cs" />
<Compile Include="TarWriter\TarWriter.WriteEntryAsync.File.Tests.Windows.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Interop.BOOL.cs" Link="Common\Interop\Windows\Interop.BOOL.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Interop.Errors.cs" Link="Common\Interop\Windows\Interop.Errors.cs" />
<Compile Include="$(CommonPath)Interop\Windows\Interop.Libraries.cs" Link="Common\Interop\Windows\Interop.Libraries.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ protected async Task Read_Archive_Many_Small_Files_Async_Internal(TarEntryFormat
int directoriesCount = entries.Count(e => e.EntryType == TarEntryType.Directory);
Assert.Equal(10, directoriesCount);

TarEntryType actualEntryType = format is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;
TarEntryType actualEntryType = GetRegularFileEntryTypeForFormat(format);

for (int i = 0; i < 10; i++)
{
Expand Down Expand Up @@ -423,7 +423,7 @@ private void VerifyRegularFileEntry(TarEntry file, TarEntryFormat format, string
Assert.Equal(expectedContents, contents);
}

TarEntryType expectedEntryType = format == TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;
TarEntryType expectedEntryType = GetRegularFileEntryTypeForFormat(format);
Assert.Equal(expectedEntryType, file.EntryType);

Assert.Equal(AssetGid, file.Gid);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ protected void Read_Archive_Many_Small_Files_Internal(TarEntryFormat format, Tes
int directoriesCount = entries.Count(e => e.EntryType == TarEntryType.Directory);
Assert.Equal(10, directoriesCount);

TarEntryType actualEntryType = format is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;
TarEntryType actualEntryType = GetRegularFileEntryTypeForFormat(format);

for (int i = 0; i < 10; i++)
{
Expand Down Expand Up @@ -382,7 +382,7 @@ private void VerifyRegularFileEntry(TarEntry file, TarEntryFormat format, string
Assert.Equal(expectedContents, contents);
}

TarEntryType expectedEntryType = format == TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;
TarEntryType expectedEntryType = GetRegularFileEntryTypeForFormat(format);
Assert.Equal(expectedEntryType, file.EntryType);

Assert.Equal(AssetGid, file.Gid);
Expand Down
2 changes: 2 additions & 0 deletions src/libraries/System.Formats.Tar/tests/TarTestsBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,8 @@ protected static TarEntryType GetTarEntryTypeForTarEntryFormat(TarEntryType entr
return entryType;
}

protected static TarEntryType GetRegularFileEntryTypeForFormat(TarEntryFormat format) => format is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;

protected static TarEntry InvokeTarEntryCreationConstructor(TarEntryFormat targetFormat, TarEntryType entryType, string entryName)
=> targetFormat switch
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;
using Xunit;

namespace System.Formats.Tar.Tests;

public partial class TarWriter_WriteEntry_File_Tests : TarWriter_File_Base
{
[Theory]
[InlineData(TarEntryFormat.V7)]
[InlineData(TarEntryFormat.Ustar)]
[InlineData(TarEntryFormat.Pax)]
[InlineData(TarEntryFormat.Gnu)]
public void Add_Junction_As_SymbolicLink(TarEntryFormat format)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you run these tests without elevation? I think it's missing a can create symlinks condition. I've added them a few times when tests failed locally.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Starting with Windows 10 Insiders build 14972, symlinks can be created without needing to elevate the console as administrator. https://blogs.windows.com/windowsdeveloper/2016/12/02/symlinks-windows-10/

If we still have tests running older Windows versions, they should break in the CI.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Junctions are ancient, they don't require elevation AFAIK. They ran without problem in my local machine. The CI should also cry if something goes wrong.

This new test does not create symlinks in the disk by the way, it adds an entry to the tar archive as a symlink. The only thing that gets created in disk is the junction.

{
using TempDirectory root = new TempDirectory();
string targetName = "TargetDirectory";
string junctionName = "JunctionDirectory";
string targetPath = Path.Join(root.Path, targetName);
string junctionPath = Path.Join(root.Path, junctionName);

Directory.CreateDirectory(targetPath);

Assert.True(MountHelper.CreateJunction(junctionPath, targetPath));
DirectoryInfo junctionInfo = new(junctionPath);

using MemoryStream archive = new MemoryStream();
using (TarWriter writer = new TarWriter(archive, format, leaveOpen: true))
{
writer.WriteEntry(fileName: junctionPath, entryName: junctionPath);
}

archive.Position = 0;
using (TarReader reader = new TarReader(archive))
{
TarEntry entry = reader.GetNextEntry();
Assert.Equal(format, entry.Format);

Assert.NotNull(entry);
Assert.Equal(junctionPath, entry.Name);
Assert.Equal(targetPath, entry.LinkName);
Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType);
Assert.Null(entry.DataStream);

VerifyPlatformSpecificMetadata(junctionPath, entry);

Assert.Null(reader.GetNextEntry());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ public void Add_File(TarEntryFormat format)
Assert.NotNull(entry);
Assert.Equal(format, entry.Format);
Assert.Equal(fileName, entry.Name);
TarEntryType expectedEntryType = format is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;
TarEntryType expectedEntryType = GetRegularFileEntryTypeForFormat(format);
Assert.Equal(expectedEntryType, entry.EntryType);
Assert.True(entry.Length > 0);
Assert.NotNull(entry.DataStream);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -337,7 +337,7 @@ private void WriteLongNameCore(TarEntryFormat format, string maxPathComponent)
MemoryStream ms = new();
using (TarWriter writer = new(ms, true))
{
TarEntryType entryType = format == TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;
TarEntryType entryType = GetRegularFileEntryTypeForFormat(format);
entry = InvokeTarEntryCreationConstructor(format, entryType, maxPathComponent);
writer.WriteEntry(entry);

Expand Down Expand Up @@ -469,7 +469,7 @@ public static IEnumerable<object[]> WriteEntry_UsingTarEntry_FromTarReader_IntoT
{
foreach (var entryFormat in new[] { TarEntryFormat.V7, TarEntryFormat.Ustar, TarEntryFormat.Pax, TarEntryFormat.Gnu })
{
foreach (var entryType in new[] { entryFormat == TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile, TarEntryType.Directory, TarEntryType.SymbolicLink })
foreach (var entryType in new[] { GetRegularFileEntryTypeForFormat(entryFormat), TarEntryType.Directory, TarEntryType.SymbolicLink })
{
foreach (bool unseekableStream in new[] { false, true })
{
Expand Down Expand Up @@ -520,7 +520,7 @@ public void WriteEntry_FileSizeOverLegacyLimit_Throws(TarEntryFormat entryFormat
using Stream s = unseekableStream ? new WrappedStream(ms, ms.CanRead, ms.CanWrite, canSeek: false) : ms;

using TarWriter writer = new(s);
TarEntry writeEntry = InvokeTarEntryCreationConstructor(entryFormat, entryFormat is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile, "foo");
TarEntry writeEntry = InvokeTarEntryCreationConstructor(entryFormat, GetRegularFileEntryTypeForFormat(entryFormat), "foo");
writeEntry.DataStream = new SimulatedDataStream(FileSizeOverLimit);

Assert.Equal(FileSizeOverLimit, writeEntry.Length);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.IO;
using System.Threading.Tasks;
using Xunit;

namespace System.Formats.Tar.Tests;

public partial class TarWriter_WriteEntryAsync_File_Tests : TarWriter_File_Base
{
[Theory]
[InlineData(TarEntryFormat.V7)]
[InlineData(TarEntryFormat.Ustar)]
[InlineData(TarEntryFormat.Pax)]
[InlineData(TarEntryFormat.Gnu)]
public async Task Add_Junction_As_SymbolicLink_Async(TarEntryFormat format)
{
using TempDirectory root = new TempDirectory();
string targetName = "TargetDirectory";
string junctionName = "JunctionDirectory";
string targetPath = Path.Join(root.Path, targetName);
string junctionPath = Path.Join(root.Path, junctionName);

Directory.CreateDirectory(targetPath);

Assert.True(MountHelper.CreateJunction(junctionPath, targetPath));
DirectoryInfo junctionInfo = new(junctionPath);

await using MemoryStream archive = new MemoryStream();
await using (TarWriter writer = new TarWriter(archive, format, leaveOpen: true))
{
await writer.WriteEntryAsync(fileName: junctionPath, entryName: junctionPath);
}

archive.Position = 0;
await using (TarReader reader = new TarReader(archive))
{
TarEntry entry = await reader.GetNextEntryAsync();
Assert.Equal(format, entry.Format);

Assert.NotNull(entry);
Assert.Equal(junctionPath, entry.Name);
Assert.Equal(targetPath, entry.LinkName);
Assert.Equal(TarEntryType.SymbolicLink, entry.EntryType);
Assert.Null(entry.DataStream);

VerifyPlatformSpecificMetadata(junctionPath, entry);

Assert.Null(await reader.GetNextEntryAsync());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public async Task Add_File_Async(TarEntryFormat format)
Assert.NotNull(entry);
Assert.Equal(format, entry.Format);
Assert.Equal(fileName, entry.Name);
TarEntryType expectedEntryType = format is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile;
TarEntryType expectedEntryType = GetRegularFileEntryTypeForFormat(format);
Assert.Equal(expectedEntryType, entry.EntryType);
Assert.True(entry.Length > 0);
Assert.NotNull(entry.DataStream);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ public async Task WriteEntry_FileSizeOverLegacyLimit_Throws_Async(TarEntryFormat

string tarFilePath = GetTestFilePath();
await using TarWriter writer = new(File.Create(tarFilePath));
TarEntry writeEntry = InvokeTarEntryCreationConstructor(entryFormat, entryFormat is TarEntryFormat.V7 ? TarEntryType.V7RegularFile : TarEntryType.RegularFile, "foo");
TarEntry writeEntry = InvokeTarEntryCreationConstructor(entryFormat, GetRegularFileEntryTypeForFormat(entryFormat), "foo");
writeEntry.DataStream = new SimulatedDataStream(FileSizeOverLimit);

Assert.Equal(FileSizeOverLimit, writeEntry.Length);
Expand Down
Loading