net: make net.BlockList cloneable

Signed-off-by: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/37917
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

doc/api/worker_threads.md

@@ -527,6 +527,9 @@ are part of the channel.
 <!-- YAML
 added: v10.5.0
 changes:
+  - version: REPLACEME
+    pr-url: https://github.com/nodejs/node/pull/37917
+    description: Add 'BlockList' to the list of cloneable types.
   - version: v15.9.0
     pr-url: https://github.com/nodejs/node/pull/37155
     description: Add 'Histogram' types to the list of cloneable types.
@@ -569,6 +572,7 @@ In particular, the significant differences to `JSON` are:
   * {Histogram}s,
   * {KeyObject}s,
   * {MessagePort}s,
+  * {net.BlockList}s,
   * {X509Certificate}s.
 
 ```js

lib/internal/blocklist.js

@@ -2,6 +2,7 @@
 
 const {
   Boolean,
+  ObjectSetPrototypeOf,
   Symbol
 } = primordials;
 
@@ -14,26 +15,28 @@ const {
 const {
   customInspectSymbol: kInspect,
 } = require('internal/util');
+
+const {
+  JSTransferable,
+  kClone,
+  kDeserialize,
+} = require('internal/worker/js_transferable');
+
 const { inspect } = require('internal/util/inspect');
 
 const kHandle = Symbol('kHandle');
 const { owner_symbol } = internalBinding('symbols');
 
 const {
-  ERR_INVALID_ARG_TYPE,
   ERR_INVALID_ARG_VALUE,
 } = require('internal/errors').codes;
 
 const { validateInt32, validateString } = require('internal/validators');
 
-class BlockList {
-  constructor(handle = new BlockListHandle()) {
-    // The handle argument is an intentionally undocumented
-    // internal API. User code will not be able to create
-    // a BlockListHandle object directly.
-    if (!(handle instanceof BlockListHandle))
-      throw new ERR_INVALID_ARG_TYPE('handle', 'BlockListHandle', handle);
-    this[kHandle] = handle;
+class BlockList extends JSTransferable {
+  constructor() {
+    super();
+    this[kHandle] = new BlockListHandle();
     this[kHandle][owner_symbol] = this;
   }
 
@@ -107,6 +110,34 @@ class BlockList {
   get rules() {
     return this[kHandle].getRules();
   }
+
+  [kClone]() {
+    const handle = this[kHandle];
+    return {
+      data: { handle },
+      deserializeInfo: 'internal/blocklist:InternalBlockList',
+    };
+  }
+
+  [kDeserialize]({ handle }) {
+    this[kHandle] = handle;
+    this[kHandle][owner_symbol] = this;
+  }
 }
 
-module.exports = BlockList;
+class InternalBlockList extends JSTransferable {
+  constructor(handle) {
+    super();
+    this[kHandle] = handle;
+    if (handle !== undefined)
+      handle[owner_symbol] = this;
+  }
+}
+
+InternalBlockList.prototype.constructor = BlockList.prototype.constructor;
+ObjectSetPrototypeOf(InternalBlockList.prototype, BlockList.prototype);
+
+module.exports = {
+  BlockList,
+  InternalBlockList,
+};

lib/net.js

@@ -1748,8 +1748,7 @@ module.exports = {
   _normalizeArgs: normalizeArgs,
   _setSimultaneousAccepts,
   get BlockList() {
-    if (BlockList === undefined)
-      BlockList = require('internal/blocklist');
+    BlockList ??= require('internal/blocklist').BlockList;
     return BlockList;
   },
   connect,

src/env-inl.h

@@ -1035,15 +1035,18 @@ inline void Environment::SetInstanceMethod(v8::Local<v8::FunctionTemplate> that,
 inline void Environment::SetConstructorFunction(
     v8::Local<v8::Object> that,
     const char* name,
-    v8::Local<v8::FunctionTemplate> tmpl) {
-  SetConstructorFunction(that, OneByteString(isolate(), name), tmpl);
+    v8::Local<v8::FunctionTemplate> tmpl,
+    SetConstructorFunctionFlag flag) {
+  SetConstructorFunction(that, OneByteString(isolate(), name), tmpl, flag);
 }
 
 inline void Environment::SetConstructorFunction(
     v8::Local<v8::Object> that,
     v8::Local<v8::String> name,
-    v8::Local<v8::FunctionTemplate> tmpl) {
-  tmpl->SetClassName(name);
+    v8::Local<v8::FunctionTemplate> tmpl,
+    SetConstructorFunctionFlag flag) {
+  if (LIKELY(flag == SetConstructorFunctionFlag::SET_CLASS_NAME))
+    tmpl->SetClassName(name);
   that->Set(
       context(),
       name,

src/env.h

@@ -442,7 +442,7 @@ constexpr size_t kFsStatsBufferLength =
   V(base_object_ctor_template, v8::FunctionTemplate)                           \
   V(binding_data_ctor_template, v8::FunctionTemplate)                          \
   V(blob_constructor_template, v8::FunctionTemplate)                           \
-  V(blocklist_instance_template, v8::ObjectTemplate)                           \
+  V(blocklist_constructor_template, v8::FunctionTemplate)                      \
   V(compiled_fn_entry_template, v8::ObjectTemplate)                            \
   V(dir_instance_template, v8::ObjectTemplate)                                 \
   V(fd_constructor_template, v8::ObjectTemplate)                               \
@@ -1231,13 +1231,22 @@ class Environment : public MemoryRetainer {
                                          const char* name,
                                          v8::FunctionCallback callback);
 
+  enum class SetConstructorFunctionFlag {
+    NONE,
+    SET_CLASS_NAME,
+  };
+
   inline void SetConstructorFunction(v8::Local<v8::Object> that,
                           const char* name,
-                          v8::Local<v8::FunctionTemplate> tmpl);
+                          v8::Local<v8::FunctionTemplate> tmpl,
+                          SetConstructorFunctionFlag flag =
+                              SetConstructorFunctionFlag::SET_CLASS_NAME);
 
   inline void SetConstructorFunction(v8::Local<v8::Object> that,
                           v8::Local<v8::String> name,
-                          v8::Local<v8::FunctionTemplate> tmpl);
+                          v8::Local<v8::FunctionTemplate> tmpl,
+                          SetConstructorFunctionFlag flag =
+                              SetConstructorFunctionFlag::SET_CLASS_NAME);
 
   void AtExit(void (*cb)(void* arg), void* arg);
   void RunAtExitCallbacks();

src/node_sockaddr.cc

@@ -390,6 +390,7 @@ SocketAddressBlockList::SocketAddressBlockList(
 
 void SocketAddressBlockList::AddSocketAddress(
     const SocketAddress& address) {
+  Mutex::ScopedLock lock(mutex_);
   std::unique_ptr<Rule> rule =
       std::make_unique<SocketAddressRule>(address);
   rules_.emplace_front(std::move(rule));
@@ -398,6 +399,7 @@ void SocketAddressBlockList::AddSocketAddress(
 
 void SocketAddressBlockList::RemoveSocketAddress(
     const SocketAddress& address) {
+  Mutex::ScopedLock lock(mutex_);
   auto it = address_rules_.find(address);
   if (it != std::end(address_rules_)) {
     rules_.erase(it->second);
@@ -408,6 +410,7 @@ void SocketAddressBlockList::RemoveSocketAddress(
 void SocketAddressBlockList::AddSocketAddressRange(
     const SocketAddress& start,
     const SocketAddress& end) {
+  Mutex::ScopedLock lock(mutex_);
   std::unique_ptr<Rule> rule =
       std::make_unique<SocketAddressRangeRule>(start, end);
   rules_.emplace_front(std::move(rule));
@@ -416,12 +419,14 @@ void SocketAddressBlockList::AddSocketAddressRange(
 void SocketAddressBlockList::AddSocketAddressMask(
     const SocketAddress& network,
     int prefix) {
+  Mutex::ScopedLock lock(mutex_);
   std::unique_ptr<Rule> rule =
       std::make_unique<SocketAddressMaskRule>(network, prefix);
   rules_.emplace_front(std::move(rule));
 }
 
 bool SocketAddressBlockList::Apply(const SocketAddress& address) {
+  Mutex::ScopedLock lock(mutex_);
   for (const auto& rule : rules_) {
     if (rule->Apply(address))
       return true;
@@ -488,14 +493,25 @@ std::string SocketAddressBlockList::SocketAddressMaskRule::ToString() {
 }
 
 MaybeLocal<Array> SocketAddressBlockList::ListRules(Environment* env) {
+  Mutex::ScopedLock lock(mutex_);
   std::vector<Local<Value>> rules;
+  if (!ListRules(env, &rules))
+    return MaybeLocal<Array>();
+  return Array::New(env->isolate(), rules.data(), rules.size());
+}
+
+bool SocketAddressBlockList::ListRules(
+    Environment* env,
+    std::vector<v8::Local<v8::Value>>* rules) {
+  if (parent_ && !parent_->ListRules(env, rules))
+    return false;
   for (const auto& rule : rules_) {
     Local<Value> str;
     if (!rule->ToV8String(env).ToLocal(&str))
-      return MaybeLocal<Array>();
-    rules.push_back(str);
+      return false;
+    rules->push_back(str);
   }
-  return Array::New(env->isolate(), rules.data(), rules.size());
+  return true;
 }
 
 void SocketAddressBlockList::MemoryInfo(node::MemoryTracker* tracker) const {
@@ -519,20 +535,42 @@ void SocketAddressBlockList::SocketAddressMaskRule::MemoryInfo(
 }
 
 SocketAddressBlockListWrap::SocketAddressBlockListWrap(
-    Environment* env, Local<Object> wrap)
-    : BaseObject(env, wrap) {
+    Environment* env,
+    Local<Object> wrap,
+    std::shared_ptr<SocketAddressBlockList> blocklist)
+    : BaseObject(env, wrap),
+      blocklist_(std::move(blocklist)) {
   MakeWeak();
 }
 
 BaseObjectPtr<SocketAddressBlockListWrap> SocketAddressBlockListWrap::New(
     Environment* env) {
   Local<Object> obj;
-  if (!env->blocklist_instance_template()
+  if (!env->blocklist_constructor_template()
+          ->InstanceTemplate()
           ->NewInstance(env->context()).ToLocal(&obj)) {
-    return {};
+    return BaseObjectPtr<SocketAddressBlockListWrap>();
   }
   BaseObjectPtr<SocketAddressBlockListWrap> wrap =
-      MakeDetachedBaseObject<SocketAddressBlockListWrap>(env, obj);
+      MakeBaseObject<SocketAddressBlockListWrap>(env, obj);
+  CHECK(wrap);
+  return wrap;
+}
+
+BaseObjectPtr<SocketAddressBlockListWrap> SocketAddressBlockListWrap::New(
+    Environment* env,
+    std::shared_ptr<SocketAddressBlockList> blocklist) {
+  Local<Object> obj;
+  if (!env->blocklist_constructor_template()
+          ->InstanceTemplate()
+          ->NewInstance(env->context()).ToLocal(&obj)) {
+    return BaseObjectPtr<SocketAddressBlockListWrap>();
+  }
+  BaseObjectPtr<SocketAddressBlockListWrap> wrap =
+      MakeBaseObject<SocketAddressBlockListWrap>(
+          env,
+          obj,
+          std::move(blocklist));
   CHECK(wrap);
   return wrap;
 }
@@ -562,7 +600,7 @@ void SocketAddressBlockListWrap::AddAddress(
   if (!SocketAddress::ToSockAddr(family, *value, 0, &address))
     return;
 
-  wrap->AddSocketAddress(
+  wrap->blocklist_->AddSocketAddress(
       SocketAddress(reinterpret_cast<const sockaddr*>(&address)));
 
   args.GetReturnValue().Set(true);
@@ -597,7 +635,7 @@ void SocketAddressBlockListWrap::AddRange(
   if (start_addr > end_addr)
     return args.GetReturnValue().Set(false);
 
-  wrap->AddSocketAddressRange(start_addr, end_addr);
+  wrap->blocklist_->AddSocketAddressRange(start_addr, end_addr);
 
   args.GetReturnValue().Set(true);
 }
@@ -628,7 +666,7 @@ void SocketAddressBlockListWrap::AddSubnet(
   CHECK_IMPLIES(family == AF_INET6, prefix <= 128);
   CHECK_GE(prefix, 0);
 
-  wrap->AddSocketAddressMask(
+  wrap->blocklist_->AddSocketAddressMask(
       SocketAddress(reinterpret_cast<const sockaddr*>(&address)),
       prefix);
 
@@ -654,7 +692,8 @@ void SocketAddressBlockListWrap::Check(
     return;
 
   args.GetReturnValue().Set(
-      wrap->Apply(SocketAddress(reinterpret_cast<const sockaddr*>(&address))));
+      wrap->blocklist_->Apply(
+          SocketAddress(reinterpret_cast<const sockaddr*>(&address))));
 }
 
 void SocketAddressBlockListWrap::GetRules(
@@ -663,10 +702,43 @@ void SocketAddressBlockListWrap::GetRules(
   SocketAddressBlockListWrap* wrap;
   ASSIGN_OR_RETURN_UNWRAP(&wrap, args.Holder());
   Local<Array> rules;
-  if (wrap->ListRules(env).ToLocal(&rules))
+  if (wrap->blocklist_->ListRules(env).ToLocal(&rules))
     args.GetReturnValue().Set(rules);
 }
 
+void SocketAddressBlockListWrap::MemoryInfo(MemoryTracker* tracker) const {
+  blocklist_->MemoryInfo(tracker);
+}
+
+std::unique_ptr<worker::TransferData>
+SocketAddressBlockListWrap::CloneForMessaging() const {
+  return std::make_unique<TransferData>(this);
+}
+
+bool SocketAddressBlockListWrap::HasInstance(
+    Environment* env,
+    Local<Value> value) {
+  return GetConstructorTemplate(env)->HasInstance(value);
+}
+
+Local<FunctionTemplate> SocketAddressBlockListWrap::GetConstructorTemplate(
+    Environment* env) {
+  Local<FunctionTemplate> tmpl = env->blocklist_constructor_template();
+  if (tmpl.IsEmpty()) {
+    tmpl = env->NewFunctionTemplate(SocketAddressBlockListWrap::New);
+    tmpl->SetClassName(FIXED_ONE_BYTE_STRING(env->isolate(), "BlockList"));
+    tmpl->Inherit(BaseObject::GetConstructorTemplate(env));
+    tmpl->InstanceTemplate()->SetInternalFieldCount(kInternalFieldCount);
+    env->SetProtoMethod(tmpl, "addAddress", AddAddress);
+    env->SetProtoMethod(tmpl, "addRange", AddRange);
+    env->SetProtoMethod(tmpl, "addSubnet", AddSubnet);
+    env->SetProtoMethod(tmpl, "check", Check);
+    env->SetProtoMethod(tmpl, "getRules", GetRules);
+    env->set_blocklist_constructor_template(tmpl);
+  }
+  return tmpl;
+}
+
 void SocketAddressBlockListWrap::Initialize(
     Local<Object> target,
     Local<Value> unused,
@@ -674,23 +746,28 @@ void SocketAddressBlockListWrap::Initialize(
     void* priv) {
   Environment* env = Environment::GetCurrent(context);
 
-  Local<FunctionTemplate> t =
-      env->NewFunctionTemplate(SocketAddressBlockListWrap::New);
-  t->InstanceTemplate()->SetInternalFieldCount(BaseObject::kInternalFieldCount);
-
-  env->SetProtoMethod(t, "addAddress", SocketAddressBlockListWrap::AddAddress);
-  env->SetProtoMethod(t, "addRange", SocketAddressBlockListWrap::AddRange);
-  env->SetProtoMethod(t, "addSubnet", SocketAddressBlockListWrap::AddSubnet);
-  env->SetProtoMethod(t, "check", SocketAddressBlockListWrap::Check);
-  env->SetProtoMethod(t, "getRules", SocketAddressBlockListWrap::GetRules);
-
-  env->set_blocklist_instance_template(t->InstanceTemplate());
-  env->SetConstructorFunction(target, "BlockList", t);
+  env->SetConstructorFunction(
+      target,
+      "BlockList",
+      GetConstructorTemplate(env),
+      Environment::SetConstructorFunctionFlag::NONE);
 
   NODE_DEFINE_CONSTANT(target, AF_INET);
   NODE_DEFINE_CONSTANT(target, AF_INET6);
 }
 
+BaseObjectPtr<BaseObject> SocketAddressBlockListWrap::TransferData::Deserialize(
+    Environment* env,
+    Local<Context> context,
+    std::unique_ptr<worker::TransferData> self) {
+  return New(env, std::move(blocklist_));
+}
+
+void SocketAddressBlockListWrap::TransferData::MemoryInfo(
+    MemoryTracker* tracker) const {
+  blocklist_->MemoryInfo(tracker);
+}
+
 }  // namespace node
 
 NODE_MODULE_CONTEXT_AWARE_INTERNAL(

src/node_sockaddr.h

@@ -7,6 +7,7 @@
 #include "memory_tracker.h"
 #include "base_object.h"
 #include "node.h"
+#include "node_worker.h"
 #include "uv.h"
 #include "v8.h"
 
@@ -267,21 +268,32 @@ class SocketAddressBlockList : public MemoryRetainer {
   SET_SELF_SIZE(SocketAddressBlockList)
 
  private:
+  bool ListRules(
+      Environment* env,
+      std::vector<v8::Local<v8::Value>>* vec);
+
   std::shared_ptr<SocketAddressBlockList> parent_;
   std::list<std::unique_ptr<Rule>> rules_;
   SocketAddress::Map<std::list<std::unique_ptr<Rule>>::iterator> address_rules_;
+
+  Mutex mutex_;
 };
 
-class SocketAddressBlockListWrap :
-    public BaseObject,
-    public SocketAddressBlockList {
+class SocketAddressBlockListWrap : public BaseObject {
  public:
+  static bool HasInstance(Environment* env, v8::Local<v8::Value> value);
+  static v8::Local<v8::FunctionTemplate> GetConstructorTemplate(
+      Environment* env);
   static void Initialize(v8::Local<v8::Object> target,
                          v8::Local<v8::Value> unused,
                          v8::Local<v8::Context> context,
                          void* priv);
 
   static BaseObjectPtr<SocketAddressBlockListWrap> New(Environment* env);
+  static BaseObjectPtr<SocketAddressBlockListWrap> New(
+      Environment* env,
+      std::shared_ptr<SocketAddressBlockList> blocklist);
+
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void AddAddress(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void AddRange(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -291,13 +303,43 @@ class SocketAddressBlockListWrap :
 
   SocketAddressBlockListWrap(
       Environment* env,
-      v8::Local<v8::Object> wrap);
+      v8::Local<v8::Object> wrap,
+      std::shared_ptr<SocketAddressBlockList> blocklist =
+          std::make_shared<SocketAddressBlockList>());
 
-  void MemoryInfo(node::MemoryTracker* tracker) const override {
-    SocketAddressBlockList::MemoryInfo(tracker);
-  }
+  void MemoryInfo(node::MemoryTracker* tracker) const override;
   SET_MEMORY_INFO_NAME(SocketAddressBlockListWrap)
   SET_SELF_SIZE(SocketAddressBlockListWrap)
+
+  TransferMode GetTransferMode() const override {
+    return TransferMode::kCloneable;
+  }
+  std::unique_ptr<worker::TransferData> CloneForMessaging() const override;
+
+  class TransferData : public worker::TransferData {
+   public:
+    inline explicit TransferData(const SocketAddressBlockListWrap* wrap)
+        : blocklist_(wrap->blocklist_) {}
+
+    inline explicit TransferData(
+        std::shared_ptr<SocketAddressBlockList> blocklist)
+        : blocklist_(std::move(blocklist)) {}
+
+    BaseObjectPtr<BaseObject> Deserialize(
+        Environment* env,
+        v8::Local<v8::Context> context,
+        std::unique_ptr<worker::TransferData> self) override;
+
+    void MemoryInfo(MemoryTracker* tracker) const override;
+    SET_MEMORY_INFO_NAME(SocketAddressBlockListWrap::TransferData)
+    SET_SELF_SIZE(TransferData)
+
+   private:
+    std::shared_ptr<SocketAddressBlockList> blocklist_;
+  };
+
+ private:
+  std::shared_ptr<SocketAddressBlockList> blocklist_;
 };
 
 }  // namespace node

test/parallel/test-blocklist-clone.js

@@ -0,0 +1,31 @@
+'use strict';
+
+const common = require('../common');
+
+const {
+  BlockList,
+} = require('net');
+
+const {
+  ok,
+  notStrictEqual,
+} = require('assert');
+
+const blocklist = new BlockList();
+blocklist.addAddress('123.123.123.123');
+
+const mc = new MessageChannel();
+
+mc.port1.onmessage = common.mustCall(({ data }) => {
+  notStrictEqual(data, blocklist);
+  ok(data.check('123.123.123.123'));
+  ok(!data.check('123.123.123.124'));
+
+  data.addAddress('123.123.123.124');
+  ok(blocklist.check('123.123.123.124'));
+  ok(data.check('123.123.123.124'));
+
+  mc.port1.close();
+});
+
+mc.port2.postMessage(blocklist);

test/parallel/test-blocklist.js

@@ -137,10 +137,6 @@ const util = require('util');
   assert(!blockList.check('8592:757c:efaf:2fff:ffff:ffff:ffff:ffff', 'ipv6'));
 }
 
-{
-  assert.throws(() => new BlockList('NOT BLOCK LIST HANDLE'), /ERR_INVALID_ARG_TYPE/);
-}
-
 {
   const blockList = new BlockList();
   assert.throws(() => blockList.addRange('1.1.1.2', '1.1.1.1'), /ERR_INVALID_ARG_VALUE/);