Implement Environment.GetEnvironmentVariables for Apple platforms usi…

…ng official API (#58161) (#58254)

Move environment handling from Mono runtime to System.Native

Fixes #58156

Co-authored-by: Filip Navara <[email protected]>
Co-authored-by: Adeel Mujahid <[email protected]>

src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnv.cs

@@ -0,0 +1,14 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.InteropServices;
+
+internal static partial class Interop
+{
+    internal unsafe partial class Sys
+    {
+        [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_GetEnv")]
+        internal static extern unsafe IntPtr GetEnv(string name);
+    }
+}

src/libraries/Common/src/Interop/Unix/System.Native/Interop.GetEnviron.cs

@@ -0,0 +1,17 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System;
+using System.Runtime.InteropServices;
+
+internal static partial class Interop
+{
+    internal unsafe partial class Sys
+    {
+        [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_GetEnviron")]
+        internal static extern unsafe IntPtr GetEnviron();
+
+        [DllImport(Interop.Libraries.SystemNative, EntryPoint = "SystemNative_FreeEnviron")]
+        internal static extern unsafe void FreeEnviron(IntPtr environ);
+    }
+}

src/libraries/Native/Unix/System.Native/CMakeLists.txt

@@ -40,6 +40,12 @@ else()
     list (APPEND NATIVE_SOURCES pal_autoreleasepool.c)
 endif()
 
+if (CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS)
+    list (APPEND NATIVE_SOURCES pal_environment.m)
+else()
+    list (APPEND NATIVE_SOURCES pal_environment.c)
+endif()
+
 if (CLR_CMAKE_TARGET_MACCATALYST OR CLR_CMAKE_TARGET_IOS OR CLR_CMAKE_TARGET_TVOS)
     set(NATIVE_SOURCES ${NATIVE_SOURCES}
         pal_log.m

src/libraries/Native/Unix/System.Native/entrypoints.c

@@ -7,6 +7,7 @@
 #include "pal_autoreleasepool.h"
 #include "pal_console.h"
 #include "pal_datetime.h"
+#include "pal_environment.h"
 #include "pal_errno.h"
 #include "pal_interfaceaddresses.h"
 #include "pal_io.h"
@@ -257,6 +258,9 @@ static const Entry s_sysNative[] =
     DllImportEntry(SystemNative_SetPosixSignalHandler)
     DllImportEntry(SystemNative_GetPlatformSignalNumber)
     DllImportEntry(SystemNative_GetGroups)
+    DllImportEntry(SystemNative_GetEnv)
+    DllImportEntry(SystemNative_GetEnviron)
+    DllImportEntry(SystemNative_FreeEnviron)
 };
 
 EXTERN_C const void* SystemResolveDllImport(const char* name);

src/libraries/Native/Unix/System.Native/pal_environment.c

@@ -0,0 +1,32 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#include "pal_config.h"
+#include "pal_environment.h"
+
+#include <stdlib.h>
+#include <string.h>
+#if HAVE_NSGETENVIRON
+#include <crt_externs.h>
+#endif
+
+char* SystemNative_GetEnv(const char* variable)
+{
+    return getenv(variable);
+}
+
+char** SystemNative_GetEnviron()
+{
+#if HAVE_NSGETENVIRON
+    return *(_NSGetEnviron());
+#else
+    extern char **environ;
+    return environ;
+#endif
+}
+
+void SystemNative_FreeEnviron(char** environ)
+{
+    // no op
+    (void)environ;
+}

src/libraries/Native/Unix/System.Native/pal_environment.h

@@ -0,0 +1,13 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#pragma once
+
+#include "pal_compiler.h"
+#include "pal_types.h"
+
+PALEXPORT char* SystemNative_GetEnv(const char* variable);
+
+PALEXPORT char** SystemNative_GetEnviron(void);
+
+PALEXPORT void SystemNative_FreeEnviron(char** environ);

src/libraries/Native/Unix/System.Native/pal_environment.m

@@ -0,0 +1,78 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+#include "pal_config.h"
+#include "pal_environment.h"
+
+#include <Foundation/Foundation.h>
+#include <CoreFoundation/CoreFoundation.h>
+#include <objc/runtime.h>
+#include <objc/message.h>
+
+char* SystemNative_GetEnv(const char* variable)
+{
+    return getenv(variable);
+}
+
+static char *empty_key_value_pair = "=";
+
+static void get_environ_helper(const void *key, const void *value, void *context)
+{
+    char ***temp_environ_ptr = (char***)context;
+    const char *utf8_key = [(NSString *)key UTF8String];
+    const char *utf8_value = [(NSString *)value UTF8String];
+    int utf8_key_length = strlen(utf8_key);
+    int utf8_value_length = strlen(utf8_value);
+    char *key_value_pair;
+
+    key_value_pair = malloc(utf8_key_length + utf8_value_length + 2);
+    if (key_value_pair != NULL)
+    {
+        strcpy(key_value_pair, utf8_key);
+        key_value_pair[utf8_key_length] = '=';
+        strcpy(key_value_pair + utf8_key_length + 1, utf8_value);
+    }
+    else
+    {
+        // In case of failed allocation add pointer to preallocated entry. This is
+        // ignored on the managed side and skipped over in SystemNative_FreeEnviron.
+        key_value_pair = empty_key_value_pair;
+    }
+
+    **temp_environ_ptr = key_value_pair;
+    (*temp_environ_ptr)++;
+}
+
+char** SystemNative_GetEnviron()
+{
+    char **temp_environ;
+    char **temp_environ_ptr;
+
+    CFDictionaryRef environment = (CFDictionaryRef)[[NSProcessInfo processInfo] environment];
+    int count = CFDictionaryGetCount(environment);
+    temp_environ = (char **)malloc((count + 1) * sizeof(char *));
+    if (temp_environ != NULL)
+    {
+        temp_environ_ptr = temp_environ;
+        CFDictionaryApplyFunction(environment, get_environ_helper, &temp_environ_ptr);
+        *temp_environ_ptr = NULL;
+    }
+
+    return temp_environ;
+}
+
+void SystemNative_FreeEnviron(char** environ)
+{
+    if (environ != NULL)
+    {
+        for (char** environ_ptr = environ; *environ_ptr != NULL; environ_ptr++)
+        {
+            if (*environ_ptr != empty_key_value_pair)
+            {
+                free(*environ_ptr);
+            }
+        }
+
+        free(environ);
+    }
+}

src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj

@@ -265,6 +265,12 @@
   <ItemGroup Condition="'$(TargetsUnix)' == 'true' or '$(TargetsBrowser)' == 'true'">
       <Compile Include="$(BclSourcesRoot)\System\Environment.Unix.Mono.cs" />
       <Compile Include="$(BclSourcesRoot)\System\Threading\LowLevelLifoSemaphore.Unix.Mono.cs" />
+      <Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.GetEnv.cs">
+        <Link>Common\Interop\Unix\System.Native\Interop.GetEnv.cs</Link>
+      </Compile>
+      <Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.GetEnviron.cs">
+        <Link>Common\Interop\Unix\System.Native\Interop.GetEnviron.cs</Link>
+      </Compile>
   </ItemGroup>
   <ItemGroup Condition="'$(TargetsMacCatalyst)' == 'true' or '$(TargetsiOS)' == 'true' or '$(TargetstvOS)' == 'true'">
       <Compile Include="$(BclSourcesRoot)\System\Environment.iOS.cs" />

src/mono/System.Private.CoreLib/src/System/Environment.Unix.Mono.cs

@@ -5,8 +5,8 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
 using System.Threading;
-using Mono;
 
 namespace System
 {
@@ -20,7 +20,7 @@ public partial class Environment
 
             if (s_environment == null)
             {
-                return InternalGetEnvironmentVariable(variable);
+                return Marshal.PtrToStringAnsi(Interop.Sys.GetEnv(variable));
             }
 
             variable = TrimStringOnFirstZero(variable);
@@ -31,23 +31,15 @@ public partial class Environment
             }
         }
 
-        private static string InternalGetEnvironmentVariable(string name)
-        {
-            using (SafeStringMarshal handle = RuntimeMarshal.MarshalString(name))
-            {
-                return internalGetEnvironmentVariable_native(handle.Value);
-            }
-        }
-
         private static unsafe void SetEnvironmentVariableCore(string variable, string? value)
         {
             Debug.Assert(variable != null);
 
             EnsureEnvironmentCached();
+            variable = TrimStringOnFirstZero(variable);
+            value = value == null ? null : TrimStringOnFirstZero(value);
             lock (s_environment!)
             {
-                variable = TrimStringOnFirstZero(variable);
-                value = value == null ? null : TrimStringOnFirstZero(value);
                 if (string.IsNullOrEmpty(value))
                 {
                     s_environment.Remove(variable);
@@ -97,22 +89,75 @@ private static Dictionary<string, string> GetSystemEnvironmentVariables()
         {
             var results = new Dictionary<string, string>();
 
-            foreach (string name in GetEnvironmentVariableNames())
+            IntPtr block = Interop.Sys.GetEnviron();
+            if (block != IntPtr.Zero)
             {
-                if (name != null)
+                try
+                {
+                    IntPtr blockIterator = block;
+
+                    // Per man page, environment variables come back as an array of pointers to strings
+                    // Parse each pointer of strings individually
+                    while (ParseEntry(blockIterator, out string? key, out string? value))
+                    {
+                        if (key != null && value != null)
+                        {
+                            try
+                            {
+                                // Add may throw if the environment block was corrupted leading to duplicate entries.
+                                // We allow such throws and eat them (rather than proactively checking for duplication)
+                                // to provide a non-fatal notification about the corruption.
+                                results.Add(key, value);
+                            }
+                            catch (ArgumentException) { }
+                        }
+
+                        // Increment to next environment variable entry
+                        blockIterator += IntPtr.Size;
+                    }
+                }
+                finally
                 {
-                    results.Add(name, InternalGetEnvironmentVariable(name));
+                    Interop.Sys.FreeEnviron(block);
                 }
             }
 
             return results;
-        }
-
-
-        [MethodImplAttribute(MethodImplOptions.InternalCall)]
-        private static extern string internalGetEnvironmentVariable_native(IntPtr variable);
 
-        [MethodImplAttribute(MethodImplOptions.InternalCall)]
-        private static extern string[] GetEnvironmentVariableNames();
+            // Use a local, unsafe function since we cannot use `yield return` inside of an `unsafe` block
+            static unsafe bool ParseEntry(IntPtr current, out string? key, out string? value)
+            {
+                // Setup
+                key = null;
+                value = null;
+
+                // Point to current entry
+                byte* entry = *(byte**)current;
+
+                // Per man page, "The last pointer in this array has the value NULL"
+                // Therefore, if entry is null then we're at the end and can bail
+                if (entry == null)
+                    return false;
+
+                // Parse each byte of the entry until we hit either the separator '=' or '\0'.
+                // This finds the split point for creating key/value strings below.
+                // On some old OS, the environment block can be corrupted.
+                // Some will not have '=', so we need to check for '\0'.
+                byte* splitpoint = entry;
+                while (*splitpoint != '=' && *splitpoint != '\0')
+                    splitpoint++;
+
+                // Skip over entries starting with '=' and entries with no value (just a null-terminating char '\0')
+                if (splitpoint == entry || *splitpoint == '\0')
+                    return true;
+
+                // The key is the bytes from start (0) until our splitpoint
+                key = new string((sbyte*)entry, 0, checked((int)(splitpoint - entry)));
+                // The value is the rest of the bytes starting after the splitpoint
+                value = new string((sbyte*)(splitpoint + 1));
+
+                return true;
+            }
+        }
     }
 }

src/mono/System.Private.CoreLib/src/System/Environment.iOS.cs

@@ -84,7 +84,7 @@ private static string GetFolderPathCore(SpecialFolder folder, SpecialFolderOptio
                     return Interop.Sys.SearchPath(NSSearchPathDirectory.NSCachesDirectory);
 
                 case SpecialFolder.UserProfile:
-                    return InternalGetEnvironmentVariable("HOME");
+                    return GetEnvironmentVariable("HOME");
 
                 case SpecialFolder.CommonApplicationData:
                     return "/usr/share";

src/mono/mono/metadata/icall-def-netcore.h

@@ -98,12 +98,10 @@ ICALL_TYPE(ENV, "System.Environment", ENV_1)
 NOHANDLES(ICALL(ENV_1, "Exit", ves_icall_System_Environment_Exit))
 HANDLES(ENV_1a, "FailFast", ves_icall_System_Environment_FailFast, void, 3, (MonoString, MonoException, MonoString))
 HANDLES(ENV_2, "GetCommandLineArgs", ves_icall_System_Environment_GetCommandLineArgs, MonoArray, 0, ())
-HANDLES(ENV_3, "GetEnvironmentVariableNames", ves_icall_System_Environment_GetEnvironmentVariableNames, MonoArray, 0, ())
 NOHANDLES(ICALL(ENV_4, "GetProcessorCount", ves_icall_System_Environment_get_ProcessorCount))
 NOHANDLES(ICALL(ENV_9, "get_ExitCode", mono_environment_exitcode_get))
 NOHANDLES(ICALL(ENV_15, "get_TickCount", ves_icall_System_Environment_get_TickCount))
 NOHANDLES(ICALL(ENV_15a, "get_TickCount64", ves_icall_System_Environment_get_TickCount64))
-HANDLES(ENV_17, "internalGetEnvironmentVariable_native", ves_icall_System_Environment_GetEnvironmentVariable_native, MonoString, 1, (const_char_ptr))
 NOHANDLES(ICALL(ENV_20, "set_ExitCode", mono_environment_exitcode_set))
 
 ICALL_TYPE(GC, "System.GC", GC_13)

src/mono/mono/metadata/icall-internals.h

@@ -35,12 +35,6 @@ mono_icall_get_new_line (MonoError *error);
 MonoBoolean
 mono_icall_is_64bit_os (void);
 
-MonoArrayHandle
-mono_icall_get_environment_variable_names (MonoError *error);
-
-void
-mono_icall_set_environment_variable (MonoString *name, MonoString *value);
-
 MonoStringHandle
 mono_icall_get_windows_folder_path (int folder, MonoError *error);