diff --git a/src/NodeApi/JSError.cs b/src/NodeApi/JSError.cs index 33437ae9..74cf119f 100644 --- a/src/NodeApi/JSError.cs +++ b/src/NodeApi/JSError.cs @@ -290,22 +290,22 @@ public static void ThrowError(string code, string message) => JSValue.GetCurrentRuntime(out napi_env env).ThrowError(env, code, message); public static void ThrowTypeError(string message) - => JSValue.GetCurrentRuntime(out napi_env env).ThrowError(env, code: null, message); + => JSValue.GetCurrentRuntime(out napi_env env).ThrowTypeError(env, code: null, message); public static void ThrowTypeError(string code, string message) - => JSValue.GetCurrentRuntime(out napi_env env).ThrowError(env, code, message); + => JSValue.GetCurrentRuntime(out napi_env env).ThrowTypeError(env, code, message); public static void ThrowRangeError(string message) - => JSValue.GetCurrentRuntime(out napi_env env).ThrowError(env, code: null, message); + => JSValue.GetCurrentRuntime(out napi_env env).ThrowRangeError(env, code: null, message); public static void ThrowRangeError(string code, string message) - => JSValue.GetCurrentRuntime(out napi_env env).ThrowError(env, code, message); + => JSValue.GetCurrentRuntime(out napi_env env).ThrowRangeError(env, code, message); public static void ThrowSyntaxError(string message) - => JSValue.GetCurrentRuntime(out napi_env env).ThrowError(env, code: null, message); + => JSValue.GetCurrentRuntime(out napi_env env).ThrowSyntaxError(env, code: null, message); public static void ThrowSyntaxError(string code, string message) - => JSValue.GetCurrentRuntime(out napi_env env).ThrowError(env, code, message); + => JSValue.GetCurrentRuntime(out napi_env env).ThrowSyntaxError(env, code, message); /// /// Gets a JS error stack trace that also includes a .NET stack trace, diff --git a/src/NodeApi/Runtime/NodejsRuntime.Embedding.cs b/src/NodeApi/Runtime/NodejsRuntime.Embedding.cs index 4c3c5da7..4ff15168 100644 --- a/src/NodeApi/Runtime/NodejsRuntime.Embedding.cs +++ b/src/NodeApi/Runtime/NodejsRuntime.Embedding.cs @@ -681,7 +681,7 @@ public override NodeEmbeddingStatus EmbeddingRuntimeConfigAddModule( node_embedding_data_release_callback release_init_module_data, int module_node_api_version) { - PooledBuffer moduleNameBuffer = PooledBuffer.FromSpanUtf8(module_name); + using PooledBuffer moduleNameBuffer = PooledBuffer.FromSpanUtf8(module_name); fixed (byte* moduleNamePtr = moduleNameBuffer) return Import(ref node_embedding_runtime_config_add_module)( runtime_config, diff --git a/src/NodeApi/Runtime/PooledBuffer.cs b/src/NodeApi/Runtime/PooledBuffer.cs index 597f1235..0e8328ed 100644 --- a/src/NodeApi/Runtime/PooledBuffer.cs +++ b/src/NodeApi/Runtime/PooledBuffer.cs @@ -68,17 +68,21 @@ public void Dispose() [EditorBrowsable(EditorBrowsableState.Never)] public readonly ref byte GetPinnableReference() => ref Span.GetPinnableReference(); + // Native callers may read these buffers as null-terminated C strings. A null string + // produces an empty buffer (null pointer); an empty string produces just the terminator. public static unsafe PooledBuffer FromStringUtf8(string? value) { - if (string.IsNullOrEmpty(value)) + if (value == null) { return Empty; } int byteLength = Encoding.UTF8.GetByteCount(value); PooledBuffer buffer = new(byteLength, byteLength + 1); - Encoding.UTF8.GetBytes(value, 0, value!.Length, buffer.Buffer, 0); + Encoding.UTF8.GetBytes(value, 0, value.Length, buffer.Buffer, 0); + // Pooled arrays are not zeroed. + buffer.Buffer[byteLength] = 0; return buffer; } @@ -86,7 +90,9 @@ public static unsafe PooledBuffer FromSpanUtf8(ReadOnlySpan value) { if (value.IsEmpty) { - return Empty; + PooledBuffer emptyString = new(0, 1); + emptyString.Buffer[0] = 0; + return emptyString; } fixed (char* valuePtr = value) @@ -94,7 +100,10 @@ public static unsafe PooledBuffer FromSpanUtf8(ReadOnlySpan value) int byteLength = Encoding.UTF8.GetByteCount(valuePtr, value.Length); PooledBuffer buffer = new(byteLength, byteLength + 1); fixed (byte* bufferPtr = buffer.Span) - Encoding.UTF8.GetBytes(valuePtr, value.Length, bufferPtr, byteLength + 1); + Encoding.UTF8.GetBytes(valuePtr, value.Length, bufferPtr, byteLength); + + // Pooled arrays are not zeroed. + buffer.Buffer[byteLength] = 0; return buffer; } } diff --git a/src/NodeApi/Runtime/Utf8StringArray.cs b/src/NodeApi/Runtime/Utf8StringArray.cs index f3b7b36c..04d0ad60 100644 --- a/src/NodeApi/Runtime/Utf8StringArray.cs +++ b/src/NodeApi/Runtime/Utf8StringArray.cs @@ -47,8 +47,10 @@ public unsafe Utf8StringArray(ReadOnlySpan strings) { Utf8Strings[i] = stringBufferPtr + offset; offset += Encoding.UTF8.GetBytes( - src, strings[i].Length, (byte*)(stringBufferPtr + offset), byteLength - offset) - + 1; // +1 for the string Null-terminator. + src, strings[i].Length, (byte*)(stringBufferPtr + offset), byteLength - offset); + + // Pooled arrays are not zeroed. + _stringBuffer[offset++] = 0; } } } diff --git a/test/NodejsEmbeddingTests.cs b/test/NodejsEmbeddingTests.cs index 6e4191c8..cc5fbc78 100644 --- a/test/NodejsEmbeddingTests.cs +++ b/test/NodejsEmbeddingTests.cs @@ -260,6 +260,89 @@ public void ErrorPropagation() $"at {typeof(NodejsEmbeddingTests).FullName}.", StringComparison.Ordinal)); } + [Theory] + [InlineData("x")] + [InlineData("")] + public void ThrowErrorFromCallback(string message) + { + using NodeEmbeddingThreadRuntime nodejs = CreateNodeEmbeddingThreadRuntime(); + + nodejs.Run(() => + { + JSValue.Global.SetProperty("throwError", JSValue.CreateFunction("throwError", (args) => + { + // Rented buffers that are not zeroed must not leak into the error message. + FillArrayPoolWithGarbage(); + JSError.ThrowError((string)args[0]); + return JSValue.Undefined; + })); + JSValue.Global.SetProperty("message", message); + + JSValue result = JSValue.RunScript(""" + (() => { + try { throwError(message); } catch (e) { return e.message; } + return 'not thrown'; + })() + """); + Assert.Equal(message, (string)result); + }); + } + + [Theory] + [InlineData("Error", null)] + [InlineData("Error", "ERR_TEST")] + [InlineData("TypeError", null)] + [InlineData("TypeError", "ERR_TEST")] + [InlineData("RangeError", null)] + [InlineData("RangeError", "ERR_TEST")] + [InlineData("SyntaxError", null)] + [InlineData("SyntaxError", "ERR_TEST")] + public void ThrowTypedError(string errorType, string? code) + { + using NodeEmbeddingThreadRuntime nodejs = CreateNodeEmbeddingThreadRuntime(); + + nodejs.Run(() => + { + JSValue.Global.SetProperty("throwError", JSValue.CreateFunction("throwError", (_) => + { + Action throwWithoutCode = errorType switch + { + "Error" => JSError.ThrowError, + "TypeError" => JSError.ThrowTypeError, + "RangeError" => JSError.ThrowRangeError, + "SyntaxError" => JSError.ThrowSyntaxError, + _ => throw new ArgumentOutOfRangeException(nameof(errorType)), + }; + Action throwWithCode = errorType switch + { + "Error" => JSError.ThrowError, + "TypeError" => JSError.ThrowTypeError, + "RangeError" => JSError.ThrowRangeError, + "SyntaxError" => JSError.ThrowSyntaxError, + _ => throw new ArgumentOutOfRangeException(nameof(errorType)), + }; + + if (code == null) throwWithoutCode("test"); + else throwWithCode(code, "test"); + return JSValue.Undefined; + })); + + JSValue error = JSValue.RunScript(""" + (() => { + try { throwError(); } catch (e) { return e; } + return null; + })() + """); + Assert.True(error.IsObject()); + Assert.Equal(errorType, (string)error["name"]); + Assert.True((bool)JSValue.RunScript( + $"(e) => e instanceof {errorType}").Call(JSValue.Undefined, error)); + Assert.Equal("test", (string)error["message"]); + if (code == null) Assert.True(error["code"].IsUndefined()); + else Assert.Equal(code, (string)error["code"]); + }); + } + [Fact] public async Task WorkerIsMainThread() { diff --git a/test/TestUtils.cs b/test/TestUtils.cs index b52c09cf..3ca96420 100644 --- a/test/TestUtils.cs +++ b/test/TestUtils.cs @@ -2,6 +2,10 @@ // Licensed under the MIT License. using System; +#if !NETFRAMEWORK +using System.Buffers; +#endif +using System.Collections.Generic; using System.Diagnostics; using System.IO; using System.Runtime.InteropServices; @@ -142,6 +146,31 @@ public static void CopyIfNewer(string sourceFilePath, string targetFilePath) } } + /// + /// Fills arrays in the shared byte array pool (as seen by the calling thread) with non-zero + /// bytes, so that code relying on rented buffers being zeroed fails deterministically. + /// + public static void FillArrayPoolWithGarbage() + { +#if !NETFRAMEWORK + List rented = new(); + for (int size = 1; size <= 1024; size *= 2) + { + for (int i = 0; i < 8; i++) + { + byte[] array = ArrayPool.Shared.Rent(size); + array.AsSpan().Fill(0xAA); + rented.Add(array); + } + } + + foreach (byte[] array in rented) + { + ArrayPool.Shared.Return(array); + } +#endif + } + public static Task RunInThread(Action action) { TaskCompletionSource threadCompletion = new(); diff --git a/test/Utf8BufferTests.cs b/test/Utf8BufferTests.cs new file mode 100644 index 00000000..5be9636d --- /dev/null +++ b/test/Utf8BufferTests.cs @@ -0,0 +1,76 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +using System; +using System.Runtime.InteropServices; +using System.Text; +using Microsoft.JavaScript.NodeApi.Runtime; +using Xunit; +using static Microsoft.JavaScript.NodeApi.Test.TestUtils; + +namespace Microsoft.JavaScript.NodeApi.Test; + +public class Utf8BufferTests +{ + public static TheoryData Strings { get; } = new() + { + "", + "x", + "module-name", + "\u00e4\u00f6\u00fc \u4e2d\u6587 \U0001F600", + }; + + [Theory] + [MemberData(nameof(Strings))] + public void FromStringUtf8IsNullTerminated(string value) + { + FillArrayPoolWithGarbage(); + using PooledBuffer buffer = PooledBuffer.FromStringUtf8(value); + AssertNullTerminatedUtf8(value, buffer); + } + + [Theory] + [MemberData(nameof(Strings))] + public void FromSpanUtf8IsNullTerminated(string value) + { + FillArrayPoolWithGarbage(); + using PooledBuffer buffer = PooledBuffer.FromSpanUtf8(value.AsSpan()); + AssertNullTerminatedUtf8(value, buffer); + } + + [Fact] + public void FromStringUtf8NullIsEmpty() + { + // A null string must still produce a null pointer when pinned. + using PooledBuffer buffer = PooledBuffer.FromStringUtf8(null); + Assert.Equal(0, buffer.Length); + Assert.True(buffer.Span.IsEmpty); + } + +#if !NETFRAMEWORK + [Fact] + public void Utf8StringArrayIsNullTerminated() + { + string[] strings = ["node", "", "--expose-gc", "\u00e4\u00f6\u00fc \U0001F600"]; + + FillArrayPoolWithGarbage(); + using Utf8StringArray utf8Strings = new(strings); + + for (int i = 0; i < strings.Length; i++) + { + Assert.Equal(strings[i], Marshal.PtrToStringUTF8(utf8Strings.Utf8Strings[i])); + } + } +#endif + + private static void AssertNullTerminatedUtf8(string expected, PooledBuffer buffer) + { + int byteLength = Encoding.UTF8.GetByteCount(expected); + Assert.Equal(byteLength, buffer.Length); + + // The buffer must hold the terminator so that pinning it never yields a null pointer. + Assert.True(buffer.Span.Length > byteLength); + Assert.Equal(0, buffer.Span[byteLength]); + Assert.Equal(expected, Encoding.UTF8.GetString(buffer.Buffer, 0, byteLength)); + } +}