From 3941a4819b0b9777c27a6eac2ba8b1a3704b7f3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20S=C3=A1nchez=20L=C3=B3pez?= <1175054+carlossanlop@users.noreply.github.com> Date: Thu, 9 Mar 2023 16:29:49 -0800 Subject: [PATCH] Add tar tests to confirm entries owned by deleted users can still be read or written (#83126) * Add tar tests to confirm entries owned by deleted users can still be read and written. * Improve Process execution code to avoid potential deadlock. --- .../TarWriter/TarWriter.File.Base.Unix.cs | 45 +++++- .../TarWriter.WriteEntry.File.Tests.Unix.cs | 127 ++++++++++++++++- ...rWriter.WriteEntryAsync.File.Tests.Unix.cs | 129 +++++++++++++++++- 3 files changed, 288 insertions(+), 13 deletions(-) diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.File.Base.Unix.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.File.Base.Unix.cs index 4bdb470ce2ae0..ccd3404d2eef6 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.File.Base.Unix.cs +++ b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.File.Base.Unix.cs @@ -59,36 +59,69 @@ protected int CreateGroup(string groupName) return GetGroupId(groupName); } + protected int CreateUser(string userName) + { + Execute("useradd", userName); + return GetUserId(userName); + } + protected int GetGroupId(string groupName) { string standardOutput = Execute("getent", $"group {groupName}"); string[] values = standardOutput.Split(':', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); return int.Parse(values[^1]); } - + + protected int GetUserId(string userName) + { + string standardOutput = Execute("id", $"-u {userName}"); + return int.Parse(standardOutput); + } + protected void SetGroupAsOwnerOfFile(string groupName, string filePath) => Execute("chgrp", $"{groupName} {filePath}"); + protected void SetUserAsOwnerOfFile(string userName, string filePath) => + Execute("chown", $"{userName} {filePath}"); - protected void DeleteGroup(string groupName) => + protected void DeleteGroup(string groupName) + { Execute("groupdel", groupName); + Threading.Thread.Sleep(250); + Assert.Throws(() => GetGroupId(groupName)); + } + + protected void DeleteUser(string userName) + { + Execute("userdel", $"-f {userName}"); + Threading.Thread.Sleep(250); + Assert.Throws(() => GetUserId(userName)); + } private string Execute(string command, string arguments) { using Process p = new Process(); - p.StartInfo.UseShellExecute = false; p.StartInfo.FileName = command; p.StartInfo.Arguments = arguments; + + p.StartInfo.UseShellExecute = false; p.StartInfo.RedirectStandardOutput = true; p.StartInfo.RedirectStandardError = true; + string standardError = string.Empty; + p.ErrorDataReceived += new DataReceivedEventHandler((sender, e) => { standardError += e.Data; }); + + string standardOutput = string.Empty; + p.OutputDataReceived += new DataReceivedEventHandler((sender, e) => { standardOutput += e.Data; }); + p.Start(); + + p.BeginOutputReadLine(); + p.BeginErrorReadLine(); + p.WaitForExit(); - string standardOutput = p.StandardOutput.ReadToEnd(); - string standardError = p.StandardError.ReadToEnd(); - if (p.ExitCode != 0) { throw new IOException($"Error '{p.ExitCode}' when executing '{command} {arguments}'. Message: {standardError}"); diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.File.Tests.Unix.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.File.Tests.Unix.cs index df9f842152c82..6423fa830c3f2 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.File.Tests.Unix.cs +++ b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.File.Tests.Unix.cs @@ -1,8 +1,8 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using Microsoft.DotNet.RemoteExecutor; using System.IO; +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.Formats.Tar.Tests @@ -172,13 +172,134 @@ public void CreateEntryFromFileOwnedByNonExistentGroup(TarEntryFormat f) writer.WriteEntry(filePath, fileName); // Should not throw } archive.Seek(0, SeekOrigin.Begin); - + + using (TarReader reader = new TarReader(archive, leaveOpen: false)) + { + PosixTarEntry entry = reader.GetNextEntry() as PosixTarEntry; + Assert.NotNull(entry); + + Assert.Equal(string.Empty, entry.GroupName); + Assert.Equal(groupId, entry.Gid); + + string extractedPath = Path.Join(root.Path, "extracted.txt"); + entry.ExtractToFile(extractedPath, overwrite: false); + Assert.True(File.Exists(extractedPath)); + + Assert.Null(reader.GetNextEntry()); + } + }, f.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose(); + } + + [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))] + [InlineData(TarEntryFormat.Ustar)] + [InlineData(TarEntryFormat.Pax)] + [InlineData(TarEntryFormat.Gnu)] + public void CreateEntryFromFileOwnedByNonExistentUser(TarEntryFormat f) + { + RemoteExecutor.Invoke((string strFormat) => + { + using TempDirectory root = new TempDirectory(); + + string fileName = "file.txt"; + string filePath = Path.Join(root.Path, fileName); + File.Create(filePath).Dispose(); + + string userName = Path.GetRandomFileName()[0..6]; + int userId = CreateUser(userName); + + try + { + SetUserAsOwnerOfFile(userName, filePath); + } + finally + { + DeleteUser(userName); + } + + using MemoryStream archive = new MemoryStream(); + using (TarWriter writer = new TarWriter(archive, Enum.Parse(strFormat), leaveOpen: true)) + { + writer.WriteEntry(filePath, fileName); // Should not throw + } + archive.Seek(0, SeekOrigin.Begin); + + using (TarReader reader = new TarReader(archive, leaveOpen: false)) + { + PosixTarEntry entry = reader.GetNextEntry() as PosixTarEntry; + Assert.NotNull(entry); + + Assert.Equal(string.Empty, entry.UserName); + Assert.Equal(userId, entry.Uid); + + string extractedPath = Path.Join(root.Path, "extracted.txt"); + entry.ExtractToFile(extractedPath, overwrite: false); + Assert.True(File.Exists(extractedPath)); + + Assert.Null(reader.GetNextEntry()); + } + }, f.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose(); + } + + [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))] + [InlineData(TarEntryFormat.Ustar)] + [InlineData(TarEntryFormat.Pax)] + [InlineData(TarEntryFormat.Gnu)] + public void CreateEntryFromFileOwnedByNonExistentGroupAndUser(TarEntryFormat f) + { + RemoteExecutor.Invoke((string strFormat) => + { + using TempDirectory root = new TempDirectory(); + + string fileName = "file.txt"; + string filePath = Path.Join(root.Path, fileName); + File.Create(filePath).Dispose(); + + string groupName = Path.GetRandomFileName()[0..6]; + int groupId = CreateGroup(groupName); + + string userName = Path.GetRandomFileName()[0..6]; + int userId = CreateUser(userName); + + try + { + SetGroupAsOwnerOfFile(groupName, filePath); + } + finally + { + DeleteGroup(groupName); + } + + try + { + SetUserAsOwnerOfFile(userName, filePath); + } + finally + { + DeleteUser(userName); + } + + using MemoryStream archive = new MemoryStream(); + using (TarWriter writer = new TarWriter(archive, Enum.Parse(strFormat), leaveOpen: true)) + { + writer.WriteEntry(filePath, fileName); // Should not throw + } + archive.Seek(0, SeekOrigin.Begin); + using (TarReader reader = new TarReader(archive, leaveOpen: false)) { PosixTarEntry entry = reader.GetNextEntry() as PosixTarEntry; Assert.NotNull(entry); - Assert.Equal(entry.GroupName, string.Empty); + + Assert.Equal(string.Empty, entry.GroupName); Assert.Equal(groupId, entry.Gid); + + Assert.Equal(string.Empty, entry.UserName); + Assert.Equal(userId, entry.Uid); + + string extractedPath = Path.Join(root.Path, "extracted.txt"); + entry.ExtractToFile(extractedPath, overwrite: false); + Assert.True(File.Exists(extractedPath)); + Assert.Null(reader.GetNextEntry()); } }, f.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose(); diff --git a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.File.Tests.Unix.cs b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.File.Tests.Unix.cs index 97d667f896b37..d53503f34c284 100644 --- a/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.File.Tests.Unix.cs +++ b/src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.File.Tests.Unix.cs @@ -1,9 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using Microsoft.DotNet.RemoteExecutor; using System.IO; using System.Threading.Tasks; +using Microsoft.DotNet.RemoteExecutor; using Xunit; namespace System.Formats.Tar.Tests @@ -149,7 +149,7 @@ public void Add_CharacterDevice_Async(TarEntryFormat format) } }, format.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose(); } - + [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))] [InlineData(TarEntryFormat.Ustar)] [InlineData(TarEntryFormat.Pax)] @@ -182,13 +182,134 @@ public void CreateEntryFromFileOwnedByNonExistentGroup_Async(TarEntryFormat f) await writer.WriteEntryAsync(filePath, fileName); // Should not throw } archive.Seek(0, SeekOrigin.Begin); - + await using (TarReader reader = new TarReader(archive, leaveOpen: false)) { PosixTarEntry entry = await reader.GetNextEntryAsync() as PosixTarEntry; Assert.NotNull(entry); - Assert.Equal(entry.GroupName, string.Empty); + + Assert.Equal(string.Empty, entry.GroupName); Assert.Equal(groupId, entry.Gid); + + string extractedPath = Path.Join(root.Path, "extracted.txt"); + await entry.ExtractToFileAsync(extractedPath, overwrite: false); + Assert.True(File.Exists(extractedPath)); + + Assert.Null(await reader.GetNextEntryAsync()); + } + }, f.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose(); + } + + [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))] + [InlineData(TarEntryFormat.Ustar)] + [InlineData(TarEntryFormat.Pax)] + [InlineData(TarEntryFormat.Gnu)] + public void CreateEntryFromFileOwnedByNonExistentUser_Async(TarEntryFormat f) + { + RemoteExecutor.Invoke(async (string strFormat) => + { + using TempDirectory root = new TempDirectory(); + + string fileName = "file.txt"; + string filePath = Path.Join(root.Path, fileName); + File.Create(filePath).Dispose(); + + string userName = Path.GetRandomFileName()[0..6]; + int userId = CreateUser(userName); + + try + { + SetUserAsOwnerOfFile(userName, filePath); + } + finally + { + DeleteUser(userName); + } + + await using MemoryStream archive = new MemoryStream(); + await using (TarWriter writer = new TarWriter(archive, Enum.Parse(strFormat), leaveOpen: true)) + { + await writer.WriteEntryAsync(filePath, fileName); // Should not throw + } + archive.Seek(0, SeekOrigin.Begin); + + await using (TarReader reader = new TarReader(archive, leaveOpen: false)) + { + PosixTarEntry entry = await reader.GetNextEntryAsync() as PosixTarEntry; + Assert.NotNull(entry); + + Assert.Equal(string.Empty, entry.UserName); + Assert.Equal(userId, entry.Uid); + + string extractedPath = Path.Join(root.Path, "extracted.txt"); + await entry.ExtractToFileAsync(extractedPath, overwrite: false); + Assert.True(File.Exists(extractedPath)); + + Assert.Null(await reader.GetNextEntryAsync()); + } + }, f.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose(); + } + + [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))] + [InlineData(TarEntryFormat.Ustar)] + [InlineData(TarEntryFormat.Pax)] + [InlineData(TarEntryFormat.Gnu)] + public void CreateEntryFromFileOwnedByNonExistentGroupAndUser_Async(TarEntryFormat f) + { + RemoteExecutor.Invoke(async (string strFormat) => + { + using TempDirectory root = new TempDirectory(); + + string fileName = "file.txt"; + string filePath = Path.Join(root.Path, fileName); + File.Create(filePath).Dispose(); + + string groupName = Path.GetRandomFileName()[0..6]; + int groupId = CreateGroup(groupName); + + string userName = Path.GetRandomFileName()[0..6]; + int userId = CreateUser(userName); + + try + { + SetGroupAsOwnerOfFile(groupName, filePath); + } + finally + { + DeleteGroup(groupName); + } + + try + { + SetUserAsOwnerOfFile(userName, filePath); + } + finally + { + DeleteUser(userName); + } + + await using MemoryStream archive = new MemoryStream(); + await using (TarWriter writer = new TarWriter(archive, Enum.Parse(strFormat), leaveOpen: true)) + { + await writer.WriteEntryAsync(filePath, fileName); // Should not throw + } + archive.Seek(0, SeekOrigin.Begin); + + await using (TarReader reader = new TarReader(archive, leaveOpen: false)) + { + PosixTarEntry entry = await reader.GetNextEntryAsync() as PosixTarEntry; + Assert.NotNull(entry); + + Assert.Equal(string.Empty, entry.GroupName); + Assert.Equal(groupId, entry.Gid); + + Assert.Equal(string.Empty, entry.UserName); + Assert.Equal(userId, entry.Uid); + + string extractedPath = Path.Join(root.Path, "extracted.txt"); + await entry.ExtractToFileAsync(extractedPath, overwrite: false); + Assert.True(File.Exists(extractedPath)); + Assert.Null(await reader.GetNextEntryAsync()); } }, f.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose();