Fix TarWriter.TarEntry exception when adding file whose Unix group ow…

…ner does not exist in the system (#81070) * Do not throw if a unix group is non-existent when creating a TarEntry from a file using TarWriter. * Adjust test that was consuming removed interop method. * Add unit tests to add file entry whose group owner does not exist. * Address src feedback. * Address test feedback * Delay creation/deletion of group in test * Test all PosixTarEntry formats Co-authored-by: carlossanlop <[email protected]>
dotnet · Jan 25, 2023 · 1bcd55a · 1bcd55a
1 parent 98ca349
commit 1bcd55a
Show file tree

Hide file tree

Showing 5 changed files with 149 additions and 7 deletions.
diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetGroupName.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetGroupName.cs
@@ -7,19 +7,26 @@
 using System;
 using System.Collections.Generic;
 using System.Reflection;
+using System.IO;
+using System.Diagnostics.CodeAnalysis;
 
 internal static partial class Interop
 {
     internal static partial class Sys
     {
         /// <summary>
-        /// Gets the group name associated to the specified group ID.
+        /// Tries to get the group name associated to the specified group ID.
         /// </summary>
         /// <param name="gid">The group ID.</param>
-        /// <returns>On success, return a string with the group name. On failure, throws an IOException.</returns>
-        internal static string GetGroupName(uint gid) => GetGroupNameInternal(gid) ?? throw GetIOException(GetLastErrorInfo());
+        /// <param name="groupName">When this method returns true, gets the value of the group name associated with the specified id. On failure, it is null.</param>
+        /// <returns>On success, returns true. On failure, returns false.</returns>
+        internal static bool TryGetGroupName(uint gid, [NotNullWhen(returnValue: true)] out string? groupName)
+        {
+            groupName = GetGroupName(gid);
+            return groupName != null;
+        }
 
         [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGroupName", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)]
-        private static unsafe partial string? GetGroupNameInternal(uint uid);
+        private static unsafe partial string? GetGroupName(uint uid);
     }
 }
diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.Unix.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarWriter.Unix.cs
@@ -82,8 +82,10 @@ private TarEntry ConstructEntryForWriting(string fullPath, string entryName, Fil
             entry._header._gid = (int)status.Gid;
             if (!_groupIdentifiers.TryGetValue(status.Gid, out string? gName))
             {
-                gName = Interop.Sys.GetGroupName(status.Gid);
-                _groupIdentifiers.Add(status.Gid, gName);
+                if (Interop.Sys.TryGetGroupName(status.Gid, out gName))
+                {
+                    _groupIdentifiers.Add(status.Gid, gName);
+                }
             }
             entry._header._gName = gName;
 

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
@@ -1,6 +1,7 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
+using System.Diagnostics;
 using System.IO;
 using Xunit;
 
@@ -20,7 +21,7 @@ protected void VerifyPlatformSpecificMetadata(string filePath, TarEntry entry)
 
             if (entry is PosixTarEntry posix)
             {
-                string gname = Interop.Sys.GetGroupName(status.Gid);
+                Assert.True(Interop.Sys.TryGetGroupName(status.Gid, out string gname));
                 string uname = Interop.Sys.GetUserNameFromPasswd(status.Uid);
 
                 Assert.Equal(gname, posix.GroupName);
@@ -51,5 +52,49 @@ protected void VerifyPlatformSpecificMetadata(string filePath, TarEntry entry)
                 }
             }
         }
+
+        protected int CreateGroup(string groupName)
+        {
+            Execute("groupadd", groupName);
+            return GetGroupId(groupName);
+        }
+
+        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 void SetGroupAsOwnerOfFile(string groupName, string filePath) =>
+            Execute("chgrp", $"{groupName} {filePath}");
+
+
+        protected void DeleteGroup(string groupName) =>
+            Execute("groupdel", groupName);
+
+        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.RedirectStandardOutput = true;
+            p.StartInfo.RedirectStandardError = true;
+
+            p.Start();
+            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}");
+            }
+
+            return standardOutput;
+        }
     }
 }
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
@@ -139,5 +139,49 @@ public void Add_CharacterDevice(TarEntryFormat format)
 
             }, format.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose();
         }
+
+        [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))]
+        [InlineData(TarEntryFormat.Ustar)]
+        [InlineData(TarEntryFormat.Pax)]
+        [InlineData(TarEntryFormat.Gnu)]
+        public void CreateEntryFromFileOwnedByNonExistentGroup(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);
+
+                try
+                {
+                    SetGroupAsOwnerOfFile(groupName, filePath);
+                }
+                finally
+                {
+                    DeleteGroup(groupName);
+                }
+
+                using MemoryStream archive = new MemoryStream();
+                using (TarWriter writer = new TarWriter(archive, Enum.Parse<TarEntryFormat>(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(groupId, entry.Gid);
+                    Assert.Null(reader.GetNextEntry());
+                }
+            }, f.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose();
+        }
     }
 }
diff --git a/...libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.File.Tests.Unix.cs b/...libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntryAsync.File.Tests.Unix.cs
@@ -149,5 +149,49 @@ public void Add_CharacterDevice_Async(TarEntryFormat format)
                 }
             }, format.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose();
         }
+
+        [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndPrivilegedProcess))]
+        [InlineData(TarEntryFormat.Ustar)]
+        [InlineData(TarEntryFormat.Pax)]
+        [InlineData(TarEntryFormat.Gnu)]
+        public void CreateEntryFromFileOwnedByNonExistentGroup_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);
+
+                try
+                {
+                    SetGroupAsOwnerOfFile(groupName, filePath);
+                }
+                finally
+                {
+                    DeleteGroup(groupName);
+                }
+
+                await using MemoryStream archive = new MemoryStream();
+                await using (TarWriter writer = new TarWriter(archive, Enum.Parse<TarEntryFormat>(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(entry.GroupName, string.Empty);
+                    Assert.Equal(groupId, entry.Gid);
+                    Assert.Null(await reader.GetNextEntryAsync());
+                }
+            }, f.ToString(), new RemoteInvokeOptions { RunAsSudo = true }).Dispose();
+        }
     }
 }