Skip to content
Open
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 87 additions & 0 deletions src/NodeApi/Runtime/NativeLibrary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#if !NET7_0_OR_GREATER

using System;
using System.IO;
using System.Reflection;
using System.Runtime.InteropServices;
#if !NETFRAMEWORK
using SysNativeLibrary = System.Runtime.InteropServices.NativeLibrary;
Expand Down Expand Up @@ -50,6 +52,25 @@ public static nint Load(string libraryName)
#endif
}

/// <summary>
/// Loads a native library using the high-level API.
/// </summary>
/// <param name="libraryName">The name of the native library to be loaded.</param>
/// <param name="assembly">The assembly loading the native library.</param>
/// <param name="searchPath">The search path.</param>
/// <returns>The OS handle for the loaded native library.</returns>
public static nint Load(string libraryName, Assembly assembly, DllImportSearchPath? searchPath)
{
#if NETFRAMEWORK
string libraryPath = FindLibrary(libraryName, assembly)
?? throw new ArgumentNullException(nameof(libraryName));

return LoadLibrary(libraryPath);
#else
return SysNativeLibrary.Load(libraryName, assembly, searchPath);
#endif
}

/// <summary>
/// Gets the address of an exported symbol.
/// </summary>
Expand All @@ -75,6 +96,72 @@ public static bool TryGetExport(nint handle, string name, out nint procAddress)
#endif
}

/// <summary>
/// Searches various well-known paths for a library and returns the first result.
/// </summary>
/// <param name="libraryName">Name of the library to search for.</param>
/// <param name="assembly">Assembly to search relative from.</param>
/// <returns>Library path if found, otherwise false.</returns>
private static string? FindLibrary(string libraryName, Assembly assembly)
{
if (File.Exists(libraryName))
Copy link
Member

Choose a reason for hiding this comment

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

If libraryName is not an absolute path this searches the current directory (or a relative path from it), which is unsafe.

Copy link
Author

Choose a reason for hiding this comment

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

Added a check to ensure it's an absolute path.

{
return Path.GetFullPath(libraryName);
}

string[] tryLibraryNames;

if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
{
tryLibraryNames =
[
libraryName,
$"{libraryName}.dll"
];
}
else
{
string libraryExtension = RuntimeInformation.IsOSPlatform(OSPlatform.OSX)
? "dylib"
: "so";

tryLibraryNames =
[
libraryName,
$"lib{libraryName}",
$"{libraryName}.{libraryExtension}",
$"lib{libraryName}.{libraryExtension}"
];
}
Copy link
Member

Choose a reason for hiding this comment

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

This else block is not needed. This method is only used with .NET Framework 4.x, which only runs on Windows, so there is no need to consider other operating systems.

Copy link
Author

Choose a reason for hiding this comment

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

I was thinking about Mono here - not sure if it's a target you might want to support though.

Copy link
Member

Choose a reason for hiding this comment

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

We're not attempting to support Mono. There are several places in code already that assume anything within #if NETFRAMEWORK is Windows-only.


string?[] tryDirectories =
[
Path.GetDirectoryName(assembly.Location),
Environment.CurrentDirectory,
Copy link
Member

Choose a reason for hiding this comment

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

Do not search the current directory; that is very unsafe.

Copy link
Author

Choose a reason for hiding this comment

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

Removed.

Environment.SystemDirectory,
];

foreach (string? tryDirectory in tryDirectories)
{
if (tryDirectory == null)
{
continue;
}

foreach (string tryLibraryName in tryLibraryNames)
{
string tryLibraryPath = Path.Combine(tryDirectory, tryLibraryName);

if (File.Exists(tryLibraryPath))
{
return tryLibraryPath;
}
}
}

return null;
}

#pragma warning disable CA2101 // Specify marshaling for P/Invoke string arguments

[DllImport("kernel32")]
Expand Down
18 changes: 13 additions & 5 deletions src/NodeApi/Runtime/NodejsPlatform.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System;
using System.Reflection;
using System.Runtime.InteropServices;

namespace Microsoft.JavaScript.NodeApi.Runtime;
Expand All @@ -25,23 +26,30 @@ public sealed class NodejsPlatform : IDisposable
/// <summary>
/// Initializes the Node.js platform.
/// </summary>
/// <param name="libnodePath">Path to the `libnode` shared library, including extension.</param>
/// <param name="libnode">
/// Name of the `libnode` shared library.
/// Has to be a full file path when using .NET Framework.
/// </param>
/// <param name="args">Optional platform arguments.</param>
/// <exception cref="InvalidOperationException">A Node.js platform instance has already been
/// loaded in the current process.</exception>
public NodejsPlatform(
string libnodePath,
string libnode,
string[]? args = null)
{
if (string.IsNullOrEmpty(libnodePath)) throw new ArgumentNullException(nameof(libnodePath));

if (Current != null)
{
throw new InvalidOperationException(
"Only one Node.js platform instance per process is allowed.");
}

nint libnodeHandle = NativeLibrary.Load(libnodePath);
var entryAssembly = Assembly.GetAssembly(typeof(NodejsPlatform));

nint libnodeHandle =
entryAssembly != null
? NativeLibrary.Load(libnode, entryAssembly!, null)
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to specify DllImportSearchPath.SafeDirectories here, because the default search behavior is considered unsafe.

If your goal was to load libnode from the same directory as the application, I think that will work, though the documentation for SafeDirectories is not entirely clear.

In that case the back-compat code in NativeLibrary should search the application directory, which is not necessarily the same as the assembly directory.

Copy link
Author

Choose a reason for hiding this comment

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

SafeDirectories won't use the assembly directory, so I'm ORing SafeDirectories and AssemblyDirectory now.

: NativeLibrary.Load(libnode);

Runtime = new NodejsRuntime(libnodeHandle);

Runtime.CreatePlatform(args, (error) => Console.WriteLine(error), out _platform)
Expand Down