From 346e81b7a3f46a56c6b79491220a764070565410 Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Mon, 27 Jan 2025 15:41:44 +0000 Subject: [PATCH 1/2] test: convert test_encoding_binding.cc to a JS test The cctest file `test_encoding_binding.cc` is never tested and it is not a valid test. Binding functions should never be tested with V8 API circumvented. A binding function should only be tested with JS calls. --- test/cctest/test_encoding_binding.cc | 176 ------------------ .../test-internal-encoding-binding.js | 48 +++++ 2 files changed, 48 insertions(+), 176 deletions(-) delete mode 100644 test/cctest/test_encoding_binding.cc create mode 100644 test/parallel/test-internal-encoding-binding.js diff --git a/test/cctest/test_encoding_binding.cc b/test/cctest/test_encoding_binding.cc deleted file mode 100644 index d5d14c60fedf7e..00000000000000 --- a/test/cctest/test_encoding_binding.cc +++ /dev/null @@ -1,176 +0,0 @@ -#include "encoding_binding.h" -#include "env-inl.h" -#include "gtest/gtest.h" -#include "node_test_fixture.h" -#include "v8.h" - -namespace node { -namespace encoding_binding { - -bool RunDecodeLatin1(Environment* env, - Local args[], - bool ignore_bom, - bool has_fatal, - Local* result) { - Isolate* isolate = env->isolate(); - TryCatch try_catch(isolate); - - Local ignoreBOMValue = Boolean::New(isolate, ignore_bom); - Local fatalValue = Boolean::New(isolate, has_fatal); - - Local updatedArgs[] = {args[0], ignoreBOMValue, fatalValue}; - - BindingData::DecodeLatin1(FunctionCallbackInfo(updatedArgs)); - - if (try_catch.HasCaught()) { - return false; - } - - *result = args[0]; - return true; -} - -class EncodingBindingTest : public NodeTestFixture {}; - -TEST_F(EncodingBindingTest, DecodeLatin1_ValidInput) { - Environment* env = CreateEnvironment(); - Isolate* isolate = env->isolate(); - HandleScope handle_scope(isolate); - - const uint8_t latin1_data[] = {0xC1, 0xE9, 0xF3}; - Local ab = ArrayBuffer::New(isolate, sizeof(latin1_data)); - memcpy(ab->GetBackingStore()->Data(), latin1_data, sizeof(latin1_data)); - - Local array = Uint8Array::New(ab, 0, sizeof(latin1_data)); - Local args[] = {array}; - - Local result; - EXPECT_TRUE(RunDecodeLatin1(env, args, false, false, &result)); - - String::Utf8Value utf8_result(isolate, result); - EXPECT_STREQ(*utf8_result, "Áéó"); -} - -TEST_F(EncodingBindingTest, DecodeLatin1_EmptyInput) { - Environment* env = CreateEnvironment(); - Isolate* isolate = env->isolate(); - HandleScope handle_scope(isolate); - - Local ab = ArrayBuffer::New(isolate, 0); - Local array = Uint8Array::New(ab, 0, 0); - Local args[] = {array}; - - Local result; - EXPECT_TRUE(RunDecodeLatin1(env, args, false, false, &result)); - - String::Utf8Value utf8_result(isolate, result); - EXPECT_STREQ(*utf8_result, ""); -} - -TEST_F(EncodingBindingTest, DecodeLatin1_InvalidInput) { - Environment* env = CreateEnvironment(); - Isolate* isolate = env->isolate(); - HandleScope handle_scope(isolate); - - Local args[] = {String::NewFromUtf8Literal(isolate, "Invalid input")}; - - Local result; - EXPECT_FALSE(RunDecodeLatin1(env, args, false, false, &result)); -} - -TEST_F(EncodingBindingTest, DecodeLatin1_IgnoreBOM) { - Environment* env = CreateEnvironment(); - Isolate* isolate = env->isolate(); - HandleScope handle_scope(isolate); - - const uint8_t latin1_data[] = {0xFE, 0xFF, 0xC1, 0xE9, 0xF3}; - Local ab = ArrayBuffer::New(isolate, sizeof(latin1_data)); - memcpy(ab->GetBackingStore()->Data(), latin1_data, sizeof(latin1_data)); - - Local array = Uint8Array::New(ab, 0, sizeof(latin1_data)); - Local args[] = {array}; - - Local result; - EXPECT_TRUE(RunDecodeLatin1(env, args, true, false, &result)); - - String::Utf8Value utf8_result(isolate, result); - EXPECT_STREQ(*utf8_result, "Áéó"); -} - -TEST_F(EncodingBindingTest, DecodeLatin1_FatalInvalidInput) { - Environment* env = CreateEnvironment(); - Isolate* isolate = env->isolate(); - HandleScope handle_scope(isolate); - - const uint8_t invalid_data[] = {0xFF, 0xFF, 0xFF}; - Local ab = ArrayBuffer::New(isolate, sizeof(invalid_data)); - memcpy(ab->GetBackingStore()->Data(), invalid_data, sizeof(invalid_data)); - - Local array = Uint8Array::New(ab, 0, sizeof(invalid_data)); - Local args[] = {array}; - - Local result; - EXPECT_FALSE(RunDecodeLatin1(env, args, false, true, &result)); -} - -TEST_F(EncodingBindingTest, DecodeLatin1_IgnoreBOMAndFatal) { - Environment* env = CreateEnvironment(); - Isolate* isolate = env->isolate(); - HandleScope handle_scope(isolate); - - const uint8_t latin1_data[] = {0xFE, 0xFF, 0xC1, 0xE9, 0xF3}; - Local ab = ArrayBuffer::New(isolate, sizeof(latin1_data)); - memcpy(ab->GetBackingStore()->Data(), latin1_data, sizeof(latin1_data)); - - Local array = Uint8Array::New(ab, 0, sizeof(latin1_data)); - Local args[] = {array}; - - Local result; - EXPECT_TRUE(RunDecodeLatin1(env, args, true, true, &result)); - - String::Utf8Value utf8_result(isolate, result); - EXPECT_STREQ(*utf8_result, "Áéó"); -} - -TEST_F(EncodingBindingTest, DecodeLatin1_BOMPresent) { - Environment* env = CreateEnvironment(); - Isolate* isolate = env->isolate(); - HandleScope handle_scope(isolate); - - const uint8_t latin1_data[] = {0xFF, 0xC1, 0xE9, 0xF3}; - Local ab = ArrayBuffer::New(isolate, sizeof(latin1_data)); - memcpy(ab->GetBackingStore()->Data(), latin1_data, sizeof(latin1_data)); - - Local array = Uint8Array::New(ab, 0, sizeof(latin1_data)); - Local args[] = {array}; - - Local result; - EXPECT_TRUE(RunDecodeLatin1(env, args, true, false, &result)); - - String::Utf8Value utf8_result(isolate, result); - EXPECT_STREQ(*utf8_result, "Áéó"); -} - -TEST_F(EncodingBindingTest, DecodeLatin1_ReturnsString) { - Environment* env = CreateEnvironment(); - Isolate* isolate = env->isolate(); - HandleScope handle_scope(isolate); - - const uint8_t latin1_data[] = {0xC1, 0xE9, 0xF3}; - Local ab = ArrayBuffer::New(isolate, sizeof(latin1_data)); - memcpy(ab->GetBackingStore()->Data(), latin1_data, sizeof(latin1_data)); - - Local array = Uint8Array::New(ab, 0, sizeof(latin1_data)); - Local args[] = {array}; - - Local result; - ASSERT_TRUE(RunDecodeLatin1(env, args, false, false, &result)); - - ASSERT_TRUE(result->IsString()); - - String::Utf8Value utf8_result(isolate, result); - EXPECT_STREQ(*utf8_result, "Áéó"); -} - -} // namespace encoding_binding -} // namespace node diff --git a/test/parallel/test-internal-encoding-binding.js b/test/parallel/test-internal-encoding-binding.js new file mode 100644 index 00000000000000..b7483bf1d22820 --- /dev/null +++ b/test/parallel/test-internal-encoding-binding.js @@ -0,0 +1,48 @@ +// Flags: --expose-internals + +'use strict'; + +require('../common'); + +const assert = require('node:assert'); +const { internalBinding } = require('internal/test/binding'); +const binding = internalBinding('encoding_binding'); + +{ + // Valid input + const buf = Uint8Array.from([0xC1, 0xE9, 0xF3]); + assert.strictEqual(binding.decodeLatin1(buf, false, false), 'Áéó'); +} + +{ + // Empty input + const buf = Uint8Array.from([]); + assert.strictEqual(binding.decodeLatin1(buf, false, false), ''); +} + +{ + // Invalid input, but Latin1 has no invalid chars and should never throw. + const buf = new TextEncoder().encode('Invalid Latin1 🧑‍🧑‍🧒‍🧒'); + assert.strictEqual( + binding.decodeLatin1(buf, false, false), + 'Invalid Latin1 ð\x9F§\x91â\x80\x8Dð\x9F§\x91â\x80\x8Dð\x9F§\x92â\x80\x8Dð\x9F§\x92' + ); +} + +{ + // IgnoreBOM with BOM + const buf = Uint8Array.from([0xFE, 0xFF, 0xC1, 0xE9, 0xF3]); + assert.strictEqual(binding.decodeLatin1(buf, true, false), 'þÿÁéó'); +} + +{ + // Fatal and InvalidInput, but Latin1 has no invalid chars and should never throw. + const buf = Uint8Array.from([0xFF, 0xFF, 0xFF]); + assert.strictEqual(binding.decodeLatin1(buf, false, true), 'ÿÿÿ'); +} + +{ + // IgnoreBOM and Fatal, but Latin1 has no invalid chars and should never throw. + const buf = Uint8Array.from([0xFE, 0xFF, 0xC1, 0xE9, 0xF3]); + assert.strictEqual(binding.decodeLatin1(buf, true, true), 'þÿÁéó'); +} From c639e6580fbaa40f5e2a261b6156873b0dda3e7d Mon Sep 17 00:00:00 2001 From: Chengzhong Wu Date: Mon, 27 Jan 2025 15:54:23 +0000 Subject: [PATCH 2/2] test: search cctest files To prevent a new cctest missing from the `node.gyp`, search cctest files with tool `search_files.py` at configure time. --- node.gyp | 29 ++++++----------------------- tools/search_files.py | 3 +++ 2 files changed, 9 insertions(+), 23 deletions(-) diff --git a/node.gyp b/node.gyp index 6166f9d577ea41..5aed08ee4164b8 100644 --- a/node.gyp +++ b/node.gyp @@ -402,26 +402,8 @@ ], 'node_cctest_sources': [ 'src/node_snapshot_stub.cc', - 'test/cctest/node_test_fixture.cc', - 'test/cctest/node_test_fixture.h', - 'test/cctest/test_aliased_buffer.cc', - 'test/cctest/test_base64.cc', - 'test/cctest/test_base_object_ptr.cc', - 'test/cctest/test_cppgc.cc', - 'test/cctest/test_node_postmortem_metadata.cc', - 'test/cctest/test_node_task_runner.cc', - 'test/cctest/test_environment.cc', - 'test/cctest/test_linked_binding.cc', - 'test/cctest/test_node_api.cc', - 'test/cctest/test_path.cc', - 'test/cctest/test_per_process.cc', - 'test/cctest/test_platform.cc', - 'test/cctest/test_report.cc', - 'test/cctest/test_json_utils.cc', - 'test/cctest/test_sockaddr.cc', - 'test/cctest/test_traced_value.cc', - 'test/cctest/test_util.cc', - 'test/cctest/test_dataqueue.cc', + '