From c1789254e008b3859096be9a626ce92068fe1730 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Wed, 4 May 2022 11:22:21 -0700 Subject: [PATCH 01/21] Fix Design time build errors by removing src project as dependency of the test project. --- .../System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj index 63240f7bd9094..05d63a468c197 100644 --- a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj +++ b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj @@ -66,7 +66,4 @@ - - - From 9f020bd7c017303e414fd42498ed6f487c152279 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Wed, 4 May 2022 11:34:27 -0700 Subject: [PATCH 02/21] Add Browser to target platform identifiers. Ensure Browser can consume the Unix specific ArchivingUtils file too. --- .../System.Formats.Tar/src/System.Formats.Tar.csproj | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj index 2cef71d5e33df..ab6b7be1a40b0 100644 --- a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj +++ b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj @@ -1,7 +1,7 @@ true - $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent) + $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent) enable @@ -62,6 +62,9 @@ + + + From c4d444b46ef1b118582c6fc1d0f712d05c813261 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Thu, 5 May 2022 11:46:23 -0700 Subject: [PATCH 03/21] Remove nullable enable from csproj since it's now default --- src/libraries/System.Formats.Tar/ref/System.Formats.Tar.csproj | 1 - src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj | 1 - 2 files changed, 2 deletions(-) diff --git a/src/libraries/System.Formats.Tar/ref/System.Formats.Tar.csproj b/src/libraries/System.Formats.Tar/ref/System.Formats.Tar.csproj index 7e826aa0b1599..b96aeb1787d94 100644 --- a/src/libraries/System.Formats.Tar/ref/System.Formats.Tar.csproj +++ b/src/libraries/System.Formats.Tar/ref/System.Formats.Tar.csproj @@ -1,7 +1,6 @@ $(NetCoreAppCurrent) - enable diff --git a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj index ab6b7be1a40b0..b0d8ca4d0cf83 100644 --- a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj +++ b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj @@ -2,7 +2,6 @@ true $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent) - enable From 354e40abfdab90db6c83db7413f44e79b00aa676 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Mon, 9 May 2022 15:47:32 -0700 Subject: [PATCH 04/21] Use FileStream constructor with FileMode.CreateNew to detect and throw if destination file exists when creating archive. --- .../System.Formats.Tar/src/System/Formats/Tar/TarFile.cs | 8 ++------ .../TarFile/TarFile.ExtractToDirectory.File.Tests.cs | 5 +---- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs index d23eb1f92a19b..118bd33457058 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs @@ -72,12 +72,8 @@ public static void CreateFromDirectory(string sourceDirectoryName, string destin throw new DirectoryNotFoundException(string.Format(SR.IO_PathNotFound_Path, sourceDirectoryName)); } - if (Path.Exists(destinationFileName)) - { - throw new IOException(string.Format(SR.IO_FileExists_Name, destinationFileName)); - } - - using FileStream fs = File.Create(destinationFileName, bufferSize: 0x1000, FileOptions.None); + // Throws if the destination file exists + using FileStream fs = new(destinationFileName, FileMode.CreateNew, FileAccess.Write); CreateFromDirectoryInternal(sourceDirectoryName, fs, includeBaseDirectory, leaveOpen: false); } diff --git a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs index 67466ca344404..70453f4d49931 100644 --- a/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarFile/TarFile.ExtractToDirectory.File.Tests.cs @@ -100,10 +100,7 @@ public void Extract_Archive_File_OverwriteFalse() string filePath = Path.Join(destination.Path, "file.txt"); - using (StreamWriter writer = File.CreateText(filePath)) - { - writer.WriteLine("My existence should cause an exception"); - } + File.Create(filePath).Dispose(); Assert.Throws(() => TarFile.ExtractToDirectory(sourceArchiveFileName, destination.Path, overwriteFiles: false)); } From d04ec6b0ffa653c7d03084174c3c6b89d74dddb5 Mon Sep 17 00:00:00 2001 From: carlossanlop Date: Mon, 9 May 2022 16:13:46 -0700 Subject: [PATCH 05/21] No error checking on syscalls that do not set errno. --- .../src/Interop/Unix/System.Native/Interop.DeviceFiles.cs | 2 +- .../src/System/Formats/Tar/TarWriter.Unix.cs | 2 +- src/native/libs/System.Native/pal_io.c | 8 +------- src/native/libs/System.Native/pal_io.h | 3 +-- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.DeviceFiles.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.DeviceFiles.cs index b212db6417c41..dc8c566a709a0 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.DeviceFiles.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.DeviceFiles.cs @@ -23,6 +23,6 @@ internal static int CreateCharacterDevice(string pathName, uint mode, uint major private static partial int MkNod(string pathName, uint mode, uint major, uint minor); [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetDeviceIdentifiers", SetLastError = true)] - internal static unsafe partial int GetDeviceIdentifiers(ulong dev, uint* majorNumber, uint* minorNumber); + internal static unsafe partial void GetDeviceIdentifiers(ulong dev, uint* majorNumber, uint* minorNumber); } } 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 index dffa8525a9405..82cffb08c98f9 100644 --- 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 @@ -47,7 +47,7 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str uint minor; unsafe { - Interop.CheckIo(Interop.Sys.GetDeviceIdentifiers((ulong)status.Dev, &major, &minor)); + Interop.Sys.GetDeviceIdentifiers((ulong)status.Dev, &major, &minor); } entry._header._devMajor = (int)major; diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index a0714a778f196..ba3a863823702 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -770,23 +770,17 @@ int32_t SystemNative_SymLink(const char* target, const char* linkPath) return result; } -int32_t SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber) +void SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber) { dev_t castedDev = (dev_t)dev; *majorNumber = (uint32_t)major(castedDev); *minorNumber = (uint32_t)minor(castedDev); - return ConvertErrorPlatformToPal(errno); } int32_t SystemNative_MkNod(const char* pathName, uint32_t mode, uint32_t major, uint32_t minor) { dev_t dev = (dev_t)makedev(major, minor); - if (errno > 0) - { - return -1; - } - int32_t result; while ((result = mknod(pathName, (mode_t)mode, dev)) < 0 && errno == EINTR); return result; diff --git a/src/native/libs/System.Native/pal_io.h b/src/native/libs/System.Native/pal_io.h index 1ace89303642a..d5677ccbc7d79 100644 --- a/src/native/libs/System.Native/pal_io.h +++ b/src/native/libs/System.Native/pal_io.h @@ -543,9 +543,8 @@ PALEXPORT int32_t SystemNative_SymLink(const char* target, const char* linkPath) /** * Given a device ID, extracts the major and minor and components and returns them. - * Return 0 on success; otherwise, returns -1 and errno is set. */ -PALEXPORT int32_t SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber); +PALEXPORT void SystemNative_GetDeviceIdentifiers(uint64_t dev, uint32_t* majorNumber, uint32_t* minorNumber); /** * Creates a special or ordinary file. From 9cb26f16fb972194a9dd5a7c8063f248459281bb Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Mon, 9 May 2022 19:18:03 -0700 Subject: [PATCH 06/21] Add RDev field in FileStatus and retrieve it with stat/lstat so devmajor and devminor get their correct values. --- .../Interop/Unix/System.Native/Interop.Stat.cs | 1 + .../src/System/Formats/Tar/TarWriter.Unix.cs | 4 ++-- .../tests/TarReader/TarReader.File.Tests.cs | 18 ++++-------------- .../TarWriter.WriteEntry.File.Tests.Unix.cs | 12 ++++-------- src/native/libs/System.Native/pal_io.c | 1 + src/native/libs/System.Native/pal_io.h | 1 + 6 files changed, 13 insertions(+), 24 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Stat.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Stat.cs index b5136bbfe2777..0796599a09649 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Stat.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.Stat.cs @@ -31,6 +31,7 @@ internal struct FileStatus internal long BirthTime; internal long BirthTimeNsec; internal long Dev; + internal long RDev; internal long Ino; internal uint UserFlags; } 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 index 82cffb08c98f9..77bbc3efdf838 100644 --- 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 @@ -41,13 +41,13 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str _ => throw new FormatException(string.Format(SR.TarInvalidFormat, Format)), }; - if ((entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice) && status.Dev > 0) + if ((entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice) && status.RDev > 0) { uint major; uint minor; unsafe { - Interop.Sys.GetDeviceIdentifiers((ulong)status.Dev, &major, &minor); + Interop.Sys.GetDeviceIdentifiers((ulong)status.RDev, &major, &minor); } entry._header._devMajor = (int)major; diff --git a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs index fa9ea86ed9d43..ef619edb472f9 100644 --- a/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs +++ b/src/libraries/System.Formats.Tar/tests/TarReader/TarReader.File.Tests.cs @@ -713,13 +713,8 @@ private void Verify_Archive_BlockDevice(PosixTarEntry blockDevice, IReadOnlyDict Assert.True(blockDevice.ModificationTime > DateTimeOffset.UnixEpoch); Assert.Equal(expectedFileName, blockDevice.Name); Assert.Equal(AssetUid, blockDevice.Uid); - - // TODO: Figure out why the numbers don't match https://github.com/dotnet/runtime/issues/68230 - // Assert.Equal(AssetBlockDeviceMajor, blockDevice.DeviceMajor); - // Assert.Equal(AssetBlockDeviceMinor, blockDevice.DeviceMinor); - // Remove these two temporary checks when the above is fixed - Assert.True(blockDevice.DeviceMajor > 0); - Assert.True(blockDevice.DeviceMinor > 0); + Assert.Equal(AssetBlockDeviceMajor, blockDevice.DeviceMajor); + Assert.Equal(AssetBlockDeviceMinor, blockDevice.DeviceMinor); Assert.Equal(AssetGName, blockDevice.GroupName); Assert.Equal(AssetUName, blockDevice.UserName); @@ -749,13 +744,8 @@ private void Verify_Archive_CharacterDevice(PosixTarEntry characterDevice, IRead Assert.True(characterDevice.ModificationTime > DateTimeOffset.UnixEpoch); Assert.Equal(expectedFileName, characterDevice.Name); Assert.Equal(AssetUid, characterDevice.Uid); - - // TODO: Figure out why the numbers don't match https://github.com/dotnet/runtime/issues/68230 - //Assert.Equal(AssetBlockDeviceMajor, characterDevice.DeviceMajor); - //Assert.Equal(AssetBlockDeviceMinor, characterDevice.DeviceMinor); - // Remove these two temporary checks when the above is fixed - Assert.True(characterDevice.DeviceMajor > 0); - Assert.True(characterDevice.DeviceMinor > 0); + Assert.Equal(AssetCharacterDeviceMajor, characterDevice.DeviceMajor); + Assert.Equal(AssetCharacterDeviceMinor, characterDevice.DeviceMinor); Assert.Equal(AssetGName, characterDevice.GroupName); Assert.Equal(AssetUName, characterDevice.UserName); 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 2c57ef21f9b16..3a77f243dcf8a 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 @@ -91,10 +91,8 @@ public void Add_BlockDevice(TarFormat format) VerifyPlatformSpecificMetadata(blockDevicePath, entry); - // TODO: Fix how these values are collected, the numbers don't match even though https://github.com/dotnet/runtime/issues/68230 - // they come from stat's dev and from the major/minor syscalls - // Assert.Equal(TestBlockDeviceMajor, entry.DeviceMajor); - // Assert.Equal(TestBlockDeviceMinor, entry.DeviceMinor); + Assert.Equal(TestBlockDeviceMajor, entry.DeviceMajor); + Assert.Equal(TestBlockDeviceMinor, entry.DeviceMinor); Assert.Null(reader.GetNextEntry()); } @@ -138,10 +136,8 @@ public void Add_CharacterDevice(TarFormat format) VerifyPlatformSpecificMetadata(characterDevicePath, entry); - // TODO: Fix how these values are collected, the numbers don't match even though https://github.com/dotnet/runtime/issues/68230 - // they come from stat's dev and from the major/minor syscalls - // Assert.Equal(TestCharacterDeviceMajor, entry.DeviceMajor); - // Assert.Equal(TestCharacterDeviceMinor, entry.DeviceMinor); + Assert.Equal(TestCharacterDeviceMajor, entry.DeviceMajor); + Assert.Equal(TestCharacterDeviceMinor, entry.DeviceMinor); Assert.Null(reader.GetNextEntry()); } diff --git a/src/native/libs/System.Native/pal_io.c b/src/native/libs/System.Native/pal_io.c index ba3a863823702..d196cbf33970f 100644 --- a/src/native/libs/System.Native/pal_io.c +++ b/src/native/libs/System.Native/pal_io.c @@ -190,6 +190,7 @@ c_static_assert(PAL_IN_ISDIR == IN_ISDIR); static void ConvertFileStatus(const struct stat_* src, FileStatus* dst) { dst->Dev = (int64_t)src->st_dev; + dst->RDev = (int64_t)src->st_rdev; dst->Ino = (int64_t)src->st_ino; dst->Flags = FILESTATUS_FLAGS_NONE; dst->Mode = (int32_t)src->st_mode; diff --git a/src/native/libs/System.Native/pal_io.h b/src/native/libs/System.Native/pal_io.h index d5677ccbc7d79..720d7623fee79 100644 --- a/src/native/libs/System.Native/pal_io.h +++ b/src/native/libs/System.Native/pal_io.h @@ -31,6 +31,7 @@ typedef struct int64_t BirthTime; // time the file was created int64_t BirthTimeNsec; // nanosecond part int64_t Dev; // ID of the device containing the file + int64_t RDev; // ID of the device if it is a special file int64_t Ino; // inode number of the file uint32_t UserFlags; // user defined flags } FileStatus; From d94536a9d0b2913ea713c2e1696556c6c84c0323 Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Mon, 9 May 2022 21:42:11 -0700 Subject: [PATCH 07/21] Simplify some File.Open calls with direct FileStream constructor calls. Simplify FileStreamOptions objects too. --- .../src/System/Formats/Tar/TarEntry.cs | 2 +- .../src/System/Formats/Tar/TarFile.cs | 11 ++--------- .../src/System/Formats/Tar/TarWriter.Unix.cs | 11 ++--------- .../System.Formats.Tar/tests/TarTestsBase.cs | 10 ++-------- 4 files changed, 7 insertions(+), 27 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs index 0800f39ecf028..ce5e076cfc38a 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarEntry.cs @@ -431,7 +431,7 @@ private void ExtractAsRegularFile(string destinationFileName) { Debug.Assert(!Path.Exists(destinationFileName)); - FileStreamOptions fileStreamOptions = new FileStreamOptions() + FileStreamOptions fileStreamOptions = new() { Access = FileAccess.Write, Mode = FileMode.CreateNew, diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs index 118bd33457058..d0b365b9409b7 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs @@ -166,15 +166,8 @@ public static void ExtractToDirectory(string sourceFileName, string destinationD throw new DirectoryNotFoundException(string.Format(SR.IO_PathNotFound_Path, destinationDirectoryName)); } - FileStreamOptions fileStreamOptions = new() - { - Access = FileAccess.Read, - BufferSize = 0x1000, - Mode = FileMode.Open, - Share = FileShare.Read - }; - - using FileStream archive = File.Open(sourceFileName, fileStreamOptions); + // FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions + using FileStream archive = new(sourceFileName, new FileStreamOptions()); ExtractToDirectoryInternal(archive, destinationDirectoryName, overwriteFiles, leaveOpen: false); } 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 index 77bbc3efdf838..07b25602819aa 100644 --- 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 @@ -74,16 +74,9 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str if (entry.EntryType is TarEntryType.RegularFile or TarEntryType.V7RegularFile) { - FileStreamOptions options = new() - { - Mode = FileMode.Open, - Access = FileAccess.Read, - Share = FileShare.Read, - Options = FileOptions.None - }; - Debug.Assert(entry._header._dataStream == null); - entry._header._dataStream = File.Open(fullPath, options); + // FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions + entry._header._dataStream = new FileStream(fullPath, new FileStreamOptions()); } WriteEntry(entry); diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index 3df9bf89cc1fe..309b017d38c09 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -99,15 +99,9 @@ protected static string GetTarFilePath(CompressionMethod compressionMethod, Test protected static MemoryStream GetTarMemoryStream(CompressionMethod compressionMethod, TestTarFormat format, string testCaseName) { string path = GetTarFilePath(compressionMethod, format, testCaseName); - FileStreamOptions options = new() - { - Access = FileAccess.Read, - Mode = FileMode.Open, - Share = FileShare.Read - - }; MemoryStream ms = new(); - using (FileStream fs = new FileStream(path, options)) + // FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions + using (FileStream fs = new FileStream(path, new FileStreamOptions())) { fs.CopyTo(ms); } From cdbbddf859d3a1668ebc1c7d14bd82413becbee7 Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Mon, 9 May 2022 23:57:16 -0700 Subject: [PATCH 08/21] Implement and consume p/invokes to retrieve uname from uid and gname from gid. --- .../Unix/System.Native/Interop.GNameUName.cs | 73 +++++++++++++++++++ .../src/System.Formats.Tar.csproj | 1 + .../src/System/Formats/Tar/TarWriter.Unix.cs | 9 +-- .../tests/System.Formats.Tar.Tests.csproj | 1 + .../TarWriter.WriteEntry.File.Tests.Unix.cs | 9 ++- src/native/libs/System.Native/entrypoints.c | 2 + src/native/libs/System.Native/pal_uid.c | 51 +++++++++++++ src/native/libs/System.Native/pal_uid.h | 10 +++ 8 files changed, 149 insertions(+), 7 deletions(-) create mode 100644 src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs new file mode 100644 index 0000000000000..2e1dc5a130be6 --- /dev/null +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs @@ -0,0 +1,73 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Runtime.InteropServices; +using System.Buffers; +using System.Text; +using System; + +internal static partial class Interop +{ + internal static partial class Sys + { + /// + /// Gets the group name associated to the specified group ID. + /// + /// The group ID. + /// On success, return a string with the group name. On failure, returns a null string. + internal static string? GetGName(uint gid) => GetGNameOrUName(gid, isGName: true); + + /// + /// Gets the user name associated to the specified user ID. + /// + /// The user ID. + /// On success, return a string with the user name. On failure, returns a null string. + internal static string? GetUName(uint uid) => GetGNameOrUName(uid, isGName: false); + + private static string? GetGNameOrUName(uint id, bool isGName) + { + // Common max name length, like /etc/passwd, useradd, groupadd + int outputBufferSize = 32; + + while (true) + { + byte[] buffer = ArrayPool.Shared.Rent(outputBufferSize); + try + { + int resultLength = isGName ? + Interop.Sys.GetGName(id, buffer, buffer.Length) : + Interop.Sys.GetUName(id, buffer, buffer.Length); + + if (resultLength < 0) + { + // error + return null; + } + else if (resultLength < buffer.Length) + { + // success + return Encoding.UTF8.GetString(buffer, 0, resultLength); + } + } + finally + { + ArrayPool.Shared.Return(buffer); + } + + // Output buffer was too small, loop around again and try with a larger buffer. + outputBufferSize = buffer.Length * 2; + + if (outputBufferSize > 256) // Upper limit allowed for login name in kernel + { + return null; + } + } + } + + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGName", SetLastError = true)] + private static partial int GetGName(uint uid, byte[] buffer, int bufferSize); + + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUName", SetLastError = true)] + private static partial int GetUName(uint uid, byte[] buffer, int bufferSize); + } +} diff --git a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj index b0d8ca4d0cf83..adfd8ace11fea 100644 --- a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj +++ b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj @@ -57,6 +57,7 @@ + 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 index 07b25602819aa..92da8791311ea 100644 --- 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 @@ -60,12 +60,11 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str entry._header._mode = (status.Mode & 4095); // First 12 bits - entry.Uid = (int)status.Uid; - entry.Gid = (int)status.Gid; + entry._header._uid = (int)status.Uid; + entry._header._gid = (int)status.Gid; - // TODO: Add these p/invokes https://github.com/dotnet/runtime/issues/68230 - entry._header._uName = "";// Interop.Sys.GetUName(); - entry._header._gName = "";// Interop.Sys.GetGName(); + entry._header._uName = Interop.Sys.GetUName(status.Uid) ?? string.Empty; + entry._header._gName = Interop.Sys.GetGName(status.Gid) ?? string.Empty; if (entry.EntryType == TarEntryType.SymbolicLink) { diff --git a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj index 05d63a468c197..2c1fc2e751d37 100644 --- a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj +++ b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj @@ -56,6 +56,7 @@ + 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 3a77f243dcf8a..74ba09d571e5c 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 @@ -157,8 +157,13 @@ partial void VerifyPlatformSpecificMetadata(string filePath, TarEntry entry) if (entry is PosixTarEntry posix) { - Assert.Equal(DefaultGName, posix.GroupName); - Assert.Equal(DefaultUName, posix.UserName); + string gname = Interop.Sys.GetGName(status.Gid); + Assert.NotNull(gname); + string uname = Interop.Sys.GetUName(status.Uid); + Assert.NotNull(uname); + + Assert.Equal(gname, posix.GroupName); + Assert.Equal(uname, posix.UserName); if (entry.EntryType is not TarEntryType.BlockDevice and not TarEntryType.CharacterDevice) { diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index 6a960a42ae14c..907cd86cd90fa 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -268,6 +268,8 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_GetEnv) DllImportEntry(SystemNative_GetEnviron) DllImportEntry(SystemNative_FreeEnviron) + DllImportEntry(SystemNative_GetUName) + DllImportEntry(SystemNative_GetGName) }; EXTERN_C const void* SystemResolveDllImport(const char* name); diff --git a/src/native/libs/System.Native/pal_uid.c b/src/native/libs/System.Native/pal_uid.c index e1f64f03d5019..2608ce74b7805 100644 --- a/src/native/libs/System.Native/pal_uid.c +++ b/src/native/libs/System.Native/pal_uid.c @@ -237,3 +237,54 @@ int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups) return getgroups(ngroups, groups); } + +int32_t SystemNative_GetUName(uint32_t uid, char* buffer, int32_t bufferSize) +{ + assert(buffer != NULL || bufferSize == 0); + assert(bufferSize >= 0); + + if (bufferSize <= 0) + { + errno = EINVAL; + return -1; + } + + errno = 0; + struct passwd *pwd; + while ((pwd = getpwuid(uid)) == NULL && errno == EINTR); + + size_t unameLength = strlen(pwd->pw_name); + if (errno < 0 || unameLength <= 0 || unameLength > (size_t)bufferSize) + { + buffer = NULL; + return -1; + } + SafeStringCopy(buffer, Int32ToSizeT(bufferSize), pwd->pw_name); + return (int32_t)unameLength; +} + +int32_t SystemNative_GetGName(uint32_t gid, char* buffer, int32_t bufferSize) +{ + assert(buffer != NULL || bufferSize == 0); + assert(bufferSize >= 0); + + if (bufferSize <= 0) + { + errno = EINVAL; + return -1; + } + + errno = 0; + struct group *grp; + while ((grp = getgrgid(gid)) == NULL && errno == EINTR); + + size_t gnameLength = strlen(grp->gr_name); + if (errno < 0 || gnameLength <= 0 || gnameLength > (size_t)bufferSize) + { + buffer = NULL; + return -1; + } + + SafeStringCopy(buffer, Int32ToSizeT(bufferSize), grp->gr_name); + return (int32_t)gnameLength; +} diff --git a/src/native/libs/System.Native/pal_uid.h b/src/native/libs/System.Native/pal_uid.h index b9a24f4230460..eab7259d195b4 100644 --- a/src/native/libs/System.Native/pal_uid.h +++ b/src/native/libs/System.Native/pal_uid.h @@ -82,3 +82,13 @@ PALEXPORT int32_t SystemNative_GetGroupList(const char* name, uint32_t group, ui * If the buffer is too small, errno is EINVAL. */ PALEXPORT int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups); + +/** +* Gets the user name associated with the specified user ID and stores it in the buffer. +*/ +PALEXPORT int32_t SystemNative_GetUName(uint32_t uid, char* buffer, int32_t bufferSize); + +/** +* Gets the user name associated with the specified group ID and stores it in the buffer. +*/ +PALEXPORT int32_t SystemNative_GetGName(uint32_t gid, char* buffer, int32_t bufferSize); From e343a3bac017b356c7720ba8d3e9293482e99675 Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Tue, 10 May 2022 19:40:44 -0700 Subject: [PATCH 09/21] size_t variables should not be checked for negative values --- src/native/libs/System.Native/pal_uid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/native/libs/System.Native/pal_uid.c b/src/native/libs/System.Native/pal_uid.c index 2608ce74b7805..6653f90765cac 100644 --- a/src/native/libs/System.Native/pal_uid.c +++ b/src/native/libs/System.Native/pal_uid.c @@ -254,7 +254,7 @@ int32_t SystemNative_GetUName(uint32_t uid, char* buffer, int32_t bufferSize) while ((pwd = getpwuid(uid)) == NULL && errno == EINTR); size_t unameLength = strlen(pwd->pw_name); - if (errno < 0 || unameLength <= 0 || unameLength > (size_t)bufferSize) + if (errno < 0 || unameLength == 0 || unameLength > (size_t)bufferSize) { buffer = NULL; return -1; @@ -279,7 +279,7 @@ int32_t SystemNative_GetGName(uint32_t gid, char* buffer, int32_t bufferSize) while ((grp = getgrgid(gid)) == NULL && errno == EINTR); size_t gnameLength = strlen(grp->gr_name); - if (errno < 0 || gnameLength <= 0 || gnameLength > (size_t)bufferSize) + if (errno < 0 || gnameLength == 0 || gnameLength > (size_t)bufferSize) { buffer = NULL; return -1; From 1af51df21e900cfa673d18d1498379ac89f7673f Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Tue, 10 May 2022 19:42:25 -0700 Subject: [PATCH 10/21] FileStream calls simplified to File.OpenRead --- .../System.Formats.Tar/src/System/Formats/Tar/TarFile.cs | 3 +-- .../src/System/Formats/Tar/TarWriter.Unix.cs | 3 +-- src/libraries/System.Formats.Tar/tests/TarTestsBase.cs | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs index d0b365b9409b7..ba56ac8744161 100644 --- a/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs +++ b/src/libraries/System.Formats.Tar/src/System/Formats/Tar/TarFile.cs @@ -166,8 +166,7 @@ public static void ExtractToDirectory(string sourceFileName, string destinationD throw new DirectoryNotFoundException(string.Format(SR.IO_PathNotFound_Path, destinationDirectoryName)); } - // FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions - using FileStream archive = new(sourceFileName, new FileStreamOptions()); + using FileStream archive = File.OpenRead(sourceFileName); ExtractToDirectoryInternal(archive, destinationDirectoryName, overwriteFiles, leaveOpen: false); } 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 index 92da8791311ea..00daa5cadd2e7 100644 --- 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 @@ -74,8 +74,7 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str if (entry.EntryType is TarEntryType.RegularFile or TarEntryType.V7RegularFile) { Debug.Assert(entry._header._dataStream == null); - // FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions - entry._header._dataStream = new FileStream(fullPath, new FileStreamOptions()); + entry._header._dataStream = File.OpenRead(fullPath); } WriteEntry(entry); diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index 309b017d38c09..5808fc78b0ff5 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -101,7 +101,7 @@ protected static MemoryStream GetTarMemoryStream(CompressionMethod compressionMe string path = GetTarFilePath(compressionMethod, format, testCaseName); MemoryStream ms = new(); // FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions - using (FileStream fs = new FileStream(path, new FileStreamOptions())) + using (FileStream fs = File.OpenRead(path)) { fs.CopyTo(ms); } From d845d564f8dac1a08f063b1f3ab8b61c413fc443 Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Tue, 10 May 2022 19:43:37 -0700 Subject: [PATCH 11/21] Remove check for RDev > 0 --- .../System.Formats.Tar/src/System/Formats/Tar/TarWriter.Unix.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 00daa5cadd2e7..befdcdd5063d8 100644 --- 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 @@ -41,7 +41,7 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str _ => throw new FormatException(string.Format(SR.TarInvalidFormat, Format)), }; - if ((entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice) && status.RDev > 0) + if (entryType is TarEntryType.BlockDevice or TarEntryType.CharacterDevice) { uint major; uint minor; From 94d2cf002a21867cfc430f28a02b6babc7485dfb Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Tue, 10 May 2022 19:44:23 -0700 Subject: [PATCH 12/21] Use dictionary to preserve repeated unames and gnames mapped to uids and gids --- .../src/System/Formats/Tar/TarWriter.Unix.cs | 30 +++++++++++++++++-- 1 file changed, 27 insertions(+), 3 deletions(-) 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 index befdcdd5063d8..b78a473002a82 100644 --- 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 @@ -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.Collections.Generic; using System.Diagnostics; using System.IO; @@ -9,6 +10,9 @@ namespace System.Formats.Tar // Unix specific methods for the TarWriter class. public sealed partial class TarWriter : IDisposable { + private Dictionary? _userIdentifiers; + private Dictionary? _groupIdentifiers; + // Unix specific implementation of the method that reads an entry from disk and writes it into the archive stream. partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, string entryName) { @@ -60,11 +64,31 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str entry._header._mode = (status.Mode & 4095); // First 12 bits + // Uid and UName entry._header._uid = (int)status.Uid; - entry._header._gid = (int)status.Gid; + _userIdentifiers ??= new Dictionary(); + if (!_userIdentifiers.ContainsKey(status.Uid)) + { + entry._header._uName = Interop.Sys.GetUName(status.Uid) ?? string.Empty; + _userIdentifiers.Add(status.Uid, entry._header._uName); + } + else + { + entry._header._uName = _userIdentifiers[status.Uid]; + } - entry._header._uName = Interop.Sys.GetUName(status.Uid) ?? string.Empty; - entry._header._gName = Interop.Sys.GetGName(status.Gid) ?? string.Empty; + // Gid and GName + entry._header._gid = (int)status.Gid; + _groupIdentifiers ??= new Dictionary(); + if (!_groupIdentifiers.ContainsKey(status.Gid)) + { + entry._header._gName = Interop.Sys.GetGName(status.Gid) ?? string.Empty; + _groupIdentifiers.Add(status.Gid, entry._header._gName); + } + else + { + entry._header._gName = _groupIdentifiers[status.Gid]; + } if (entry.EntryType == TarEntryType.SymbolicLink) { From 90cc58c396f20a074b77abf4a050ddfc556aaf8e Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Tue, 10 May 2022 21:59:49 -0700 Subject: [PATCH 13/21] Missing documentation in pal_uid.h new PALEXPORT methods. --- src/native/libs/System.Native/pal_uid.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/native/libs/System.Native/pal_uid.h b/src/native/libs/System.Native/pal_uid.h index eab7259d195b4..f7c205824220f 100644 --- a/src/native/libs/System.Native/pal_uid.h +++ b/src/native/libs/System.Native/pal_uid.h @@ -85,10 +85,14 @@ PALEXPORT int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups); /** * Gets the user name associated with the specified user ID and stores it in the buffer. +* On failure, returns -1 and sets the buffer to NULL. +* On success, returns the length of the uname string, and sets buffer to a valid pointer. */ PALEXPORT int32_t SystemNative_GetUName(uint32_t uid, char* buffer, int32_t bufferSize); /** * Gets the user name associated with the specified group ID and stores it in the buffer. +* On failure, returns -1 and sets the buffer to NULL. +* On success, returns the length of the gname string, and sets buffer to a valid pointer. */ PALEXPORT int32_t SystemNative_GetGName(uint32_t gid, char* buffer, int32_t bufferSize); From fe9a92751bdcaf0d95ab210b11606986af9637f7 Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Wed, 11 May 2022 22:07:24 -0700 Subject: [PATCH 14/21] Adjust syscalls to thread-safe ones. Start with stackalloc, then use loop. --- .../Unix/System.Native/Interop.GNameUName.cs | 71 ++++++++++++------- src/native/libs/System.Native/pal_uid.c | 47 ++++++------ src/native/libs/System.Native/pal_uid.h | 12 ++-- 3 files changed, 76 insertions(+), 54 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs index 2e1dc5a130be6..6e3c6908ce0b9 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs @@ -5,6 +5,7 @@ using System.Buffers; using System.Text; using System; +using System.Collections.Generic; internal static partial class Interop { @@ -15,59 +16,79 @@ internal static partial class Sys /// /// The group ID. /// On success, return a string with the group name. On failure, returns a null string. - internal static string? GetGName(uint gid) => GetGNameOrUName(gid, isGName: true); + internal static string GetGName(uint gid) => GetGNameOrUName(gid, isGName: true); /// /// Gets the user name associated to the specified user ID. /// /// The user ID. /// On success, return a string with the user name. On failure, returns a null string. - internal static string? GetUName(uint uid) => GetGNameOrUName(uid, isGName: false); + internal static string GetUName(uint uid) => GetGNameOrUName(uid, isGName: false); - private static string? GetGNameOrUName(uint id, bool isGName) + private static string GetGNameOrUName(uint id, bool isGName) { - // Common max name length, like /etc/passwd, useradd, groupadd - int outputBufferSize = 32; + int bufferSize = 256; // Upper limit allowed for login name in kernel + + Span buffer = stackalloc byte[bufferSize]; + + int resultLength = GetGNameOrUnameInternal(id, isGName, buffer); + if(resultLength > 0 && resultLength <= buffer.Length) + { + return Encoding.UTF8.GetString(buffer.Slice(0, resultLength)); + } while (true) { - byte[] buffer = ArrayPool.Shared.Rent(outputBufferSize); + bufferSize *= 2; + byte[] pooledBuffer = ArrayPool.Shared.Rent(bufferSize); try { - int resultLength = isGName ? - Interop.Sys.GetGName(id, buffer, buffer.Length) : - Interop.Sys.GetUName(id, buffer, buffer.Length); - - if (resultLength < 0) - { - // error - return null; - } - else if (resultLength < buffer.Length) + resultLength = GetGNameOrUnameInternal(id, isGName, pooledBuffer); + if(resultLength > 0 && resultLength <= pooledBuffer.Length) { - // success - return Encoding.UTF8.GetString(buffer, 0, resultLength); + return Encoding.UTF8.GetString(pooledBuffer.AsSpan(0, resultLength)); } } finally { - ArrayPool.Shared.Return(buffer); + ArrayPool.Shared.Return(pooledBuffer); } - // Output buffer was too small, loop around again and try with a larger buffer. - outputBufferSize = buffer.Length * 2; + if (bufferSize >= 1024) + { + return string.Empty; + } + } + } - if (outputBufferSize > 256) // Upper limit allowed for login name in kernel + private static int GetGNameOrUnameInternal(uint id, bool isGName, Span buffer) + { + unsafe + { + fixed (byte* pBuffer = &MemoryMarshal.GetReference(buffer)) { - return null; + int result = isGName ? + Interop.Sys.GetGName(id, pBuffer, buffer.Length) : + Interop.Sys.GetUName(id, pBuffer, buffer.Length); + + if (result <= 0) + { + ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); + if (errorInfo.Error == Interop.Error.ERANGE) // Insufficient buffer space, try again + { + return -1; + } + throw Interop.GetExceptionForIoErrno(errorInfo); + } + return result; } } } [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGName", SetLastError = true)] - private static partial int GetGName(uint uid, byte[] buffer, int bufferSize); + private static unsafe partial int GetGName(uint uid, byte* buffer, long bufferSize); [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUName", SetLastError = true)] - private static partial int GetUName(uint uid, byte[] buffer, int bufferSize); + private static unsafe partial int GetUName(uint uid, byte* buffer, long bufferSize); } } diff --git a/src/native/libs/System.Native/pal_uid.c b/src/native/libs/System.Native/pal_uid.c index 6653f90765cac..95393d41b4d1f 100644 --- a/src/native/libs/System.Native/pal_uid.c +++ b/src/native/libs/System.Native/pal_uid.c @@ -238,53 +238,54 @@ int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups) return getgroups(ngroups, groups); } -int32_t SystemNative_GetUName(uint32_t uid, char* buffer, int32_t bufferSize) +int32_t SystemNative_GetUName(uint32_t uid, char* buffer, size_t bufferSize) { - assert(buffer != NULL || bufferSize == 0); - assert(bufferSize >= 0); + assert(buffer != NULL); + assert(bufferSize > 0); - if (bufferSize <= 0) + if (buffer == NULL || bufferSize <= 0) { errno = EINVAL; return -1; } + struct passwd u; + struct passwd* pu; + errno = 0; - struct passwd *pwd; - while ((pwd = getpwuid(uid)) == NULL && errno == EINTR); + int e; + while ((e = getpwuid_r(uid, &u, buffer, bufferSize, &pu)) == EINTR); - size_t unameLength = strlen(pwd->pw_name); - if (errno < 0 || unameLength == 0 || unameLength > (size_t)bufferSize) + if (e < 0) { - buffer = NULL; + errno = e; return -1; } - SafeStringCopy(buffer, Int32ToSizeT(bufferSize), pwd->pw_name); - return (int32_t)unameLength; + return (int32_t)strlen(buffer); } -int32_t SystemNative_GetGName(uint32_t gid, char* buffer, int32_t bufferSize) +int32_t SystemNative_GetGName(uint32_t gid, char* buffer, size_t bufferSize) { - assert(buffer != NULL || bufferSize == 0); - assert(bufferSize >= 0); + assert(buffer != NULL); + assert(bufferSize > 0); - if (bufferSize <= 0) + if (buffer == NULL || bufferSize <= 0) { errno = EINVAL; return -1; } + struct group g; + struct group* pg; + errno = 0; - struct group *grp; - while ((grp = getgrgid(gid)) == NULL && errno == EINTR); + int e; + while ((e = getgrgid_r(gid, &g, buffer, bufferSize, &pg)) == EINTR); - size_t gnameLength = strlen(grp->gr_name); - if (errno < 0 || gnameLength == 0 || gnameLength > (size_t)bufferSize) + if (e < 0) { - buffer = NULL; + errno = e; return -1; } - - SafeStringCopy(buffer, Int32ToSizeT(bufferSize), grp->gr_name); - return (int32_t)gnameLength; + return (int32_t)strlen(buffer); } diff --git a/src/native/libs/System.Native/pal_uid.h b/src/native/libs/System.Native/pal_uid.h index f7c205824220f..ec3636e3655c2 100644 --- a/src/native/libs/System.Native/pal_uid.h +++ b/src/native/libs/System.Native/pal_uid.h @@ -85,14 +85,14 @@ PALEXPORT int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups); /** * Gets the user name associated with the specified user ID and stores it in the buffer. -* On failure, returns -1 and sets the buffer to NULL. -* On success, returns the length of the uname string, and sets buffer to a valid pointer. +* On failure, returns -1 and sets errno. +* On success, return the length of the buffer, and the buffer contains the uname. */ -PALEXPORT int32_t SystemNative_GetUName(uint32_t uid, char* buffer, int32_t bufferSize); +PALEXPORT int32_t SystemNative_GetUName(uint32_t uid, char* buffer, size_t bufferSize); /** * Gets the user name associated with the specified group ID and stores it in the buffer. -* On failure, returns -1 and sets the buffer to NULL. -* On success, returns the length of the gname string, and sets buffer to a valid pointer. +* On failure, returns -1 and sets errno. +* On success, return the length of the buffer, and the buffer contains the gname. */ -PALEXPORT int32_t SystemNative_GetGName(uint32_t gid, char* buffer, int32_t bufferSize); +PALEXPORT int32_t SystemNative_GetGName(uint32_t gid, char* buffer, size_t bufferSize); From a8f5cf64508e5bee235472c61510fbfd84b41511 Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Wed, 11 May 2022 22:12:56 -0700 Subject: [PATCH 15/21] Make dicts readonly and non-nullable, use TryGetValue --- .../src/System/Formats/Tar/TarWriter.Unix.cs | 28 +++++++------------ 1 file changed, 10 insertions(+), 18 deletions(-) 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 index b78a473002a82..9612692b19873 100644 --- 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 @@ -10,8 +10,8 @@ namespace System.Formats.Tar // Unix specific methods for the TarWriter class. public sealed partial class TarWriter : IDisposable { - private Dictionary? _userIdentifiers; - private Dictionary? _groupIdentifiers; + private readonly Dictionary _userIdentifiers = new Dictionary(); + private readonly Dictionary _groupIdentifiers = new Dictionary(); // Unix specific implementation of the method that reads an entry from disk and writes it into the archive stream. partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, string entryName) @@ -66,29 +66,21 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str // Uid and UName entry._header._uid = (int)status.Uid; - _userIdentifiers ??= new Dictionary(); - if (!_userIdentifiers.ContainsKey(status.Uid)) + if (!_userIdentifiers.TryGetValue(status.Uid, out string? uName)) { - entry._header._uName = Interop.Sys.GetUName(status.Uid) ?? string.Empty; - _userIdentifiers.Add(status.Uid, entry._header._uName); - } - else - { - entry._header._uName = _userIdentifiers[status.Uid]; + uName = Interop.Sys.GetUName(status.Uid); + _userIdentifiers.Add(status.Uid, uName); } + entry._header._uName = uName; // Gid and GName entry._header._gid = (int)status.Gid; - _groupIdentifiers ??= new Dictionary(); - if (!_groupIdentifiers.ContainsKey(status.Gid)) - { - entry._header._gName = Interop.Sys.GetGName(status.Gid) ?? string.Empty; - _groupIdentifiers.Add(status.Gid, entry._header._gName); - } - else + if (!_groupIdentifiers.TryGetValue(status.Gid, out string? gName)) { - entry._header._gName = _groupIdentifiers[status.Gid]; + gName = Interop.Sys.GetGName(status.Gid); + _groupIdentifiers.Add(status.Gid, gName); } + entry._header._gName = gName; if (entry.EntryType == TarEntryType.SymbolicLink) { From 7b4e7b502ac06ca13e0b9b4a88c5a0b073918950 Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Fri, 13 May 2022 12:44:08 -0700 Subject: [PATCH 16/21] Reuse 'GetNameFromUid' from pal_networking.c, move to pal_uid.c, use similar logic for Gid method. Simplify Interop.Sys method. --- .../Unix/System.Native/Interop.GNameUName.cs | 81 +++------------ .../libs/System.Native/pal_networking.c | 40 +------- src/native/libs/System.Native/pal_uid.c | 99 ++++++++++++------- src/native/libs/System.Native/pal_uid.h | 12 +-- 4 files changed, 84 insertions(+), 148 deletions(-) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs index 6e3c6908ce0b9..c342d229bdf44 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs @@ -6,6 +6,7 @@ using System.Text; using System; using System.Collections.Generic; +using System.Reflection; internal static partial class Interop { @@ -15,80 +16,28 @@ internal static partial class Sys /// Gets the group name associated to the specified group ID. /// /// The group ID. - /// On success, return a string with the group name. On failure, returns a null string. - internal static string GetGName(uint gid) => GetGNameOrUName(gid, isGName: true); + /// On success, return a string with the group name. On failure, throws an IOException. + internal static string GetGName(uint gid) + { + string result = GetGNameInternal(gid); + return (result == null) ? throw GetIOException(GetLastErrorInfo()) : result; + } /// /// Gets the user name associated to the specified user ID. /// /// The user ID. - /// On success, return a string with the user name. On failure, returns a null string. - internal static string GetUName(uint uid) => GetGNameOrUName(uid, isGName: false); - - private static string GetGNameOrUName(uint id, bool isGName) + /// On success, return a string with the user name. On failure, throws an IOException. + internal static string GetUName(uint uid) { - int bufferSize = 256; // Upper limit allowed for login name in kernel - - Span buffer = stackalloc byte[bufferSize]; - - int resultLength = GetGNameOrUnameInternal(id, isGName, buffer); - if(resultLength > 0 && resultLength <= buffer.Length) - { - return Encoding.UTF8.GetString(buffer.Slice(0, resultLength)); - } - - while (true) - { - bufferSize *= 2; - byte[] pooledBuffer = ArrayPool.Shared.Rent(bufferSize); - try - { - resultLength = GetGNameOrUnameInternal(id, isGName, pooledBuffer); - if(resultLength > 0 && resultLength <= pooledBuffer.Length) - { - return Encoding.UTF8.GetString(pooledBuffer.AsSpan(0, resultLength)); - } - } - finally - { - ArrayPool.Shared.Return(pooledBuffer); - } - - if (bufferSize >= 1024) - { - return string.Empty; - } - } - } - - private static int GetGNameOrUnameInternal(uint id, bool isGName, Span buffer) - { - unsafe - { - fixed (byte* pBuffer = &MemoryMarshal.GetReference(buffer)) - { - int result = isGName ? - Interop.Sys.GetGName(id, pBuffer, buffer.Length) : - Interop.Sys.GetUName(id, pBuffer, buffer.Length); - - if (result <= 0) - { - ErrorInfo errorInfo = Interop.Sys.GetLastErrorInfo(); - if (errorInfo.Error == Interop.Error.ERANGE) // Insufficient buffer space, try again - { - return -1; - } - throw Interop.GetExceptionForIoErrno(errorInfo); - } - return result; - } - } + string result = GetUNameInternal(uid); + return (result == null) ? throw GetIOException(GetLastErrorInfo()) : result; } - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGName", SetLastError = true)] - private static unsafe partial int GetGName(uint uid, byte* buffer, long bufferSize); + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGName", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + private static unsafe partial string GetGNameInternal(uint uid); - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUName", SetLastError = true)] - private static unsafe partial int GetUName(uint uid, byte* buffer, long bufferSize); + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUName", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + private static unsafe partial string GetUNameInternal(uint uid); } } diff --git a/src/native/libs/System.Native/pal_networking.c b/src/native/libs/System.Native/pal_networking.c index a4deeeb8d20f4..3169961dbb936 100644 --- a/src/native/libs/System.Native/pal_networking.c +++ b/src/native/libs/System.Native/pal_networking.c @@ -4,6 +4,7 @@ #include "pal_config.h" #include "pal_networking.h" #include "pal_safecrt.h" +#include "pal_uid.h" #include "pal_utilities.h" #include #include @@ -3026,48 +3027,11 @@ int32_t SystemNative_PlatformSupportsDualModeIPv4PacketInfo(void) #endif } -static char* GetNameFromUid(uid_t uid) -{ - size_t bufferLength = 512; - while (1) - { - char *buffer = (char*)malloc(bufferLength); - if (buffer == NULL) - return NULL; - - struct passwd pw; - struct passwd* result; - if (getpwuid_r(uid, &pw, buffer, bufferLength, &result) == 0) - { - if (result == NULL) - { - errno = ENOENT; - free(buffer); - return NULL; - } - else - { - char* name = strdup(pw.pw_name); - free(buffer); - return name; - } - } - - free(buffer); - size_t tmpBufferLength; - if (errno != ERANGE || !multiply_s(bufferLength, (size_t)2, &tmpBufferLength)) - { - return NULL; - } - bufferLength = tmpBufferLength; - } -} - char* SystemNative_GetPeerUserName(intptr_t socket) { uid_t euid; return SystemNative_GetPeerID(socket, &euid) == 0 ? - GetNameFromUid(euid) : + SystemNative_GetUName(euid) : NULL; } diff --git a/src/native/libs/System.Native/pal_uid.c b/src/native/libs/System.Native/pal_uid.c index 95393d41b4d1f..452bcf2e9b21b 100644 --- a/src/native/libs/System.Native/pal_uid.c +++ b/src/native/libs/System.Native/pal_uid.c @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. #include "pal_config.h" +#include "pal_safecrt.h" #include "pal_uid.h" #include "pal_utilities.h" @@ -238,54 +239,76 @@ int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups) return getgroups(ngroups, groups); } -int32_t SystemNative_GetUName(uint32_t uid, char* buffer, size_t bufferSize) +char* SystemNative_GetUName(uint32_t uid) { - assert(buffer != NULL); - assert(bufferSize > 0); - - if (buffer == NULL || bufferSize <= 0) + size_t bufferLength = 512; + while (1) { - errno = EINVAL; - return -1; - } - - struct passwd u; - struct passwd* pu; + char *buffer = (char*)malloc(bufferLength); + if (buffer == NULL) + return NULL; - errno = 0; - int e; - while ((e = getpwuid_r(uid, &u, buffer, bufferSize, &pu)) == EINTR); + struct passwd pw; + struct passwd* result; + if (getpwuid_r(uid, &pw, buffer, bufferLength, &result) == 0) + { + if (result == NULL) + { + errno = ENOENT; + free(buffer); + return NULL; + } + else + { + char* name = strdup(pw.pw_name); + free(buffer); + return name; + } + } - if (e < 0) - { - errno = e; - return -1; + free(buffer); + size_t tmpBufferLength; + if (errno != ERANGE || !multiply_s(bufferLength, (size_t)2, &tmpBufferLength)) + { + return NULL; + } + bufferLength = tmpBufferLength; } - return (int32_t)strlen(buffer); } -int32_t SystemNative_GetGName(uint32_t gid, char* buffer, size_t bufferSize) +char* SystemNative_GetGName(uint32_t gid) { - assert(buffer != NULL); - assert(bufferSize > 0); - - if (buffer == NULL || bufferSize <= 0) + size_t bufferLength = 512; + while (1) { - errno = EINVAL; - return -1; - } - - struct group g; - struct group* pg; + char *buffer = (char*)malloc(bufferLength); + if (buffer == NULL) + return NULL; - errno = 0; - int e; - while ((e = getgrgid_r(gid, &g, buffer, bufferSize, &pg)) == EINTR); + struct group gr; + struct group* result; + if (getgrgid_r(gid, &gr, buffer, bufferLength, &result) == 0) + { + if (result == NULL) + { + errno = ENOENT; + free(buffer); + return NULL; + } + else + { + char* name = strdup(gr.gr_name); + free(buffer); + return name; + } + } - if (e < 0) - { - errno = e; - return -1; + free(buffer); + size_t tmpBufferLength; + if (errno != ERANGE || !multiply_s(bufferLength, (size_t)2, &tmpBufferLength)) + { + return NULL; + } + bufferLength = tmpBufferLength; } - return (int32_t)strlen(buffer); } diff --git a/src/native/libs/System.Native/pal_uid.h b/src/native/libs/System.Native/pal_uid.h index ec3636e3655c2..39b90e8a9ee09 100644 --- a/src/native/libs/System.Native/pal_uid.h +++ b/src/native/libs/System.Native/pal_uid.h @@ -85,14 +85,14 @@ PALEXPORT int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups); /** * Gets the user name associated with the specified user ID and stores it in the buffer. -* On failure, returns -1 and sets errno. -* On success, return the length of the buffer, and the buffer contains the uname. +* On failure, returns a null char pointer and sets errno. +* On success, returns a valid char pointer containing the user name. */ -PALEXPORT int32_t SystemNative_GetUName(uint32_t uid, char* buffer, size_t bufferSize); +PALEXPORT char* SystemNative_GetUName(uint32_t uid); /** * Gets the user name associated with the specified group ID and stores it in the buffer. -* On failure, returns -1 and sets errno. -* On success, return the length of the buffer, and the buffer contains the gname. +* On failure, returns a null char pointer and sets errno. +* On success, returns a valid char pointer containing the group name. */ -PALEXPORT int32_t SystemNative_GetGName(uint32_t gid, char* buffer, size_t bufferSize); +PALEXPORT char* SystemNative_GetGName(uint32_t gid); From a9090b4ec193e88da196d0dc15de10d370b475ff Mon Sep 17 00:00:00 2001 From: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com> Date: Fri, 13 May 2022 12:45:54 -0700 Subject: [PATCH 17/21] Remove unnecessary comment Co-authored-by: Adam Sitnik --- src/libraries/System.Formats.Tar/tests/TarTestsBase.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs index 5808fc78b0ff5..fb0467593d18e 100644 --- a/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs +++ b/src/libraries/System.Formats.Tar/tests/TarTestsBase.cs @@ -100,7 +100,6 @@ protected static MemoryStream GetTarMemoryStream(CompressionMethod compressionMe { string path = GetTarFilePath(compressionMethod, format, testCaseName); MemoryStream ms = new(); - // FileMode.Open, FileAccess.Read and FileShare.Read are default FileStreamOptions using (FileStream fs = File.OpenRead(path)) { fs.CopyTo(ms); From 7c01996f6bd23ee0a6efc400e4e4361308bbbf13 Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Fri, 13 May 2022 15:46:50 -0700 Subject: [PATCH 18/21] Put TargetFrameworks back in the first position of the PropertyGroup. --- src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj index adfd8ace11fea..927df82442c00 100644 --- a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj +++ b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj @@ -1,7 +1,7 @@ - true $(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;$(NetCoreAppCurrent)-Browser;$(NetCoreAppCurrent) + true From 8de50adfa562fbdaec967e0d549d8f338e1d8d98 Mon Sep 17 00:00:00 2001 From: carlossanlop <1175054+carlossanlop@users.noreply.github.com> Date: Fri, 13 May 2022 15:55:04 -0700 Subject: [PATCH 19/21] Address eerhardt suggestions --- ...eUName.cs => Interop.GroupNameUserName.cs} | 20 ++++++------------- .../src/System.Formats.Tar.csproj | 2 +- .../src/System/Formats/Tar/TarWriter.Unix.cs | 4 ++-- .../tests/System.Formats.Tar.Tests.csproj | 2 +- .../TarWriter.WriteEntry.File.Tests.Unix.cs | 7 ++----- src/native/libs/System.Native/entrypoints.c | 4 ++-- .../libs/System.Native/pal_networking.c | 2 +- src/native/libs/System.Native/pal_uid.c | 20 +++++++++---------- src/native/libs/System.Native/pal_uid.h | 12 +++++------ 9 files changed, 31 insertions(+), 42 deletions(-) rename src/libraries/Common/src/Interop/Unix/System.Native/{Interop.GNameUName.cs => Interop.GroupNameUserName.cs} (59%) diff --git a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GroupNameUserName.cs similarity index 59% rename from src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs rename to src/libraries/Common/src/Interop/Unix/System.Native/Interop.GroupNameUserName.cs index c342d229bdf44..742c7a30fa02e 100644 --- a/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GNameUName.cs +++ b/src/libraries/Common/src/Interop/Unix/System.Native/Interop.GroupNameUserName.cs @@ -17,27 +17,19 @@ internal static partial class Sys /// /// The group ID. /// On success, return a string with the group name. On failure, throws an IOException. - internal static string GetGName(uint gid) - { - string result = GetGNameInternal(gid); - return (result == null) ? throw GetIOException(GetLastErrorInfo()) : result; - } + internal static string GetGroupName(uint gid) => GetGroupNameInternal(gid) ?? throw GetIOException(GetLastErrorInfo()); /// /// Gets the user name associated to the specified user ID. /// /// The user ID. /// On success, return a string with the user name. On failure, throws an IOException. - internal static string GetUName(uint uid) - { - string result = GetUNameInternal(uid); - return (result == null) ? throw GetIOException(GetLastErrorInfo()) : result; - } + internal static string GetUserName(uint uid) => GetUserNameInternal(uid) ?? throw GetIOException(GetLastErrorInfo()); - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGName", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] - private static unsafe partial string GetGNameInternal(uint uid); + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetGroupName", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + private static unsafe partial string? GetGroupNameInternal(uint uid); - [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUName", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] - private static unsafe partial string GetUNameInternal(uint uid); + [LibraryImport(Libraries.SystemNative, EntryPoint = "SystemNative_GetUserName", StringMarshalling = StringMarshalling.Utf8, SetLastError = true)] + private static unsafe partial string? GetUserNameInternal(uint uid); } } diff --git a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj index 927df82442c00..3437f384e86c1 100644 --- a/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj +++ b/src/libraries/System.Formats.Tar/src/System.Formats.Tar.csproj @@ -57,7 +57,7 @@ - + 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 index 9612692b19873..8b598f61aa9f7 100644 --- 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 @@ -68,7 +68,7 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str entry._header._uid = (int)status.Uid; if (!_userIdentifiers.TryGetValue(status.Uid, out string? uName)) { - uName = Interop.Sys.GetUName(status.Uid); + uName = Interop.Sys.GetUserName(status.Uid); _userIdentifiers.Add(status.Uid, uName); } entry._header._uName = uName; @@ -77,7 +77,7 @@ partial void ReadFileFromDiskAndWriteToArchiveStreamAsEntry(string fullPath, str entry._header._gid = (int)status.Gid; if (!_groupIdentifiers.TryGetValue(status.Gid, out string? gName)) { - gName = Interop.Sys.GetGName(status.Gid); + gName = Interop.Sys.GetGroupName(status.Gid); _groupIdentifiers.Add(status.Gid, gName); } entry._header._gName = gName; diff --git a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj index 2c1fc2e751d37..c64b271873953 100644 --- a/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj +++ b/src/libraries/System.Formats.Tar/tests/System.Formats.Tar.Tests.csproj @@ -56,7 +56,7 @@ - + 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 74ba09d571e5c..8fef5e7c6a30e 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 @@ -9,7 +9,6 @@ namespace System.Formats.Tar.Tests { public partial class TarWriter_WriteEntry_File_Tests : TarTestsBase { - private static bool IsRemoteExecutorSupportedAndOnUnixAndSuperUser => RemoteExecutor.IsSupported && PlatformDetection.IsUnixAndSuperUser; [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndOnUnixAndSuperUser))] [InlineData(TarFormat.Ustar)] @@ -157,10 +156,8 @@ partial void VerifyPlatformSpecificMetadata(string filePath, TarEntry entry) if (entry is PosixTarEntry posix) { - string gname = Interop.Sys.GetGName(status.Gid); - Assert.NotNull(gname); - string uname = Interop.Sys.GetUName(status.Uid); - Assert.NotNull(uname); + string gname = Interop.Sys.GetGroupName(status.Gid); + string uname = Interop.Sys.GetUserName(status.Uid); Assert.Equal(gname, posix.GroupName); Assert.Equal(uname, posix.UserName); diff --git a/src/native/libs/System.Native/entrypoints.c b/src/native/libs/System.Native/entrypoints.c index 907cd86cd90fa..f2e9e3c1bc11d 100644 --- a/src/native/libs/System.Native/entrypoints.c +++ b/src/native/libs/System.Native/entrypoints.c @@ -268,8 +268,8 @@ static const Entry s_sysNative[] = DllImportEntry(SystemNative_GetEnv) DllImportEntry(SystemNative_GetEnviron) DllImportEntry(SystemNative_FreeEnviron) - DllImportEntry(SystemNative_GetUName) - DllImportEntry(SystemNative_GetGName) + DllImportEntry(SystemNative_GetGroupName) + DllImportEntry(SystemNative_GetUserName) }; EXTERN_C const void* SystemResolveDllImport(const char* name); diff --git a/src/native/libs/System.Native/pal_networking.c b/src/native/libs/System.Native/pal_networking.c index 3169961dbb936..924c6d0cc2533 100644 --- a/src/native/libs/System.Native/pal_networking.c +++ b/src/native/libs/System.Native/pal_networking.c @@ -3031,7 +3031,7 @@ char* SystemNative_GetPeerUserName(intptr_t socket) { uid_t euid; return SystemNative_GetPeerID(socket, &euid) == 0 ? - SystemNative_GetUName(euid) : + SystemNative_GetUserName(euid) : NULL; } diff --git a/src/native/libs/System.Native/pal_uid.c b/src/native/libs/System.Native/pal_uid.c index 452bcf2e9b21b..15a1253b11b5e 100644 --- a/src/native/libs/System.Native/pal_uid.c +++ b/src/native/libs/System.Native/pal_uid.c @@ -239,7 +239,7 @@ int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups) return getgroups(ngroups, groups); } -char* SystemNative_GetUName(uint32_t uid) +char* SystemNative_GetGroupName(uint32_t gid) { size_t bufferLength = 512; while (1) @@ -248,9 +248,9 @@ char* SystemNative_GetUName(uint32_t uid) if (buffer == NULL) return NULL; - struct passwd pw; - struct passwd* result; - if (getpwuid_r(uid, &pw, buffer, bufferLength, &result) == 0) + struct group gr; + struct group* result; + if (getgrgid_r(gid, &gr, buffer, bufferLength, &result) == 0) { if (result == NULL) { @@ -260,7 +260,7 @@ char* SystemNative_GetUName(uint32_t uid) } else { - char* name = strdup(pw.pw_name); + char* name = strdup(gr.gr_name); free(buffer); return name; } @@ -276,7 +276,7 @@ char* SystemNative_GetUName(uint32_t uid) } } -char* SystemNative_GetGName(uint32_t gid) +char* SystemNative_GetUserName(uint32_t uid) { size_t bufferLength = 512; while (1) @@ -285,9 +285,9 @@ char* SystemNative_GetGName(uint32_t gid) if (buffer == NULL) return NULL; - struct group gr; - struct group* result; - if (getgrgid_r(gid, &gr, buffer, bufferLength, &result) == 0) + struct passwd pw; + struct passwd* result; + if (getpwuid_r(uid, &pw, buffer, bufferLength, &result) == 0) { if (result == NULL) { @@ -297,7 +297,7 @@ char* SystemNative_GetGName(uint32_t gid) } else { - char* name = strdup(gr.gr_name); + char* name = strdup(pw.pw_name); free(buffer); return name; } diff --git a/src/native/libs/System.Native/pal_uid.h b/src/native/libs/System.Native/pal_uid.h index 39b90e8a9ee09..920b495d89f54 100644 --- a/src/native/libs/System.Native/pal_uid.h +++ b/src/native/libs/System.Native/pal_uid.h @@ -84,15 +84,15 @@ PALEXPORT int32_t SystemNative_GetGroupList(const char* name, uint32_t group, ui PALEXPORT int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups); /** -* Gets the user name associated with the specified user ID and stores it in the buffer. +* Gets the user name associated with the specified group ID and stores it in the buffer. * On failure, returns a null char pointer and sets errno. -* On success, returns a valid char pointer containing the user name. +* On success, returns a valid char pointer containing the group name. */ -PALEXPORT char* SystemNative_GetUName(uint32_t uid); +PALEXPORT char* SystemNative_GetGroupName(uint32_t gid); /** -* Gets the user name associated with the specified group ID and stores it in the buffer. +* Gets the user name associated with the specified user ID and stores it in the buffer. * On failure, returns a null char pointer and sets errno. -* On success, returns a valid char pointer containing the group name. +* On success, returns a valid char pointer containing the user name. */ -PALEXPORT char* SystemNative_GetGName(uint32_t gid); +PALEXPORT char* SystemNative_GetUserName(uint32_t uid); From 31ab76bf6c660221fcaba488bd5b3b5a3c961ee3 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com> Date: Mon, 16 May 2022 17:36:18 -0700 Subject: [PATCH 20/21] Update src/libraries/System.Formats.Tar/tests/TarWriter/TarWriter.WriteEntry.File.Tests.Unix.cs --- .../tests/TarWriter/TarWriter.WriteEntry.File.Tests.Unix.cs | 1 + 1 file changed, 1 insertion(+) 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 8fef5e7c6a30e..a975535eceb94 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 @@ -9,6 +9,7 @@ namespace System.Formats.Tar.Tests { public partial class TarWriter_WriteEntry_File_Tests : TarTestsBase { + private static bool IsRemoteExecutorSupportedAndOnUnixAndSuperUser => RemoteExecutor.IsSupported && PlatformDetection.IsUnixAndSuperUser; [ConditionalTheory(nameof(IsRemoteExecutorSupportedAndOnUnixAndSuperUser))] [InlineData(TarFormat.Ustar)] From 67fe81266a29a8235e9febfe73d4f17ac61a2d58 Mon Sep 17 00:00:00 2001 From: Carlos Sanchez <1175054+carlossanlop@users.noreply.github.com> Date: Mon, 16 May 2022 17:40:09 -0700 Subject: [PATCH 21/21] Clarify in pal_uid.h methods comments that new memory is returned --- src/native/libs/System.Native/pal_uid.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/native/libs/System.Native/pal_uid.h b/src/native/libs/System.Native/pal_uid.h index 920b495d89f54..2d6dd89fd9b6f 100644 --- a/src/native/libs/System.Native/pal_uid.h +++ b/src/native/libs/System.Native/pal_uid.h @@ -87,6 +87,7 @@ PALEXPORT int32_t SystemNative_GetGroups(int32_t ngroups, uint32_t* groups); * Gets the user name associated with the specified group ID and stores it in the buffer. * On failure, returns a null char pointer and sets errno. * On success, returns a valid char pointer containing the group name. +* Note that this method returns new memory. Consumers can rely on the marshalling behaviour to free the returned string. */ PALEXPORT char* SystemNative_GetGroupName(uint32_t gid); @@ -94,5 +95,6 @@ PALEXPORT char* SystemNative_GetGroupName(uint32_t gid); * Gets the user name associated with the specified user ID and stores it in the buffer. * On failure, returns a null char pointer and sets errno. * On success, returns a valid char pointer containing the user name. +* Note that this method returns new memory. Consumers can rely on the marshalling behaviour to free the returned string. */ PALEXPORT char* SystemNative_GetUserName(uint32_t uid);